diff --git a/scripts/double-transfer.js b/scripts/double-transfer.js deleted file mode 100644 index d86d396..0000000 --- a/scripts/double-transfer.js +++ /dev/null @@ -1,69 +0,0 @@ -import "dotenv/config"; -import { Account, Contract, RpcProvider } from "starknet"; - -// connect provider -const provider = new RpcProvider({ nodeUrl: process.env.RPC_PROVIDER }); -const account = new Account(provider, process.env.ACCOUNT, process.env.PRIVATE_KEY); -const ethAddress = "0x049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7"; -const ethClassHash = "0x05ffbcfeb50d200a0677c48a129a11245a3fc519d1d98d76882d1c9a1b19c6ed"; -const receiver = "0x01a10f22c98BA5Fb02374089EAA9c62deaced318a909c264a5270B2844CCb37d"; -const amount = 50; -const maxFee = 1e15; - -// setInterval(getNonce, 3000); - -const ethContract = await loadContract(ethAddress, ethClassHash); - -let nonce = await getNonce(); - -doSendTx(nonce, amount, "0"); -doSendTx(nonce, amount + 1, "0b"); -doSendTx(nonce + 1, amount + 3, "+1"); -doSendTx(nonce + 1, amount + 4, "+1b"); -doSendTx(nonce + 2, amount + 5, "+2"); -doSendTx(nonce + 2, amount + 6, "+2b"); - -function doSendTx(nonce, amount, name) { - account - .execute(ethContract.populateTransaction.transfer(receiver, amount), undefined, { - skipValidate: true, - maxFee, - nonce, - }) - .then(async (tx) => handle(name, tx)); -} -async function handle(name, tx) { - console.log(`${getFormattedDate()} ${name}`); - getNonce(); - console.log(tx); - try { - const x = await provider.waitForTransaction(tx.transaction_hash); - console.log(`${getFormattedDate()} result ${name}`); - console.log(x); - getNonce(); - } catch (error) { - console.log(`${getFormattedDate()} error ${name}`); - console.log(error); - getNonce(); - } -} - -function getFormattedDate() { - return "[" + new Date().toLocaleTimeString() + "]"; -} - -async function getNonce() { - let nonce = await account.getNonce(); - console.log(`${getFormattedDate()} Nonce: ${nonce}`); - return Number(nonce); -} - -async function loadContract(contractAddress) { - const { abi } = await provider.getClassAt(contractAddress); - return new Contract( - abi, - contractAddress, - provider, - "0x05ffbcfeb50d200a0677c48a129a11245a3fc519d1d98d76882d1c9a1b19c6ed", - ); -} diff --git a/scripts/monitor-nonce.js b/scripts/monitor-nonce.js deleted file mode 100644 index ff3a28a..0000000 --- a/scripts/monitor-nonce.js +++ /dev/null @@ -1,17 +0,0 @@ -import "dotenv/config"; -import { Account, RpcProvider } from "starknet"; - -// connect provider -const provider = new RpcProvider({ nodeUrl: process.env.RPC_PROVIDER }); -const account = new Account(provider, process.env.ACCOUNT); - -async function monitorNonce() { - let nonce = await account.getNonce("latest"); - console.log(`${getFormattedDate()} Nonce: ${nonce}`); -} - -function getFormattedDate() { - return "[" + new Date().toLocaleTimeString() + "]"; -} - -setInterval(monitorNonce, 1000); diff --git a/scripts/profile.ts b/scripts/profile.ts index 1cff752..e1a1784 100644 --- a/scripts/profile.ts +++ b/scripts/profile.ts @@ -34,6 +34,7 @@ for (const useTxV3 of [false, true]) { // Ensure there is a contract for the claim const claimAddress = await factory.get_claim_address( + claimAccountClassHash, deployer.address, amount, maxFee, diff --git a/src/contracts/claim_account.cairo b/src/contracts/claim_account.cairo index 41f450c..a7ede61 100644 --- a/src/contracts/claim_account.cairo +++ b/src/contracts/claim_account.cairo @@ -7,14 +7,13 @@ mod ClaimAccount { use openzeppelin::token::erc20::interface::{IERC20, IERC20DispatcherTrait, IERC20Dispatcher}; use starknet::{ ClassHash, account::Call, VALIDATED, call_contract_syscall, ContractAddress, get_contract_address, - get_caller_address, contract_address::contract_address_const, get_execution_info + get_caller_address, contract_address::contract_address_const, get_execution_info, info::v2::ResourceBounds, }; use starknet_gifting::contracts::claim_hash::{ClaimExternal, IOffChainMessageHashRev1}; use starknet_gifting::contracts::claim_utils::calculate_claim_account_address; use starknet_gifting::contracts::interface::{IAccount, IGiftAccount, ClaimData, AccountConstructorArguments}; use starknet_gifting::contracts::utils::{ - full_deserialize, STRK_ADDRESS, ETH_ADDRESS, TX_V1_ESTIMATE, TX_V1, TX_V3, TX_V3_ESTIMATE, compute_max_fee_v3, - execute_multicall + full_deserialize, STRK_ADDRESS, ETH_ADDRESS, TX_V1_ESTIMATE, TX_V1, TX_V3, TX_V3_ESTIMATE, execute_multicall }; #[storage] struct Storage {} @@ -56,12 +55,10 @@ mod ClaimAccount { if claim.token == STRK_ADDRESS() { assert(tx_version == TX_V3 || tx_version == TX_V3_ESTIMATE, 'gift-acc/invalid-tx3-version'); let tx_fee = compute_max_fee_v3(tx_info.resource_bounds, tx_info.tip); - // TODO: should this error be max fee too high? - assert(tx_fee <= claim.max_fee, 'gift-acc/insufficient-v3-fee'); + assert(tx_fee <= claim.max_fee, 'gift-acc/max-fee-too-high-v3'); } else if claim.token == ETH_ADDRESS() { assert(tx_version == TX_V1 || tx_version == TX_V1_ESTIMATE, 'gift-acc/invalid-tx1-version'); - // TODO: should this error be max fee too high? - assert(tx_info.max_fee <= claim.max_fee, 'gift-acc/insufficient-v1-fee'); + assert(tx_info.max_fee <= claim.max_fee, 'gift-acc/max-fee-too-high-v1'); } else { core::panic_with_felt252('gift-acc/invalid-token'); } @@ -99,4 +96,18 @@ mod ClaimAccount { assert(calculated_address == get_contract_address(), 'gift-acc/invalid-claim-address'); } } + + fn compute_max_fee_v3(mut resource_bounds: Span, tip: u128) -> u128 { + let mut max_fee: u128 = 0; + let mut max_tip: u128 = 0; + while let Option::Some(r) = resource_bounds + .pop_front() { + let max_resource_amount: u128 = (*r.max_amount).into(); + max_fee += *r.max_price_per_unit * max_resource_amount; + if *r.resource == 'L2_GAS' { + max_tip += tip * max_resource_amount; + } + }; + max_fee + max_tip + } } diff --git a/src/contracts/claim_hash.cairo b/src/contracts/claim_hash.cairo index 3d9366a..14d929a 100644 --- a/src/contracts/claim_hash.cairo +++ b/src/contracts/claim_hash.cairo @@ -23,7 +23,6 @@ struct StarknetDomain { #[derive(Drop, Copy)] struct ClaimExternal { - claim: ClaimData, receiver: ContractAddress } diff --git a/src/contracts/gift_factory.cairo b/src/contracts/gift_factory.cairo index fd71c0c..7756003 100644 --- a/src/contracts/gift_factory.cairo +++ b/src/contracts/gift_factory.cairo @@ -102,16 +102,15 @@ mod GiftFactory { ) { self.pausable.assert_not_paused(); assert(token == STRK_ADDRESS() || token == ETH_ADDRESS(), 'gift-fac/invalid-token'); - // TODO: Assert max_fee is not zero? assert(max_fee.into() < amount, 'gift-fac/fee-too-high'); let sender = get_caller_address(); let factory = get_contract_address(); // TODO We could manually serialize for better performance - // TODO pubkey can be zero? + let class_hash = self.claim_class_hash.read(); let constructor_arguments = AccountConstructorArguments { sender, amount, max_fee, token, claim_pubkey }; let (claim_contract, _) = deploy_syscall( - self.claim_class_hash.read(), // class_hash + class_hash, // class_hash 0, // salt serialize(@constructor_arguments).span(), // constructor data false // deploy_from_zero @@ -120,14 +119,7 @@ mod GiftFactory { self .emit( GiftCreated { - claim_pubkey, - factory, - gift_address: claim_contract, - class_hash: self.claim_class_hash.read(), - sender, - amount, - max_fee, - token, + claim_pubkey, factory, gift_address: claim_contract, class_hash, sender, amount, max_fee, token, } ); let transfer_status = IERC20Dispatcher { contract_address: token } @@ -139,7 +131,7 @@ mod GiftFactory { let claim_address = self.check_factory_and_get_account_address(claim); assert(get_caller_address() == claim_address, 'gift/only-claim-account'); let balance = IERC20Dispatcher { contract_address: claim.token }.balance_of(claim_address); - assert(balance > claim.amount, 'gift-acc/gift-canceled'); + assert(balance >= claim.amount, 'gift/already-claimed-or-cancel'); self.transfer_from_account(claim, claim_address, claim.token, claim.amount, receiver); self.emit(GiftClaimed { receiver }); } @@ -148,25 +140,26 @@ mod GiftFactory { ref self: ContractState, claim: ClaimData, receiver: ContractAddress, signature: Array ) { let claim_address = self.check_factory_and_get_account_address(claim); - let claim_external_hash = ClaimExternal { claim, receiver }.get_message_hash_rev_1(claim_address); + let claim_external_hash = ClaimExternal { receiver }.get_message_hash_rev_1(claim_address); assert( check_ecdsa_signature(claim_external_hash, claim.claim_pubkey, *signature[0], *signature[1]), - 'gift-acc/invalid-ext-signature' + 'gift/invalid-ext-signature' ); let balance = IERC20Dispatcher { contract_address: claim.token }.balance_of(claim_address); + assert(balance >= claim.amount, 'gift/already-claimed-or-cancel'); self.transfer_from_account(claim, claim_address, claim.token, balance, receiver); self.emit(GiftClaimed { receiver }); } fn cancel(ref self: ContractState, claim: ClaimData) { let claim_address = self.check_factory_and_get_account_address(claim); - assert(get_caller_address() == claim.sender, 'gift-acc/wrong-sender'); + assert(get_caller_address() == claim.sender, 'gift/wrong-sender'); let balance = IERC20Dispatcher { contract_address: claim.token }.balance_of(claim_address); // Won't that lead to the sender also being able to get the extra dust? // assert(balance > claim.max_fee, 'already claimed'); - assert(balance > 0, 'gift-acc/already-claimed'); + assert(balance > 0, 'gift/already-claimed'); self.transfer_from_account(claim, claim_address, claim.token, balance, claim.sender); self.emit(GiftCanceled {}); } @@ -181,12 +174,13 @@ mod GiftFactory { self.transfer_from_account(claim, claim_address, claim.token, balance, receiver); } - fn get_claim_class_hash(ref self: ContractState) -> ClassHash { + fn get_latest_claim_class_hash(self: @ContractState) -> ClassHash { self.claim_class_hash.read() } fn get_claim_address( self: @ContractState, + class_hash: ClassHash, sender: ContractAddress, amount: u256, max_fee: u128, @@ -194,15 +188,7 @@ mod GiftFactory { claim_pubkey: felt252 ) -> ContractAddress { calculate_claim_account_address( - ClaimData { - factory: get_contract_address(), - class_hash: self.claim_class_hash.read(), - sender, - amount, - max_fee, - token, - claim_pubkey, - } + ClaimData { factory: get_contract_address(), class_hash, sender, amount, max_fee, token, claim_pubkey, } ) } } @@ -254,7 +240,7 @@ mod GiftFactory { ] ); let transfer_status = full_deserialize::(*results.at(0)).expect('gift/invalid-result-calldata'); - assert(transfer_status, 'gift-acc/transfer-failed'); + assert(transfer_status, 'gift/transfer-failed'); } } } diff --git a/src/contracts/interface.cairo b/src/contracts/interface.cairo index b405f0c..effe5a2 100644 --- a/src/contracts/interface.cairo +++ b/src/contracts/interface.cairo @@ -10,23 +10,21 @@ trait IAccount { #[starknet::interface] trait IGiftFactory { fn deposit(ref self: TContractState, amount: u256, max_fee: u128, token: ContractAddress, claim_pubkey: felt252); + fn claim_internal(ref self: TContractState, claim: ClaimData, receiver: ContractAddress); + fn claim_external(ref self: TContractState, claim: ClaimData, receiver: ContractAddress, signature: Array); + fn cancel(ref self: TContractState, claim: ClaimData); + fn get_dust(ref self: TContractState, claim: ClaimData, receiver: ContractAddress); + + fn get_latest_claim_class_hash(self: @TContractState) -> ClassHash; fn get_claim_address( self: @TContractState, + class_hash: ClassHash, sender: ContractAddress, amount: u256, max_fee: u128, token: ContractAddress, claim_pubkey: felt252 ) -> ContractAddress; - fn get_claim_class_hash(ref self: TContractState) -> ClassHash; - - fn claim_internal(ref self: TContractState, claim: ClaimData, receiver: ContractAddress); - - fn claim_external(ref self: TContractState, claim: ClaimData, receiver: ContractAddress, signature: Array); - - fn cancel(ref self: TContractState, claim: ClaimData); - - fn get_dust(ref self: TContractState, claim: ClaimData, receiver: ContractAddress); } #[starknet::interface] diff --git a/src/contracts/utils.cairo b/src/contracts/utils.cairo index 93131e0..68901cb 100644 --- a/src/contracts/utils.cairo +++ b/src/contracts/utils.cairo @@ -2,10 +2,7 @@ use core::hash::{HashStateTrait, HashStateExTrait, Hash}; use core::poseidon::{PoseidonTrait, HashState}; use openzeppelin::token::erc20::interface::IERC20Dispatcher; -use starknet::{ - ContractAddress, account::Call, contract_address::contract_address_const, info::v2::ResourceBounds, - call_contract_syscall -}; +use starknet::{ContractAddress, account::Call, contract_address::contract_address_const, call_contract_syscall}; pub const TX_V1: felt252 = 1; // INVOKE pub const TX_V1_ESTIMATE: felt252 = consteval_int!(0x100000000000000000000000000000000 + 1); // 2**128 + TX_V1 @@ -37,21 +34,6 @@ fn serialize>(value: @E) -> Array { output } - -fn compute_max_fee_v3(mut resource_bounds: Span, tip: u128) -> u128 { - let mut max_fee: u128 = 0; - let mut max_tip: u128 = 0; - while let Option::Some(r) = resource_bounds - .pop_front() { - let max_resource_amount: u128 = (*r.max_amount).into(); - max_fee += *r.max_price_per_unit * max_resource_amount; - if *r.resource == 'L2_GAS' { - max_tip += tip * max_resource_amount; - } - }; - max_fee + max_tip -} - fn execute_multicall(mut calls: Span) -> Array> { let mut result = array![]; let mut index = 0; diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index d108b26..08bd3af 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -28,6 +28,7 @@ describe("Gifting", function () { // Ensure there is a contract for the claim const claimAddress = await factory.get_claim_address( + claimAccountClassHash, deployer.address, amount, maxFee, @@ -74,7 +75,7 @@ describe("Gifting", function () { max_price_per_unit: num.toHexString(4), }, }; - await expectRevertWithErrorMessage("gift-acc/insufficient-v3-fee", () => + await expectRevertWithErrorMessage("gift-acc/max-fee-too-high-v3", () => claimAccount.execute( [ { @@ -88,7 +89,7 @@ describe("Gifting", function () { ), ); } else { - await expectRevertWithErrorMessage("gift-acc/insufficient-v1-fee", () => + await expectRevertWithErrorMessage("gift-acc/max-fee-too-high-v1", () => factory.claim_internal(claim, receiver, { maxFee: maxFee + 1n }), ); } @@ -124,6 +125,7 @@ describe("Gifting", function () { // Ensure there is a contract for the claim const claimAddress = await factory.get_claim_address( + claimAccountClassHash, deployer.address, amount, maxFee, @@ -131,9 +133,7 @@ describe("Gifting", function () { claimPubkey, ); - const claim = { - factory: factory.address, - class_hash: claimAccountClassHash, + const constructorArgs = { sender: deployer.address, amount: uint256.bnToUint256(amount), max_fee: maxFee, @@ -141,8 +141,13 @@ describe("Gifting", function () { claim_pubkey: claimPubkey, }; - const constructorCalldata = CallData.compile(claim); - const correctAddress = hash.calculateContractAddressFromHash(0, claimAccountClassHash, constructorCalldata, 0); + const constructorCalldata = CallData.compile(constructorArgs); + const correctAddress = hash.calculateContractAddressFromHash( + 0, + claimAccountClassHash, + constructorCalldata, + factory.address, + ); expect(claimAddress).to.be.equal(num.toBigInt(correctAddress)); // Check balance of the claim contract is correct @@ -156,6 +161,12 @@ describe("Gifting", function () { claimContract.connect(claimAccount); await expectRevertWithErrorMessage("gift-acc/only-protocol", () => claimContract.__validate__([])); + const claim = { + factory: factory.address, + class_hash: claimAccountClassHash, + ...constructorArgs + }; + // cant call another contract fakeFactory.connect(claimAccount); await expectRevertWithErrorMessage("gift-acc/invalid-call-to", () => @@ -164,7 +175,7 @@ describe("Gifting", function () { // wrong selector factory.connect(claimAccount); - await expectRevertWithErrorMessage("gift-acc/invalid-call-selector", () => factory.get_claim_class_hash()); + await expectRevertWithErrorMessage("gift-acc/invalid-call-selector", () => factory.get_latest_claim_class_hash()); // multicall await expectRevertWithErrorMessage("gift-acc/invalid-call-len", () => diff --git a/tests-integration/claim_external.test.ts b/tests-integration/claim_external.test.ts index c00d171..f8c67d7 100644 --- a/tests-integration/claim_external.test.ts +++ b/tests-integration/claim_external.test.ts @@ -25,6 +25,7 @@ describe("claim_external", function () { await factory.deposit(amount, maxFee, tokenContract.address, claimPubkey); const claimAddress = await factory.get_claim_address( + claimAccountClassHash, deployer.address, amount, maxFee, diff --git a/tests-integration/factory.test.ts b/tests-integration/factory.test.ts index 7b61436..7dbddad 100644 --- a/tests-integration/factory.test.ts +++ b/tests-integration/factory.test.ts @@ -28,6 +28,7 @@ describe("Factory", function () { // Ensure there is a contract for the claim const claimAddress = await factory.get_claim_address( + claimAccountClassHash, deployer.address, amount, maxFee, @@ -94,6 +95,7 @@ describe("Factory", function () { // Ensure there is a contract for the claim const claimAddress = await factory.get_claim_address( + claimAccountClassHash, deployer.address, amount, maxFee, @@ -130,7 +132,7 @@ describe("Factory", function () { await tokenContract.balance_of(claimAddress).should.eventually.equal(0n); factory.connect(claimAccount); - await expectRevertWithErrorMessage("gift-acc/gift-canceled", () => factory.claim_internal(claim, receiver)); + await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () => factory.claim_internal(claim, receiver)); }); it(`Test pausable`, async function () { @@ -166,6 +168,7 @@ describe("Factory", function () { // Ensure there is a contract for the claim const claimAddress = await factory.get_claim_address( + claimAccountClassHash, deployer.address, amount, maxFee, diff --git a/tests/setup.cairo b/tests/setup.cairo index 66f253c..a60cfc3 100644 --- a/tests/setup.cairo +++ b/tests/setup.cairo @@ -34,7 +34,7 @@ fn deploy_gifting_broken_erc20() -> GiftingSetup { ]; let (factory_contract_address, _) = factory_contract.deploy(@factory_calldata).expect('Failed to deploy factory'); let gift_factory = IGiftFactoryDispatcher { contract_address: factory_contract_address }; - assert(gift_factory.get_claim_class_hash() == claim_contract.class_hash, 'Incorrect factory setup'); + assert(gift_factory.get_latest_claim_class_hash() == claim_contract.class_hash, 'Incorrect factory setup'); GiftingSetup { mock_eth: broken_erc20, mock_strk: broken_erc20, gift_factory, claim_class_hash: claim_contract.class_hash @@ -83,7 +83,7 @@ fn deploy_gifting_normal() -> GiftingSetup { ]; let (factory_contract_address, _) = factory_contract.deploy(@factory_calldata).expect('Failed to deploy factory'); let gift_factory = IGiftFactoryDispatcher { contract_address: factory_contract_address }; - assert(gift_factory.get_claim_class_hash() == claim_contract.class_hash, 'Incorrect factory setup'); + assert(gift_factory.get_latest_claim_class_hash() == claim_contract.class_hash, 'Incorrect factory setup'); start_cheat_caller_address(mock_eth_address, OWNER()); start_cheat_caller_address(mock_strk.contract_address, OWNER()); diff --git a/tests/test_gift_factory.cairo b/tests/test_gift_factory.cairo index 8ad8836..f59270a 100644 --- a/tests/test_gift_factory.cairo +++ b/tests/test_gift_factory.cairo @@ -75,12 +75,17 @@ fn test_claim_account_deployed() { // un-deployed claim account should return 0 let fetched_claim_class_hash = get_class_hash(calculated_claim_address); assert(claim_class_hash == fetched_claim_class_hash, 'Claim account not deployed'); - assert(claim_class_hash == gift_factory.get_claim_class_hash(), 'Incorrect claim class hash'); + assert(claim_class_hash == gift_factory.get_latest_claim_class_hash(), 'Incorrect claim class hash'); // Check that factory calculates claim address correctly let get_claim_address = gift_factory .get_claim_address( - claim_data.sender, claim_data.amount, claim_data.max_fee, claim_data.token, claim_data.claim_pubkey + claim_data.class_hash, + claim_data.sender, + claim_data.amount, + claim_data.max_fee, + claim_data.token, + claim_data.claim_pubkey ); assert!(calculated_claim_address == get_claim_address, "Claim address not calculated correctly"); }