From a325e9c021599beeec5b93907b518f275acbf278 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 10:55:52 +0200 Subject: [PATCH 1/5] first draft --- src/contracts/escrow_account.cairo | 48 +++++++++++++----------- src/contracts/escrow_library.cairo | 28 ++++++++------ src/contracts/gift_factory.cairo | 7 +++- src/contracts/timelock_upgrade.cairo | 1 + tests-integration/account.test.ts | 14 +++---- tests-integration/cancel.test.ts | 10 ++--- tests-integration/claim_external.test.ts | 6 +-- tests-integration/claim_internal.test.ts | 8 ++-- tests-integration/factory.test.ts | 2 +- tests-integration/upgrade.test.ts | 2 +- 10 files changed, 70 insertions(+), 56 deletions(-) diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 8a1b0c9..4bf867e 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -76,55 +76,60 @@ mod EscrowAccount { impl IAccountImpl of IAccount { fn __validate__(ref self: ContractState, calls: Array) -> felt252 { let execution_info = get_execution_info().unbox(); - assert(execution_info.caller_address.is_zero(), 'gift-acc/only-protocol'); - assert(calls.len() == 1, 'gift-acc/invalid-call-len'); + assert(execution_info.caller_address.is_zero(), 'escrow/only-protocol'); // Not tested + assert(calls.len() == 1, 'escrow/invalid-call-len'); let Call { to, selector, calldata } = calls.at(0); - assert(*to == get_contract_address(), 'gift-acc/invalid-call-to'); - assert(*selector == selector!("claim_internal"), 'gift-acc/invalid-call-selector'); + assert(*to == get_contract_address(), 'escrow/invalid-call-to'); + assert(*selector == selector!("claim_internal"), 'escrow/invalid-call-selector'); let (gift, _): (GiftData, ContractAddress) = full_deserialize(*calldata) - .expect('gift-acc/invalid-calldata'); + .expect('escrow/invalid-calldata'); // Not tested assert_valid_claim(gift); let tx_info = execution_info.tx_info.unbox(); - assert(tx_info.nonce == 0, 'gift-acc/invalid-gift-nonce'); + assert(tx_info.nonce == 0, 'escrow/invalid-gift-nonce'); let execution_hash = tx_info.transaction_hash; let signature = tx_info.signature; - assert(signature.len() == 2, 'gift-acc/invalid-signature-len'); + assert(signature.len() == 2, 'escrow/invalid-signature-len'); // Not tested let tx_version = tx_info.version; assert( check_ecdsa_signature(execution_hash, gift.gift_pubkey, *signature[0], *signature[1]) || tx_version == TX_V3_ESTIMATE || tx_version == TX_V1_ESTIMATE, - 'invalid-signature' - ); + 'escrow/invalid-signature' + ); // Not tested if gift.fee_token == STRK_ADDRESS() { - assert(tx_version == TX_V3 || tx_version == TX_V3_ESTIMATE, 'gift-acc/invalid-tx3-version'); + // Not tested + assert(tx_version == TX_V3 || tx_version == TX_V3_ESTIMATE, 'escrow/invalid-tx3-version'); let tx_fee = compute_max_fee_v3(tx_info, tx_info.tip); - assert(tx_fee <= gift.fee_amount, 'gift-acc/max-fee-too-high-v3'); + assert(tx_fee <= gift.fee_amount, 'escrow/max-fee-too-high-v3'); } else if gift.fee_token == ETH_ADDRESS() { - assert(tx_version == TX_V1 || tx_version == TX_V1_ESTIMATE, 'gift-acc/invalid-tx1-version'); - assert(tx_info.max_fee <= gift.fee_amount, 'gift-acc/max-fee-too-high-v1'); + // Not tested + assert(tx_version == TX_V1 || tx_version == TX_V1_ESTIMATE, 'escrow/invalid-tx1-version'); + assert(tx_info.max_fee <= gift.fee_amount, 'escrow/max-fee-too-high-v1'); } else { - core::panic_with_felt252('gift-acc/invalid-token'); + // Not tested + core::panic_with_felt252('escrow/invalid-token-fee'); } VALIDATED } fn __execute__(ref self: ContractState, calls: Array) -> Array> { let execution_info = get_execution_info().unbox(); - assert(execution_info.caller_address.is_zero(), 'gift-acc/only-protocol'); + assert(execution_info.caller_address.is_zero(), 'escrow/only-protocol'); let tx_version = execution_info.tx_info.unbox().version; + // Not tested assert( tx_version == TX_V3 || tx_version == TX_V1 || tx_version == TX_V3_ESTIMATE || tx_version == TX_V1_ESTIMATE, - 'gift-acc/invalid-tx-version' + 'escrow/invalid-tx-version' ); let Call { .., calldata }: @Call = calls[0]; + // Not tested let (gift, receiver): (GiftData, ContractAddress) = full_deserialize(*calldata) - .expect('gift-acc/invalid-calldata'); + .expect('escrow/invalid-calldata'); // The __validate__ function already ensures the claim is valid let library_class_hash: ClassHash = IGiftFactoryDispatcher { contract_address: gift.factory } .get_escrow_lib_class_hash(gift.escrow_class_hash); @@ -133,7 +138,7 @@ mod EscrowAccount { fn is_valid_signature(self: @ContractState, hash: felt252, signature: Array) -> felt252 { let mut signature_span = signature.span(); - let gift: GiftData = Serde::deserialize(ref signature_span).expect('gift-acc/invalid-gift'); + let gift: GiftData = Serde::deserialize(ref signature_span).expect('escrow/invalid-gift'); get_validated_lib(gift).is_valid_account_signature(gift, hash, signature_span) } @@ -148,7 +153,7 @@ mod EscrowAccount { impl GiftAccountImpl of IEscrowAccount { fn execute_action(ref self: ContractState, selector: felt252, calldata: Array) -> Span { let mut calldata_span = calldata.span(); - let gift: GiftData = Serde::deserialize(ref calldata_span).expect('gift-acc/invalid-gift'); + let gift: GiftData = Serde::deserialize(ref calldata_span).expect('escrow/invalid-gift'); let lib = get_validated_lib(gift); lib.execute_action(lib.class_hash, selector, calldata.span()) } @@ -159,7 +164,7 @@ mod EscrowAccount { fn execute_from_outside_v2( ref self: ContractState, outside_execution: OutsideExecution, mut signature: Span ) -> Array> { - let gift: GiftData = Serde::deserialize(ref signature).expect('gift-acc/invalid-gift'); + let gift: GiftData = Serde::deserialize(ref signature).expect('escrow/invalid-gift'); get_validated_lib(gift).execute_from_outside_v2(gift, outside_execution, signature) } @@ -177,7 +182,8 @@ mod EscrowAccount { fn assert_valid_claim(gift: GiftData) { let calculated_address = calculate_escrow_account_address(gift); - assert(calculated_address == get_contract_address(), 'gift-acc/invalid-escrow-address'); + // Not tested + assert(calculated_address == get_contract_address(), 'escrow/invalid-escrow-address'); } fn compute_max_fee_v3(tx_info: TxInfo, tip: u128) -> u128 { diff --git a/src/contracts/escrow_library.cairo b/src/contracts/escrow_library.cairo index 9b1bdd0..f15332c 100644 --- a/src/contracts/escrow_library.cairo +++ b/src/contracts/escrow_library.cairo @@ -78,7 +78,8 @@ mod EscrowLibrary { // This prevents creating instances of this classhash by mistake, as it's not needed. // While it is technically possible to create instances by replacing classhashes, this practice is not recommended. // This contract is intended to be used exclusively through library calls. - panic_with_felt252('instances-not-recommended') + // Not tested + panic_with_felt252('escr-lib/instance-not-recommend') } #[abi(embed_v0)] @@ -94,7 +95,8 @@ mod EscrowLibrary { let is_whitelisted = selector == selector!("claim_external") || selector == selector!("claim_dust") || selector == selector!("cancel"); - assert(is_whitelisted, 'gift/invalid-selector'); + // not tested + assert(is_whitelisted, 'escr-lib/invalid-selector'); library_call_syscall(this_class_hash, selector, args).unwrap() } @@ -109,17 +111,16 @@ mod EscrowLibrary { .get_message_hash_rev_1(get_contract_address()); assert( check_ecdsa_signature(claim_external_hash, gift.gift_pubkey, signature.r, signature.s), - 'gift/invalid-ext-signature' + 'escr-lib/invalid-ext-signature' ); self.proceed_with_claim(gift, receiver, dust_receiver); } fn cancel(ref self: ContractState, gift: GiftData) { let contract_address = get_contract_address(); - assert(get_caller_address() == gift.sender, 'gift/wrong-sender'); - + assert(get_caller_address() == gift.sender, 'escr-lib/wrong-sender'); let gift_balance = balance_of(gift.gift_token, contract_address); - assert(gift_balance > 0, 'gift/already-claimed'); + assert(gift_balance > 0, 'escr-lib/already-claimed'); if gift.gift_token == gift.fee_token { // Sender also gets the dust transfer_from_account(gift.gift_token, gift.sender, gift_balance); @@ -135,9 +136,10 @@ mod EscrowLibrary { fn claim_dust(ref self: ContractState, gift: GiftData, receiver: ContractAddress) { let contract_address = get_contract_address(); let factory_owner = IOwnableDispatcher { contract_address: gift.factory }.owner(); - assert(factory_owner == get_caller_address(), 'gift/only-factory-owner'); + assert(factory_owner == get_caller_address(), 'escr-lib/only-factory-owner'); let gift_balance = balance_of(gift.gift_token, contract_address); - assert(gift_balance < gift.gift_amount, 'gift/not-yet-claimed'); + // Not tested + assert(gift_balance < gift.gift_amount, 'escr-lib/not-yet-claimed'); if gift.gift_token == gift.fee_token { transfer_from_account(gift.gift_token, receiver, gift_balance); } else { @@ -155,7 +157,8 @@ mod EscrowLibrary { fn execute_from_outside_v2( ref self: ContractState, gift: GiftData, outside_execution: OutsideExecution, signature: Span ) -> Array> { - panic_with_felt252('not-allowed-yet') + // Not tested + panic_with_felt252('escr-lib/not-allowed-yet') } } @@ -164,10 +167,10 @@ mod EscrowLibrary { fn proceed_with_claim( ref self: ContractState, gift: GiftData, receiver: ContractAddress, dust_receiver: ContractAddress ) { - assert(receiver.is_non_zero(), 'gift/zero-receiver'); + assert(receiver.is_non_zero(), 'escr-lib/zero-receiver'); let contract_address = get_contract_address(); let gift_balance = balance_of(gift.gift_token, contract_address); - assert(gift_balance >= gift.gift_amount, 'gift/already-claimed-or-cancel'); + assert(gift_balance >= gift.gift_amount, 'escr-lib/claimed-or-cancel'); // could be optimized to 1 transfer only when the receiver is also the dust receiver, // and the fee token is the same as the gift token @@ -193,7 +196,8 @@ mod EscrowLibrary { } fn transfer_from_account(token: ContractAddress, receiver: ContractAddress, amount: u256,) { - assert(IERC20Dispatcher { contract_address: token }.transfer(receiver, amount), 'gift/transfer-failed'); + // Not tested + assert(IERC20Dispatcher { contract_address: token }.transfer(receiver, amount), 'escr-lib/transfer-failed'); } fn balance_of(token: ContractAddress, account: ContractAddress) -> u256 { diff --git a/src/contracts/gift_factory.cairo b/src/contracts/gift_factory.cairo index 65279bd..d02a329 100644 --- a/src/contracts/gift_factory.cairo +++ b/src/contracts/gift_factory.cairo @@ -147,6 +147,7 @@ mod GiftFactory { gift_pubkey: felt252 ) { self.pausable.assert_not_paused(); + // Not tested assert(fee_token == STRK_ADDRESS() || fee_token == ETH_ADDRESS(), 'gift-fac/invalid-fee-token'); if gift_token == fee_token { // This is needed so we can tell if a gift has been claimed or not just by looking at the balances @@ -163,7 +164,7 @@ mod GiftFactory { escrow_class_hash, 0, // salt serialize(@constructor_arguments).span(), false // deploy_from_zero ) - .expect('gift-fac/deploy-failed'); + .expect('gift-fac/deploy-failed'); // Not tested? self .emit( GiftCreated { @@ -181,6 +182,7 @@ mod GiftFactory { if (gift_token == fee_token) { let transfer_status = IERC20Dispatcher { contract_address: gift_token } .transfer_from(get_caller_address(), escrow_contract, gift_amount + fee_amount.into()); + // Not tested assert(transfer_status, 'gift-fac/transfer-failed'); } else { let transfer_gift_status = IERC20Dispatcher { contract_address: gift_token } @@ -188,6 +190,7 @@ mod GiftFactory { assert(transfer_gift_status, 'gift-fac/transfer-gift-failed'); let transfer_fee_status = IERC20Dispatcher { contract_address: fee_token } .transfer_from(get_caller_address(), escrow_contract, fee_amount.into()); + // Not tested assert(transfer_fee_status, 'gift-fac/transfer-fee-failed'); } } @@ -231,7 +234,7 @@ mod GiftFactory { // This should do some sanity checks // We should check that the new implementation is a valid implementation // Execute the upgrade using replace_class_syscall(...) - panic_with_felt252('downgrade-not-allowed'); + panic_with_felt252('gift-fac/downgrade-not-allowed'); } } diff --git a/src/contracts/timelock_upgrade.cairo b/src/contracts/timelock_upgrade.cairo index 9b332aa..5d4c817 100644 --- a/src/contracts/timelock_upgrade.cairo +++ b/src/contracts/timelock_upgrade.cairo @@ -128,6 +128,7 @@ pub mod TimelockUpgradeComponent { let block_timestamp = get_block_timestamp(); let calldata_hash = poseidon_hash_span(calldata.span()); assert(calldata_hash == self.calldata_hash.read(), 'upgrade/invalid-calldata'); + // Not tested assert(new_implementation.is_non_zero(), 'upgrade/no-pending-upgrade'); assert(block_timestamp >= ready_at, 'upgrade/too-early'); assert(block_timestamp < ready_at + VALID_WINDOW_PERIOD, 'upgrade/upgrade-too-late'); diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index b507805..4943094 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -14,8 +14,8 @@ describe("Escrow Account", function () { const { factory } = await setupGiftProtocol(); const { gift } = await defaultDepositTestSetup({ factory }); const escrowAddress = calculateEscrowAddress(gift); - - await expectRevertWithErrorMessage("gift-acc/only-protocol", () => +// This says validate but calls execute? + await expectRevertWithErrorMessage("escrow/only-protocol", () => deployer.execute([{ contractAddress: escrowAddress, calldata: [0x0], entrypoint: "__validate__" }]), ); }); @@ -25,7 +25,7 @@ describe("Escrow Account", function () { const { gift } = await defaultDepositTestSetup({ factory }); const escrowAddress = calculateEscrowAddress(gift); - await expectRevertWithErrorMessage("gift-acc/only-protocol", () => + await expectRevertWithErrorMessage("escrow/only-protocol", () => deployer.execute([{ contractAddress: escrowAddress, calldata: [0x0], entrypoint: "__execute__" }]), ); }); @@ -35,7 +35,7 @@ describe("Escrow Account", function () { const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory }); const receiver = randomReceiver(); - await expectRevertWithErrorMessage("gift-acc/invalid-call-to", () => + await expectRevertWithErrorMessage("escrow/invalid-call-to", () => claimInternal({ gift, receiver, @@ -52,7 +52,7 @@ describe("Escrow Account", function () { const escrowAccount = getEscrowAccount(gift, giftPrivateKey); - await expectRevertWithErrorMessage("gift-acc/invalid-call-selector", () => + await expectRevertWithErrorMessage("escrow/invalid-call-selector", () => escrowAccount.execute( [{ contractAddress: escrowAccount.address, calldata: [], entrypoint: "execute_action" }], undefined, @@ -65,7 +65,7 @@ describe("Escrow Account", function () { const { factory } = await setupGiftProtocol(); const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory }); const escrowAccount = getEscrowAccount(gift, giftPrivateKey); - await expectRevertWithErrorMessage("gift-acc/invalid-call-len", () => + await expectRevertWithErrorMessage("escrow/invalid-call-len", () => escrowAccount.execute( [ { contractAddress: escrowAccount.address, calldata: [], entrypoint: "execute_action" }, @@ -84,7 +84,7 @@ describe("Escrow Account", function () { // double claim await claimInternal({ gift, receiver, giftPrivateKey: giftPrivateKey }); - await expectRevertWithErrorMessage("gift-acc/invalid-gift-nonce", () => + await expectRevertWithErrorMessage("escrow/invalid-gift-nonce", () => claimInternal({ gift, receiver, giftPrivateKey: giftPrivateKey, details: { skipValidate: false } }), ); }); diff --git a/tests-integration/cancel.test.ts b/tests-integration/cancel.test.ts index 46afe8b..5362a79 100644 --- a/tests-integration/cancel.test.ts +++ b/tests-integration/cancel.test.ts @@ -31,7 +31,7 @@ describe("Cancel Gift", function () { // Check balance gift address address == 0 await manager.tokens.tokenBalance(escrowAddress, gift.fee_token).should.eventually.equal(0n); - await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () => + await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () => claimInternal({ gift, receiver, giftPrivateKey }), ); }); @@ -62,7 +62,7 @@ describe("Cancel Gift", function () { await manager.tokens.tokenBalance(escrowAddress, gift.gift_token).should.eventually.equal(0n); await manager.tokens.tokenBalance(escrowAddress, gift.fee_token).should.eventually.equal(0n); - await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () => + await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () => claimInternal({ gift, receiver, giftPrivateKey }), ); }); @@ -70,7 +70,7 @@ describe("Cancel Gift", function () { it(`wrong sender`, async function () { const { factory } = await setupGiftProtocol(); const { gift } = await defaultDepositTestSetup({ factory }); - await expectRevertWithErrorMessage("gift/wrong-sender", () => cancelGift({ gift, senderAccount: devnetAccount() })); + await expectRevertWithErrorMessage("escr-lib/wrong-sender", () => cancelGift({ gift, senderAccount: devnetAccount() })); }); it(`owner reclaim dust (gift_token == fee_token)`, async function () { @@ -99,7 +99,7 @@ describe("Cancel Gift", function () { await manager.tokens.tokenBalance(escrowAddress, gift.gift_token).should.eventually.equal(0n); }); - it(`gift/already-claimed (gift_token != fee_token)`, async function () { + it(`escr-lib/already-claimed (gift_token != fee_token)`, async function () { const mockERC20 = await deployMockERC20(); const { factory } = await setupGiftProtocol(); const { gift, giftPrivateKey } = await defaultDepositTestSetup({ @@ -109,6 +109,6 @@ describe("Cancel Gift", function () { const receiver = randomReceiver(); await claimInternal({ gift, receiver, giftPrivateKey }); - await expectRevertWithErrorMessage("gift/already-claimed", () => cancelGift({ gift })); + await expectRevertWithErrorMessage("escr-lib/already-claimed", () => cancelGift({ gift })); }); }); diff --git a/tests-integration/claim_external.test.ts b/tests-integration/claim_external.test.ts index b8b10ea..859082b 100644 --- a/tests-integration/claim_external.test.ts +++ b/tests-integration/claim_external.test.ts @@ -97,7 +97,7 @@ describe("Claim External", function () { const receiver = randomReceiver(); await claimExternal({ gift, receiver, giftPrivateKey }); - await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () => + await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () => claimExternal({ gift, receiver, giftPrivateKey }), ); }); @@ -106,7 +106,7 @@ describe("Claim External", function () { const { factory } = await setupGiftProtocol(); const { gift } = await defaultDepositTestSetup({ factory }); const receiver = randomReceiver(); - await expectRevertWithErrorMessage("gift/invalid-ext-signature", () => + await expectRevertWithErrorMessage("escr-lib/invalid-ext-signature", () => claimExternal({ gift: gift, receiver, giftPrivateKey: "0x1234" }), ); }); @@ -127,7 +127,7 @@ describe("Claim External", function () { // Check balance gift address address == 0 await manager.tokens.tokenBalance(escrowAddress, gift.gift_token).should.eventually.equal(0n); - await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () => + await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () => claimExternal({ gift, receiver, giftPrivateKey }), ); }); diff --git a/tests-integration/claim_internal.test.ts b/tests-integration/claim_internal.test.ts index 3c608e4..286cc61 100644 --- a/tests-integration/claim_internal.test.ts +++ b/tests-integration/claim_internal.test.ts @@ -37,7 +37,7 @@ describe("Claim Internal", function () { overrides: { feeAmount: 0n }, }); - const errorMsg = useTxV3 ? "gift-acc/max-fee-too-high-v3" : "gift-acc/max-fee-too-high-v1"; + const errorMsg = useTxV3 ? "escrow/max-fee-too-high-v3" : "escrow/max-fee-too-high-v1"; await expectRevertWithErrorMessage(errorMsg, () => claimInternal({ gift, receiver, giftPrivateKey })); }); @@ -59,7 +59,7 @@ describe("Claim Internal", function () { max_price_per_unit: num.toHexString(gasPrice), }, }; - await expectRevertWithErrorMessage("gift-acc/max-fee-too-high-v3", () => + await expectRevertWithErrorMessage("escrow/max-fee-too-high-v3", () => claimInternal({ gift, receiver, @@ -68,7 +68,7 @@ describe("Claim Internal", function () { }), ); } else { - await expectRevertWithErrorMessage("gift-acc/max-fee-too-high-v1", () => + await expectRevertWithErrorMessage("escrow/max-fee-too-high-v1", () => claimInternal({ gift, receiver, @@ -88,7 +88,7 @@ describe("Claim Internal", function () { const receiver = randomReceiver(); await claimInternal({ gift, receiver, giftPrivateKey }); - await expectRevertWithErrorMessage("gift/already-claimed-or-cancel", () => + await expectRevertWithErrorMessage("escr-lib/claimed-or-cancel", () => claimInternal({ gift, receiver, giftPrivateKey }), ); }); diff --git a/tests-integration/factory.test.ts b/tests-integration/factory.test.ts index f3900bd..0101594 100644 --- a/tests-integration/factory.test.ts +++ b/tests-integration/factory.test.ts @@ -132,7 +132,7 @@ describe("Test Core Factory Functions", function () { const { gift } = await defaultDepositTestSetup({ factory }); const dustReceiver = randomReceiver(); - await expectRevertWithErrorMessage("gift/only-factory-owner", () => + await expectRevertWithErrorMessage("escr-lib/only-factory-owner", () => claimDust({ gift, receiver: dustReceiver, factoryOwner: devnetAccount() }), ); }); diff --git a/tests-integration/upgrade.test.ts b/tests-integration/upgrade.test.ts index 6bcdd8f..537df0f 100644 --- a/tests-integration/upgrade.test.ts +++ b/tests-integration/upgrade.test.ts @@ -59,7 +59,7 @@ describe("Test Factory Upgrade", function () { await factory.propose_upgrade(oldFactoryClassHash, calldata); await manager.setTime(CURRENT_TIME + MIN_SECURITY_PERIOD + 1n); - await expectRevertWithErrorMessage("downgrade-not-allowed", () => factory.upgrade([])); + await expectRevertWithErrorMessage("gift-fac/downgrade-not-allowed", () => factory.upgrade([])); }); it("only-owner", async function () { From 302714557ac362cd0b339a56b3bff22672dbe710 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 11:08:49 +0200 Subject: [PATCH 2/5] remove not tested --- src/contracts/escrow_account.cairo | 15 ++++----------- src/contracts/escrow_library.cairo | 5 ----- src/contracts/gift_factory.cairo | 5 +---- src/contracts/timelock_upgrade.cairo | 1 - 4 files changed, 5 insertions(+), 21 deletions(-) diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 4bf867e..9138213 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -76,20 +76,19 @@ mod EscrowAccount { impl IAccountImpl of IAccount { fn __validate__(ref self: ContractState, calls: Array) -> felt252 { let execution_info = get_execution_info().unbox(); - assert(execution_info.caller_address.is_zero(), 'escrow/only-protocol'); // Not tested + assert(execution_info.caller_address.is_zero(), 'escrow/only-protocol'); assert(calls.len() == 1, 'escrow/invalid-call-len'); let Call { to, selector, calldata } = calls.at(0); assert(*to == get_contract_address(), 'escrow/invalid-call-to'); assert(*selector == selector!("claim_internal"), 'escrow/invalid-call-selector'); - let (gift, _): (GiftData, ContractAddress) = full_deserialize(*calldata) - .expect('escrow/invalid-calldata'); // Not tested + let (gift, _): (GiftData, ContractAddress) = full_deserialize(*calldata).expect('escrow/invalid-calldata'); assert_valid_claim(gift); let tx_info = execution_info.tx_info.unbox(); assert(tx_info.nonce == 0, 'escrow/invalid-gift-nonce'); let execution_hash = tx_info.transaction_hash; let signature = tx_info.signature; - assert(signature.len() == 2, 'escrow/invalid-signature-len'); // Not tested + assert(signature.len() == 2, 'escrow/invalid-signature-len'); let tx_version = tx_info.version; assert( @@ -97,18 +96,15 @@ mod EscrowAccount { || tx_version == TX_V3_ESTIMATE || tx_version == TX_V1_ESTIMATE, 'escrow/invalid-signature' - ); // Not tested + ); if gift.fee_token == STRK_ADDRESS() { - // Not tested assert(tx_version == TX_V3 || tx_version == TX_V3_ESTIMATE, 'escrow/invalid-tx3-version'); let tx_fee = compute_max_fee_v3(tx_info, tx_info.tip); assert(tx_fee <= gift.fee_amount, 'escrow/max-fee-too-high-v3'); } else if gift.fee_token == ETH_ADDRESS() { - // Not tested assert(tx_version == TX_V1 || tx_version == TX_V1_ESTIMATE, 'escrow/invalid-tx1-version'); assert(tx_info.max_fee <= gift.fee_amount, 'escrow/max-fee-too-high-v1'); } else { - // Not tested core::panic_with_felt252('escrow/invalid-token-fee'); } VALIDATED @@ -118,7 +114,6 @@ mod EscrowAccount { let execution_info = get_execution_info().unbox(); assert(execution_info.caller_address.is_zero(), 'escrow/only-protocol'); let tx_version = execution_info.tx_info.unbox().version; - // Not tested assert( tx_version == TX_V3 || tx_version == TX_V1 @@ -127,7 +122,6 @@ mod EscrowAccount { 'escrow/invalid-tx-version' ); let Call { .., calldata }: @Call = calls[0]; - // Not tested let (gift, receiver): (GiftData, ContractAddress) = full_deserialize(*calldata) .expect('escrow/invalid-calldata'); // The __validate__ function already ensures the claim is valid @@ -182,7 +176,6 @@ mod EscrowAccount { fn assert_valid_claim(gift: GiftData) { let calculated_address = calculate_escrow_account_address(gift); - // Not tested assert(calculated_address == get_contract_address(), 'escrow/invalid-escrow-address'); } diff --git a/src/contracts/escrow_library.cairo b/src/contracts/escrow_library.cairo index f15332c..1e73049 100644 --- a/src/contracts/escrow_library.cairo +++ b/src/contracts/escrow_library.cairo @@ -78,7 +78,6 @@ mod EscrowLibrary { // This prevents creating instances of this classhash by mistake, as it's not needed. // While it is technically possible to create instances by replacing classhashes, this practice is not recommended. // This contract is intended to be used exclusively through library calls. - // Not tested panic_with_felt252('escr-lib/instance-not-recommend') } @@ -95,7 +94,6 @@ mod EscrowLibrary { let is_whitelisted = selector == selector!("claim_external") || selector == selector!("claim_dust") || selector == selector!("cancel"); - // not tested assert(is_whitelisted, 'escr-lib/invalid-selector'); library_call_syscall(this_class_hash, selector, args).unwrap() } @@ -138,7 +136,6 @@ mod EscrowLibrary { let factory_owner = IOwnableDispatcher { contract_address: gift.factory }.owner(); assert(factory_owner == get_caller_address(), 'escr-lib/only-factory-owner'); let gift_balance = balance_of(gift.gift_token, contract_address); - // Not tested assert(gift_balance < gift.gift_amount, 'escr-lib/not-yet-claimed'); if gift.gift_token == gift.fee_token { transfer_from_account(gift.gift_token, receiver, gift_balance); @@ -157,7 +154,6 @@ mod EscrowLibrary { fn execute_from_outside_v2( ref self: ContractState, gift: GiftData, outside_execution: OutsideExecution, signature: Span ) -> Array> { - // Not tested panic_with_felt252('escr-lib/not-allowed-yet') } } @@ -196,7 +192,6 @@ mod EscrowLibrary { } fn transfer_from_account(token: ContractAddress, receiver: ContractAddress, amount: u256,) { - // Not tested assert(IERC20Dispatcher { contract_address: token }.transfer(receiver, amount), 'escr-lib/transfer-failed'); } diff --git a/src/contracts/gift_factory.cairo b/src/contracts/gift_factory.cairo index d02a329..2e5806e 100644 --- a/src/contracts/gift_factory.cairo +++ b/src/contracts/gift_factory.cairo @@ -147,7 +147,6 @@ mod GiftFactory { gift_pubkey: felt252 ) { self.pausable.assert_not_paused(); - // Not tested assert(fee_token == STRK_ADDRESS() || fee_token == ETH_ADDRESS(), 'gift-fac/invalid-fee-token'); if gift_token == fee_token { // This is needed so we can tell if a gift has been claimed or not just by looking at the balances @@ -164,7 +163,7 @@ mod GiftFactory { escrow_class_hash, 0, // salt serialize(@constructor_arguments).span(), false // deploy_from_zero ) - .expect('gift-fac/deploy-failed'); // Not tested? + .expect('gift-fac/deploy-failed'); self .emit( GiftCreated { @@ -182,7 +181,6 @@ mod GiftFactory { if (gift_token == fee_token) { let transfer_status = IERC20Dispatcher { contract_address: gift_token } .transfer_from(get_caller_address(), escrow_contract, gift_amount + fee_amount.into()); - // Not tested assert(transfer_status, 'gift-fac/transfer-failed'); } else { let transfer_gift_status = IERC20Dispatcher { contract_address: gift_token } @@ -190,7 +188,6 @@ mod GiftFactory { assert(transfer_gift_status, 'gift-fac/transfer-gift-failed'); let transfer_fee_status = IERC20Dispatcher { contract_address: fee_token } .transfer_from(get_caller_address(), escrow_contract, fee_amount.into()); - // Not tested assert(transfer_fee_status, 'gift-fac/transfer-fee-failed'); } } diff --git a/src/contracts/timelock_upgrade.cairo b/src/contracts/timelock_upgrade.cairo index 5d4c817..9b332aa 100644 --- a/src/contracts/timelock_upgrade.cairo +++ b/src/contracts/timelock_upgrade.cairo @@ -128,7 +128,6 @@ pub mod TimelockUpgradeComponent { let block_timestamp = get_block_timestamp(); let calldata_hash = poseidon_hash_span(calldata.span()); assert(calldata_hash == self.calldata_hash.read(), 'upgrade/invalid-calldata'); - // Not tested assert(new_implementation.is_non_zero(), 'upgrade/no-pending-upgrade'); assert(block_timestamp >= ready_at, 'upgrade/too-early'); assert(block_timestamp < ready_at + VALID_WINDOW_PERIOD, 'upgrade/upgrade-too-late'); From f25ed3dce728a758cf521494295869ae5a56d9f8 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 11:09:36 +0200 Subject: [PATCH 3/5] remvoe extra comment --- tests-integration/account.test.ts | 2 +- tests-integration/cancel.test.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index 4943094..af609e0 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -14,7 +14,7 @@ describe("Escrow Account", function () { const { factory } = await setupGiftProtocol(); const { gift } = await defaultDepositTestSetup({ factory }); const escrowAddress = calculateEscrowAddress(gift); -// This says validate but calls execute? + await expectRevertWithErrorMessage("escrow/only-protocol", () => deployer.execute([{ contractAddress: escrowAddress, calldata: [0x0], entrypoint: "__validate__" }]), ); diff --git a/tests-integration/cancel.test.ts b/tests-integration/cancel.test.ts index 5362a79..54fc26b 100644 --- a/tests-integration/cancel.test.ts +++ b/tests-integration/cancel.test.ts @@ -70,7 +70,9 @@ describe("Cancel Gift", function () { it(`wrong sender`, async function () { const { factory } = await setupGiftProtocol(); const { gift } = await defaultDepositTestSetup({ factory }); - await expectRevertWithErrorMessage("escr-lib/wrong-sender", () => cancelGift({ gift, senderAccount: devnetAccount() })); + await expectRevertWithErrorMessage("escr-lib/wrong-sender", () => + cancelGift({ gift, senderAccount: devnetAccount() }), + ); }); it(`owner reclaim dust (gift_token == fee_token)`, async function () { From 37b55d540d66d5c227c74a5b6379a69fc3fa7073 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 11:19:15 +0200 Subject: [PATCH 4/5] fixing tests --- tests-integration/claim_external.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests-integration/claim_external.test.ts b/tests-integration/claim_external.test.ts index 859082b..daf7d72 100644 --- a/tests-integration/claim_external.test.ts +++ b/tests-integration/claim_external.test.ts @@ -88,7 +88,9 @@ describe("Claim External", function () { const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory }); const receiver = "0x0"; - await expectRevertWithErrorMessage("gift/zero-receiver", () => claimExternal({ gift, receiver, giftPrivateKey })); + await expectRevertWithErrorMessage("escr-lib/zero-receiver", () => + claimExternal({ gift, receiver, giftPrivateKey }), + ); }); it(`Cannot call claim external twice`, async function () { From cc9463713753c6da18fe57badf79e441eb486d4e Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 12:55:45 +0200 Subject: [PATCH 5/5] fixing tests --- tests-integration/account.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index 9491764..89d78f0 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -37,7 +37,7 @@ describe("Escrow Account", function () { const { gift } = await defaultDepositTestSetup({ factory }); const minimalCallData = CallData.compile([buildGiftCallData(gift)]); - await expectRevertWithErrorMessage("gift/invalid-selector", () => + await expectRevertWithErrorMessage("escr-lib/invalid-selector", () => deployer.execute(executeActionOnAccount("claim_internal", calculateEscrowAddress(gift), minimalCallData)), ); });