Fix small remarks

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<ResourceBounds>, 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
+    }
 }

src/contracts/claim_hash.cairo

@@ -23,7 +23,6 @@ struct StarknetDomain {
 
 #[derive(Drop, Copy)]
 struct ClaimExternal {
-    claim: ClaimData,
     receiver: ContractAddress
 }
 

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<felt252>
         ) {
             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,28 +174,21 @@ 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,
             token: ContractAddress,
             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::<bool>(*results.at(0)).expect('gift/invalid-result-calldata');
-            assert(transfer_status, 'gift-acc/transfer-failed');
+            assert(transfer_status, 'gift/transfer-failed');
         }
     }
 }

src/contracts/interface.cairo

@@ -10,23 +10,21 @@ trait IAccount<TContractState> {
 #[starknet::interface]
 trait IGiftFactory<TContractState> {
     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<felt252>);
+    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<felt252>);
-
-    fn cancel(ref self: TContractState, claim: ClaimData);
-
-    fn get_dust(ref self: TContractState, claim: ClaimData, receiver: ContractAddress);
 }
 
 #[starknet::interface]

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<E, impl ESerde: Serde<E>>(value: @E) -> Array<felt252> {
     output
 }
 
-
-fn compute_max_fee_v3(mut resource_bounds: Span<ResourceBounds>, 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<Call>) -> Array<Span<felt252>> {
     let mut result = array![];
     let mut index = 0;

tests-integration/account.test.ts

@@ -164,7 +164,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", () =>

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());

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");
 }