From 1899509152b5e836a91ed6b9fff052b9c971a7b4 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 11:07:32 +0200 Subject: [PATCH 01/15] save --- src/contracts/escrow_account.cairo | 1 + 1 file changed, 1 insertion(+) diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 4bf867e..1b8dc50 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -72,6 +72,7 @@ mod EscrowAccount { #[constructor] fn constructor(ref self: ContractState, args: AccountConstructorArguments) {} + #[abi(embed_v0)] impl IAccountImpl of IAccount { fn __validate__(ref self: ContractState, calls: Array) -> felt252 { From 8317ffd2ec92908cd5bd4003bab00135cb0271ce Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 11:23:06 +0200 Subject: [PATCH 02/15] revert --- src/contracts/escrow_account.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 1b8dc50..4bf867e 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -72,7 +72,6 @@ mod EscrowAccount { #[constructor] fn constructor(ref self: ContractState, args: AccountConstructorArguments) {} - #[abi(embed_v0)] impl IAccountImpl of IAccount { fn __validate__(ref self: ContractState, calls: Array) -> felt252 { From 5f2a576efd356fbe6e14454d1790b9f373cdc0ca Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 12:14:15 +0200 Subject: [PATCH 03/15] fixing tests --- src/contracts/escrow_account.cairo | 6 ++---- tests-integration/account.test.ts | 2 +- tests-integration/cancel.test.ts | 4 +++- tests-integration/claim_external.test.ts | 4 +++- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 4bf867e..3ba341c 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -76,13 +76,12 @@ 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(); @@ -127,7 +126,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 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 () { 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 6d8b7c8eea67a275d0670062cd56159af585416c Mon Sep 17 00:00:00 2001 From: gaetbout Date: Thu, 27 Jun 2024 12:16:50 +0200 Subject: [PATCH 04/15] revert comment --- src/contracts/escrow_account.cairo | 1 + tests-integration/account.test.ts | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 3ba341c..42fa19f 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -81,6 +81,7 @@ mod EscrowAccount { 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'); + // Not tested let (gift, _): (GiftData, ContractAddress) = full_deserialize(*calldata).expect('escrow/invalid-calldata'); assert_valid_claim(gift); diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index af609e0..0ab39c4 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -25,7 +25,7 @@ describe("Escrow Account", function () { const { gift } = await defaultDepositTestSetup({ factory }); const escrowAddress = calculateEscrowAddress(gift); - await expectRevertWithErrorMessage("escrow/only-protocol", () => + await expectRevertWithErrorMessage("gift-acc/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("escrow/invalid-call-to", () => + await expectRevertWithErrorMessage("gift-acc/invalid-call-to", () => claimInternal({ gift, receiver, @@ -52,7 +52,7 @@ describe("Escrow Account", function () { const escrowAccount = getEscrowAccount(gift, giftPrivateKey); - await expectRevertWithErrorMessage("escrow/invalid-call-selector", () => + await expectRevertWithErrorMessage("gift-acc/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("escrow/invalid-call-len", () => + await expectRevertWithErrorMessage("gift-acc/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("escrow/invalid-gift-nonce", () => + await expectRevertWithErrorMessage("gift-acc/invalid-gift-nonce", () => claimInternal({ gift, receiver, giftPrivateKey: giftPrivateKey, details: { skipValidate: false } }), ); }); From 7a74bfe3a11170f6f71b246bb1febea43a14d4ad Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 09:22:16 +0200 Subject: [PATCH 05/15] fix tests --- tests-integration/account.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index 0ab39c4..af609e0 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -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 } }), ); }); From 277b3d291f67a644bc2737e27986e6d5806b06d0 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 10:09:54 +0200 Subject: [PATCH 06/15] adding all non tested mesgs --- src/contracts/escrow_account.cairo | 6 ++++++ src/contracts/escrow_library.cairo | 3 +++ src/contracts/gift_factory.cairo | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 006ee9b..928b67b 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -89,9 +89,11 @@ mod EscrowAccount { assert(tx_info.nonce == 0, 'escrow/invalid-gift-nonce'); let execution_hash = tx_info.transaction_hash; let signature = tx_info.signature; + // Not tested assert(signature.len() == 2, 'escrow/invalid-signature-len'); let tx_version = tx_info.version; + // Not tested assert( check_ecdsa_signature(execution_hash, gift.gift_pubkey, *signature[0], *signature[1]) || tx_version == TX_V3_ESTIMATE @@ -106,6 +108,7 @@ mod EscrowAccount { 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 @@ -115,6 +118,7 @@ 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 @@ -123,6 +127,7 @@ 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 @@ -177,6 +182,7 @@ 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 a1e6945..cab9a86 100644 --- a/src/contracts/escrow_library.cairo +++ b/src/contracts/escrow_library.cairo @@ -78,6 +78,7 @@ 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') } @@ -138,6 +139,7 @@ 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); @@ -193,6 +195,7 @@ 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 2e5806e..3b62353 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 @@ -159,6 +160,7 @@ mod GiftFactory { let constructor_arguments = AccountConstructorArguments { sender, gift_token, gift_amount, fee_token, fee_amount, gift_pubkey }; + // Not tested let (escrow_contract, _) = deploy_syscall( escrow_class_hash, 0, // salt serialize(@constructor_arguments).span(), false // deploy_from_zero @@ -181,6 +183,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 +191,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'); } } From ec2e32f64f3919635bf04c41b3c6efe01845086e Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 12:36:35 +0200 Subject: [PATCH 07/15] adding some tests lib acc --- src/contracts/escrow_library.cairo | 2 -- tests-integration/account.test.ts | 7 +++++++ tests-integration/factory.test.ts | 8 ++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/contracts/escrow_library.cairo b/src/contracts/escrow_library.cairo index cab9a86..137fc6e 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') } @@ -139,7 +138,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); diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index 89d78f0..eb98967 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -8,6 +8,7 @@ import { executeActionOnAccount, expectRevertWithErrorMessage, getEscrowAccount, + manager, randomReceiver, setupGiftProtocol, } from "../lib"; @@ -100,4 +101,10 @@ describe("Escrow Account", function () { claimInternal({ gift, receiver, giftPrivateKey: giftPrivateKey, details: { skipValidate: false } }), ); }); + + it(`Shouldn't be possible to instantiate the library account`, async function () { + const classHash = await manager.declareLocalContract("EscrowLibrary"); + + await expectRevertWithErrorMessage("escr-lib/instance-not-recommend", () => deployer.deployContract({ classHash })); + }); }); diff --git a/tests-integration/factory.test.ts b/tests-integration/factory.test.ts index 0101594..9b8c66e 100644 --- a/tests-integration/factory.test.ts +++ b/tests-integration/factory.test.ts @@ -63,6 +63,14 @@ describe("Test Core Factory Functions", function () { await manager.tokens.tokenBalance(escrowAddress, gift.gift_token).should.eventually.equal(0n); await manager.tokens.tokenBalance(dustReceiver, gift.gift_token).should.eventually.equal(dustBalance); }); + + it(`Shouldn't be possible to claim_dust for an unclaimed gift: ${useTxV3}`, async function () { + const { factory } = await setupGiftProtocol(); + const { gift } = await defaultDepositTestSetup({ factory, useTxV3 }); + const dustReceiver = randomReceiver(); + + await expectRevertWithErrorMessage("escr-lib/not-yet-claimed", () => claimDust({ gift, receiver: dustReceiver })); + }); } it(`Pausable`, async function () { From bea8081d7f77a7e205689a314498ae216d2843b8 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 12:46:12 +0200 Subject: [PATCH 08/15] adding factory test --- src/contracts/gift_factory.cairo | 2 -- tests-integration/deposit.test.ts | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/contracts/gift_factory.cairo b/src/contracts/gift_factory.cairo index 3b62353..522b9e5 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 @@ -160,7 +159,6 @@ mod GiftFactory { let constructor_arguments = AccountConstructorArguments { sender, gift_token, gift_amount, fee_token, fee_amount, gift_pubkey }; - // Not tested let (escrow_contract, _) = deploy_syscall( escrow_class_hash, 0, // salt serialize(@constructor_arguments).span(), false // deploy_from_zero diff --git a/tests-integration/deposit.test.ts b/tests-integration/deposit.test.ts index fccff6c..6559f14 100644 --- a/tests-integration/deposit.test.ts +++ b/tests-integration/deposit.test.ts @@ -98,6 +98,20 @@ describe("Deposit", function () { return txReceipt; }); }); + + it(`Fee not ETH nor STRK`, async function () { + const { factory } = await setupGiftProtocol(); + const mockERC20 = await deployMockERC20(); + + await expectRevertWithErrorMessage("gift-fac/invalid-fee-token", async () => { + const { txReceipt } = await defaultDepositTestSetup({ + factory, + useTxV3, + overrides: { feeTokenAddress: mockERC20.address }, + }); + return txReceipt; + }); + }); } it("Deposit fails class hash passed != class hash in factory storage", async function () { From 121b8ae60640c1b03a2021d13b30987c28eaa4af Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 13:10:28 +0200 Subject: [PATCH 09/15] wrong address --- lib/claim.ts | 4 ++-- src/contracts/escrow_account.cairo | 2 -- tests-integration/claim_internal.test.ts | 23 +++++++++++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/claim.ts b/lib/claim.ts index 9b85e86..ccdb2f8 100644 --- a/lib/claim.ts +++ b/lib/claim.ts @@ -148,10 +148,10 @@ export async function claimInternal(args: { gift: Gift; receiver: string; giftPrivateKey: string; - overrides?: { EscrowAccountAddress?: string; callToAddress?: string }; + overrides?: { escrowAccountAddress?: string; callToAddress?: string }; details?: UniversalDetails; }): Promise { - const escrowAddress = args.overrides?.EscrowAccountAddress || calculateEscrowAddress(args.gift); + const escrowAddress = args.overrides?.escrowAccountAddress || calculateEscrowAddress(args.gift); const escrowAccount = getEscrowAccount(args.gift, args.giftPrivateKey, escrowAddress); const response = await escrowAccount.execute( [ diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 928b67b..e401cd9 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -108,7 +108,6 @@ mod EscrowAccount { 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 @@ -182,7 +181,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/tests-integration/claim_internal.test.ts b/tests-integration/claim_internal.test.ts index 286cc61..53707f0 100644 --- a/tests-integration/claim_internal.test.ts +++ b/tests-integration/claim_internal.test.ts @@ -3,10 +3,13 @@ import { num } from "starknet"; import { ETH_GIFT_MAX_FEE, STRK_GIFT_MAX_FEE, + buildGiftCallData, calculateEscrowAddress, claimInternal, defaultDepositTestSetup, + deployMockERC20, expectRevertWithErrorMessage, + getEscrowAccount, manager, randomReceiver, setupGiftProtocol, @@ -27,6 +30,26 @@ describe("Claim Internal", function () { await manager.tokens.tokenBalance(receiver, gift.gift_token).should.eventually.equal(gift.gift_amount); }); + it(`fee token not ETH nor STRK using txV3: ${useTxV3}`, async function () { + const { factory } = await setupGiftProtocol(); + const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory, useTxV3 }); + const receiver = randomReceiver(); + const escrowAddress = calculateEscrowAddress(gift); + + const escrowAccount = getEscrowAccount(gift, giftPrivateKey, escrowAddress); + const mockERC20 = await deployMockERC20(); + gift.fee_token = mockERC20.address; + await expectRevertWithErrorMessage("escrow/invalid-escrow-address", () => + escrowAccount.execute([ + { + contractAddress: escrowAddress, + calldata: [buildGiftCallData(gift), receiver], + entrypoint: "claim_internal", + }, + ]), + ); + }); + it(`Can't claim if no fee amount deposited (fee token == gift token) using txV3: ${useTxV3}`, async function () { const { factory } = await setupGiftProtocol(); const receiver = randomReceiver(); From 9034a3e1d43148206ea75b533e84ec04933b9db8 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 13:13:57 +0200 Subject: [PATCH 10/15] invalid calldata --- src/contracts/escrow_account.cairo | 1 - tests-integration/claim_internal.test.ts | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index e401cd9..e18ea09 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -81,7 +81,6 @@ mod EscrowAccount { 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'); - // Not tested let (gift, _): (GiftData, ContractAddress) = full_deserialize(*calldata).expect('escrow/invalid-calldata'); assert_valid_claim(gift); diff --git a/tests-integration/claim_internal.test.ts b/tests-integration/claim_internal.test.ts index 53707f0..c863b10 100644 --- a/tests-integration/claim_internal.test.ts +++ b/tests-integration/claim_internal.test.ts @@ -50,6 +50,26 @@ describe("Claim Internal", function () { ); }); + it(`Invalid calldata using txV3: ${useTxV3}`, async function () { + const { factory } = await setupGiftProtocol(); + const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory, useTxV3 }); + const receiver = randomReceiver(); + const escrowAddress = calculateEscrowAddress(gift); + + const escrowAccount = getEscrowAccount(gift, giftPrivateKey, escrowAddress); + const mockERC20 = await deployMockERC20(); + gift.fee_token = mockERC20.address; + await expectRevertWithErrorMessage("escrow/invalid-calldata", () => + escrowAccount.execute([ + { + contractAddress: escrowAddress, + calldata: [buildGiftCallData(gift), receiver, 1], + entrypoint: "claim_internal", + }, + ]), + ); + }); + it(`Can't claim if no fee amount deposited (fee token == gift token) using txV3: ${useTxV3}`, async function () { const { factory } = await setupGiftProtocol(); const receiver = randomReceiver(); From f3e5f41d3456f6022821fd209eeb1b2236a97207 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 13:14:30 +0200 Subject: [PATCH 11/15] invalid calldata --- tests-integration/claim_internal.test.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests-integration/claim_internal.test.ts b/tests-integration/claim_internal.test.ts index c863b10..68a3894 100644 --- a/tests-integration/claim_internal.test.ts +++ b/tests-integration/claim_internal.test.ts @@ -54,15 +54,12 @@ describe("Claim Internal", function () { const { factory } = await setupGiftProtocol(); const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory, useTxV3 }); const receiver = randomReceiver(); - const escrowAddress = calculateEscrowAddress(gift); - const escrowAccount = getEscrowAccount(gift, giftPrivateKey, escrowAddress); - const mockERC20 = await deployMockERC20(); - gift.fee_token = mockERC20.address; + const escrowAccount = getEscrowAccount(gift, giftPrivateKey); await expectRevertWithErrorMessage("escrow/invalid-calldata", () => escrowAccount.execute([ { - contractAddress: escrowAddress, + contractAddress: escrowAccount.address, calldata: [buildGiftCallData(gift), receiver, 1], entrypoint: "claim_internal", }, From 4d5b22f92983db5dff49df06e9a320211eee35d8 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 13:38:53 +0200 Subject: [PATCH 12/15] the long dong --- lib/signers/legacy.ts | 6 ++++++ src/contracts/escrow_account.cairo | 3 --- tests-integration/account.test.ts | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/signers/legacy.ts b/lib/signers/legacy.ts index d1347f7..39319f4 100644 --- a/lib/signers/legacy.ts +++ b/lib/signers/legacy.ts @@ -46,3 +46,9 @@ export class LegacyStarknetKeyPair extends LegacyKeyPair { return [r.toString(), s.toString()]; } } + +export class LongSigner extends LegacyStarknetKeyPair { + public async signRaw(messageHash: string): Promise { + return ["", ...(await super.signRaw(messageHash))]; + } +} diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index e18ea09..273b278 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -88,7 +88,6 @@ mod EscrowAccount { assert(tx_info.nonce == 0, 'escrow/invalid-gift-nonce'); let execution_hash = tx_info.transaction_hash; let signature = tx_info.signature; - // Not tested assert(signature.len() == 2, 'escrow/invalid-signature-len'); let tx_version = tx_info.version; @@ -116,7 +115,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 @@ -125,7 +123,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 diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index eb98967..3c897d0 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -1,5 +1,6 @@ import { CallData } from "starknet"; import { + LongSigner, buildGiftCallData, calculateEscrowAddress, claimInternal, @@ -102,6 +103,24 @@ describe("Escrow Account", function () { ); }); + it(`Long signature shouldn't be accepted`, async function () { + const { factory } = await setupGiftProtocol(); + const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory }); + const receiver = randomReceiver(); + + const escrowAccount = getEscrowAccount(gift, giftPrivateKey); + escrowAccount.signer = new LongSigner(); + await expectRevertWithErrorMessage("escrow/invalid-signature-len", () => + escrowAccount.execute([ + { + contractAddress: escrowAccount.address, + calldata: [buildGiftCallData(gift), receiver], + entrypoint: "claim_internal", + }, + ]), + ); + }); + it(`Shouldn't be possible to instantiate the library account`, async function () { const classHash = await manager.declareLocalContract("EscrowLibrary"); From b44de8d6900487154945a94f1d5e246a52c5c345 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 13:50:31 +0200 Subject: [PATCH 13/15] wrong signature --- lib/signers/legacy.ts | 6 ++++++ src/contracts/escrow_account.cairo | 1 - tests-integration/account.test.ts | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/signers/legacy.ts b/lib/signers/legacy.ts index 39319f4..b182037 100644 --- a/lib/signers/legacy.ts +++ b/lib/signers/legacy.ts @@ -52,3 +52,9 @@ export class LongSigner extends LegacyStarknetKeyPair { return ["", ...(await super.signRaw(messageHash))]; } } + +export class WrongSigner extends LegacyStarknetKeyPair { + public async signRaw(messageHash: string): Promise { + return ["", ""]; + } +} diff --git a/src/contracts/escrow_account.cairo b/src/contracts/escrow_account.cairo index 273b278..9138213 100644 --- a/src/contracts/escrow_account.cairo +++ b/src/contracts/escrow_account.cairo @@ -91,7 +91,6 @@ mod EscrowAccount { assert(signature.len() == 2, 'escrow/invalid-signature-len'); let tx_version = tx_info.version; - // Not tested assert( check_ecdsa_signature(execution_hash, gift.gift_pubkey, *signature[0], *signature[1]) || tx_version == TX_V3_ESTIMATE diff --git a/tests-integration/account.test.ts b/tests-integration/account.test.ts index 3c897d0..10e5c89 100644 --- a/tests-integration/account.test.ts +++ b/tests-integration/account.test.ts @@ -1,6 +1,7 @@ import { CallData } from "starknet"; import { LongSigner, + WrongSigner, buildGiftCallData, calculateEscrowAddress, claimInternal, @@ -121,6 +122,24 @@ describe("Escrow Account", function () { ); }); + it(`Wrong signature shouldn't be accepted`, async function () { + const { factory } = await setupGiftProtocol(); + const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory }); + const receiver = randomReceiver(); + + const escrowAccount = getEscrowAccount(gift, giftPrivateKey); + escrowAccount.signer = new WrongSigner(); + await expectRevertWithErrorMessage("escrow/invalid-signature", () => + escrowAccount.execute([ + { + contractAddress: escrowAccount.address, + calldata: [buildGiftCallData(gift), receiver], + entrypoint: "claim_internal", + }, + ]), + ); + }); + it(`Shouldn't be possible to instantiate the library account`, async function () { const classHash = await manager.declareLocalContract("EscrowLibrary"); From 7c3384c1403b797b389489e3578fcdfee82b16cd Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 13:51:55 +0200 Subject: [PATCH 14/15] remove comments --- src/contracts/escrow_library.cairo | 1 - src/contracts/gift_factory.cairo | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/contracts/escrow_library.cairo b/src/contracts/escrow_library.cairo index 137fc6e..a1e6945 100644 --- a/src/contracts/escrow_library.cairo +++ b/src/contracts/escrow_library.cairo @@ -193,7 +193,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 522b9e5..2e5806e 100644 --- a/src/contracts/gift_factory.cairo +++ b/src/contracts/gift_factory.cairo @@ -181,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 } @@ -189,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'); } } From a07a009fb9c7a07143c0ecc2613d4f65dae84f41 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Mon, 1 Jul 2024 11:00:34 +0200 Subject: [PATCH 15/15] fixing SGC remarks --- lib/signers/legacy.ts | 2 +- tests-integration/claim_internal.test.ts | 16 ++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/signers/legacy.ts b/lib/signers/legacy.ts index b182037..0606b4b 100644 --- a/lib/signers/legacy.ts +++ b/lib/signers/legacy.ts @@ -55,6 +55,6 @@ export class LongSigner extends LegacyStarknetKeyPair { export class WrongSigner extends LegacyStarknetKeyPair { public async signRaw(messageHash: string): Promise { - return ["", ""]; + return ["0x1", "0x1"]; } } diff --git a/tests-integration/claim_internal.test.ts b/tests-integration/claim_internal.test.ts index 68a3894..e1c3112 100644 --- a/tests-integration/claim_internal.test.ts +++ b/tests-integration/claim_internal.test.ts @@ -7,7 +7,6 @@ import { calculateEscrowAddress, claimInternal, defaultDepositTestSetup, - deployMockERC20, expectRevertWithErrorMessage, getEscrowAccount, manager, @@ -30,23 +29,16 @@ describe("Claim Internal", function () { await manager.tokens.tokenBalance(receiver, gift.gift_token).should.eventually.equal(gift.gift_amount); }); - it(`fee token not ETH nor STRK using txV3: ${useTxV3}`, async function () { + it(`Invalid gift data txV3: ${useTxV3}`, async function () { const { factory } = await setupGiftProtocol(); const { gift, giftPrivateKey } = await defaultDepositTestSetup({ factory, useTxV3 }); const receiver = randomReceiver(); const escrowAddress = calculateEscrowAddress(gift); - const escrowAccount = getEscrowAccount(gift, giftPrivateKey, escrowAddress); - const mockERC20 = await deployMockERC20(); - gift.fee_token = mockERC20.address; + const escrowAccountAddress = getEscrowAccount(gift, giftPrivateKey, escrowAddress).address; + gift.fee_amount = 42n; await expectRevertWithErrorMessage("escrow/invalid-escrow-address", () => - escrowAccount.execute([ - { - contractAddress: escrowAddress, - calldata: [buildGiftCallData(gift), receiver], - entrypoint: "claim_internal", - }, - ]), + claimInternal({ gift, receiver, giftPrivateKey, overrides: { escrowAccountAddress } }), ); });