From cc41f237d54a842c0ce385d7b79d1da4b30f19e0 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 14:12:25 +0200 Subject: [PATCH 1/9] first draft --- .github/workflows/integration-ci.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/integration-ci.yml b/.github/workflows/integration-ci.yml index ef623b9..c92d17f 100644 --- a/.github/workflows/integration-ci.yml +++ b/.github/workflows/integration-ci.yml @@ -44,3 +44,21 @@ jobs: - name: Step 3 - Check correct linting run: yarn eslint . + + gas-report: + runs-on: ubuntu-latest + steps: + - name: Check out main branch + uses: actions/checkout@v3 + + - name: Setup Scarb + uses: software-mansion/setup-scarb@v1.3.2 + + - name: Install project + run: yarn install --frozen-lockfile + + - name: Start devnet in background + run: scarb run start-devnet + + - name: Gas report + run: scarb run profile --check \ No newline at end of file From 6ba61f0f2aed6ecd4aefbc7717a65fd29f5b9658 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Fri, 28 Jun 2024 14:14:16 +0200 Subject: [PATCH 2/9] update gas report --- gas-report.txt | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/gas-report.txt b/gas-report.txt index 131cdae..d83d043 100644 --- a/gas-report.txt +++ b/gas-report.txt @@ -6,16 +6,16 @@ Summary: │ Transfer STRK (FeeToken: WEI) │ '828.000.000.320' │ 0.0033 │ 828000000000 │ 23 │ 21 │ 2 │ 1 │ 'steps' │ 4 │ 320 │ 'BLOB' │ │ Gifting WEI (FeeToken: WEI) │ '1.548.000.000.288' │ 0.0061 │ 1548000000000 │ 43 │ 37 │ 5 │ 2 │ 'steps' │ 3 │ 288 │ 'BLOB' │ │ Claiming WEI (FeeToken: WEI) │ '1.188.000.000.192' │ 0.0047 │ 1188000000000 │ 33 │ 30 │ 2 │ 2 │ 'steps' │ 3 │ 192 │ 'BLOB' │ -│ Claiming external WEI (FeeToken: WEI) │ '1.512.000.000.256' │ 0.006 │ 1512000000000 │ 42 │ 39 │ 2 │ 2 │ 'steps' │ 4 │ 256 │ 'BLOB' │ +│ Claiming external WEI (FeeToken: WEI) │ '1.620.000.000.256' │ 0.0064 │ 1620000000000 │ 45 │ 42 │ 2 │ 2 │ 'steps' │ 4 │ 256 │ 'BLOB' │ │ Gifting WEI (FeeToken: FRI) │ '2.196.000.000.480' │ 0.0087 │ 2196000000000 │ 61 │ 52 │ 7 │ 3 │ 'steps' │ 5 │ 480 │ 'BLOB' │ │ Claiming WEI (FeeToken: FRI) │ '1.188.000.000.320' │ 0 │ 1188000000000 │ 33 │ 30 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │ -│ Claiming external WEI (FeeToken: FRI) │ '1.512.000.000.256' │ 0.006 │ 1512000000000 │ 42 │ 39 │ 2 │ 2 │ 'steps' │ 4 │ 256 │ 'BLOB' │ +│ Claiming external WEI (FeeToken: FRI) │ '1.620.000.000.256' │ 0.0064 │ 1620000000000 │ 45 │ 42 │ 2 │ 2 │ 'steps' │ 4 │ 256 │ 'BLOB' │ │ Gifting FRI (FeeToken: WEI) │ '2.196.000.000.480' │ 0.0087 │ 2196000000000 │ 61 │ 52 │ 7 │ 3 │ 'steps' │ 5 │ 480 │ 'BLOB' │ │ Claiming FRI (FeeToken: WEI) │ '1.188.000.000.320' │ 0.0047 │ 1188000000000 │ 33 │ 30 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │ -│ Claiming external FRI (FeeToken: WEI) │ '1.512.000.000.320' │ 0.006 │ 1512000000000 │ 42 │ 39 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │ +│ Claiming external FRI (FeeToken: WEI) │ '1.620.000.000.320' │ 0.0064 │ 1620000000000 │ 45 │ 42 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │ │ Gifting FRI (FeeToken: FRI) │ '1.548.000.000.416' │ 0.0061 │ 1548000000000 │ 43 │ 37 │ 5 │ 2 │ 'steps' │ 4 │ 416 │ 'BLOB' │ │ Claiming FRI (FeeToken: FRI) │ '1.188.000.000.192' │ 0 │ 1188000000000 │ 33 │ 30 │ 2 │ 2 │ 'steps' │ 3 │ 192 │ 'BLOB' │ -│ Claiming external FRI (FeeToken: FRI) │ '1.512.000.000.320' │ 0.006 │ 1512000000000 │ 42 │ 39 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │ +│ Claiming external FRI (FeeToken: FRI) │ '1.620.000.000.320' │ 0.0064 │ 1620000000000 │ 45 │ 42 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │ └───────────────────────────────────────┴─────────────────────┴─────────┴────────────────┴────────────────┴─────────────────┴───────────┴──────────────┴──────────────────────────────┴───────────────┴────────┴─────────┘ Resources: ┌───────────────────────────────────────┬─────────┬───────┬───────┬────────┬──────────┬──────────┬─────────────┬───────┐ @@ -23,16 +23,16 @@ Resources: ├───────────────────────────────────────┼─────────┼───────┼───────┼────────┼──────────┼──────────┼─────────────┼───────┤ │ Transfer ETH (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 25 │ 0 │ 181 │ 8184 │ │ Transfer STRK (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 25 │ 0 │ 181 │ 8184 │ -│ Gifting WEI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 48 │ 0 │ 339 │ 14627 │ +│ Gifting WEI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 48 │ 0 │ 339 │ 14624 │ │ Claiming WEI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 47 │ 0 │ 373 │ 11715 │ -│ Claiming external WEI (FeeToken: WEI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 427 │ 15434 │ -│ Gifting WEI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 64 │ 0 │ 465 │ 20599 │ +│ Claiming external WEI (FeeToken: WEI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 477 │ 16713 │ +│ Gifting WEI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 64 │ 0 │ 465 │ 20607 │ │ Claiming WEI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 47 │ 0 │ 407 │ 11913 │ -│ Claiming external WEI (FeeToken: FRI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 427 │ 15434 │ -│ Gifting FRI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 64 │ 0 │ 465 │ 20598 │ +│ Claiming external WEI (FeeToken: FRI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 477 │ 16713 │ +│ Gifting FRI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 64 │ 0 │ 465 │ 20606 │ │ Claiming FRI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 47 │ 0 │ 373 │ 11715 │ -│ Claiming external FRI (FeeToken: WEI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 427 │ 15434 │ -│ Gifting FRI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 48 │ 0 │ 339 │ 14628 │ +│ Claiming external FRI (FeeToken: WEI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 477 │ 16713 │ +│ Gifting FRI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 48 │ 0 │ 339 │ 14625 │ │ Claiming FRI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 47 │ 0 │ 407 │ 11913 │ -│ Claiming external FRI (FeeToken: FRI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 427 │ 15434 │ +│ Claiming external FRI (FeeToken: FRI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 477 │ 16713 │ └───────────────────────────────────────┴─────────┴───────┴───────┴────────┴──────────┴──────────┴─────────────┴───────┘ From 33e26566e92a6f773f915b69382a4b67603a1be6 Mon Sep 17 00:00:00 2001 From: gaetbout Date: Mon, 1 Jul 2024 13:37:10 +0200 Subject: [PATCH 3/9] first draft --- tests/lib.cairo | 1 + tests/setup.cairo | 42 +++++++++++++++++++++++++--------------- tests/test_deposit.cairo | 27 ++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 tests/test_deposit.cairo diff --git a/tests/lib.cairo b/tests/lib.cairo index c031fa1..1e8af34 100644 --- a/tests/lib.cairo +++ b/tests/lib.cairo @@ -1,3 +1,4 @@ mod constants; mod setup; mod test_claim_hash; +mod test_deposit; diff --git a/tests/setup.cairo b/tests/setup.cairo index 492b8af..2e1528f 100644 --- a/tests/setup.cairo +++ b/tests/setup.cairo @@ -4,7 +4,10 @@ use argent_gifting::contracts::utils::{STRK_ADDRESS, ETH_ADDRESS}; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; use openzeppelin::utils::serde::SerializedAppend; -use snforge_std::{declare, ContractClassTrait, ContractClass, start_cheat_caller_address, stop_cheat_caller_address}; +use snforge_std::{ + declare, ContractClassTrait, ContractClass, cheat_caller_address_global, start_cheat_caller_address, + stop_cheat_caller_address, stop_cheat_caller_address_global +}; use starknet::ClassHash; use super::constants::{OWNER, DEPOSITOR, CLAIMER}; @@ -27,10 +30,15 @@ pub fn deploy_gifting_broken_erc20() -> GiftingSetup { // escrow contract let escrow_contract = declare("EscrowAccount").expect('Failed to declare escrow'); + // escrow lib contract + let escrow_lib_contract = declare("EscrowLibrary").expect('Failed to declare escrow lib'); + // gift factory let factory_contract = declare("GiftFactory").expect('Failed to declare factory'); let mut factory_calldata: Array = array![ - escrow_contract.class_hash.try_into().unwrap(), OWNER().try_into().unwrap() + escrow_contract.class_hash.try_into().unwrap(), + escrow_lib_contract.class_hash.try_into().unwrap(), + OWNER().try_into().unwrap() ]; let (factory_contract_address, _) = factory_contract.deploy(@factory_calldata).expect('Failed to deploy factory'); let gift_factory = IGiftFactoryDispatcher { contract_address: factory_contract_address }; @@ -59,16 +67,16 @@ pub fn deploy_gifting_normal() -> GiftingSetup { assert(mock_eth.balance_of(OWNER()) == erc20_supply, 'Failed to mint ETH'); // mock STRK contract - let mut mock_eth_calldata: Array = array![]; + let mut mock_strk_calldata: Array = array![]; let name: ByteArray = "STARK"; let symbol: ByteArray = "STRK"; - mock_eth_calldata.append_serde(name); - mock_eth_calldata.append_serde(symbol); - mock_eth_calldata.append_serde(erc20_supply); - mock_eth_calldata.append_serde(OWNER()); - mock_eth_calldata.append_serde(OWNER()); + mock_strk_calldata.append_serde(name); + mock_strk_calldata.append_serde(symbol); + mock_strk_calldata.append_serde(erc20_supply); + mock_strk_calldata.append_serde(OWNER()); + mock_strk_calldata.append_serde(OWNER()); let (mock_strk_address, _) = mock_erc20 - .deploy_at(@mock_eth_calldata, STRK_ADDRESS()) + .deploy_at(@mock_strk_calldata, STRK_ADDRESS()) .expect('Failed to deploy STRK'); let mock_strk = IERC20Dispatcher { contract_address: mock_strk_address }; assert(mock_strk.balance_of(OWNER()) == erc20_supply, 'Failed to mint STRK'); @@ -76,25 +84,27 @@ pub fn deploy_gifting_normal() -> GiftingSetup { // escrow contract let escrow_contract = declare("EscrowAccount").expect('Failed to declare escrow'); + // escrow lib contract + let escrow_lib_contract = declare("EscrowLibrary").expect('Failed to declare escrow lib'); + // gift factory let factory_contract = declare("GiftFactory").expect('Failed to declare factory'); let mut factory_calldata: Array = array![ - escrow_contract.class_hash.try_into().unwrap(), OWNER().try_into().unwrap() + escrow_contract.class_hash.try_into().unwrap(), + escrow_lib_contract.class_hash.try_into().unwrap(), + OWNER().try_into().unwrap() ]; 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_latest_escrow_class_hash() == escrow_contract.class_hash, 'Incorrect factory setup'); - start_cheat_caller_address(mock_eth_address, OWNER()); - start_cheat_caller_address(mock_strk.contract_address, OWNER()); + cheat_caller_address_global(OWNER()); mock_eth.transfer(DEPOSITOR(), 1000); mock_strk.transfer(DEPOSITOR(), 1000); - start_cheat_caller_address(mock_eth_address, DEPOSITOR()); - start_cheat_caller_address(mock_strk_address, DEPOSITOR()); + cheat_caller_address_global(DEPOSITOR()); mock_eth.approve(factory_contract_address, 1000); mock_strk.approve(factory_contract_address, 1000); - stop_cheat_caller_address(mock_eth_address); - stop_cheat_caller_address(mock_strk_address); + stop_cheat_caller_address_global(); assert(mock_eth.balance_of(DEPOSITOR()) == 1000, 'Failed to transfer ETH'); assert(mock_strk.balance_of(DEPOSITOR()) == 1000, 'Failed to transfer STRK'); diff --git a/tests/test_deposit.cairo b/tests/test_deposit.cairo new file mode 100644 index 0000000..2ea1c6c --- /dev/null +++ b/tests/test_deposit.cairo @@ -0,0 +1,27 @@ +use argent_gifting::contracts::claim_hash::{ + IStructHashRev1, StarknetDomain, MAINNET_FIRST_HADES_PERMUTATION, SEPOLIA_FIRST_HADES_PERMUTATION +}; +use argent_gifting::contracts::gift_factory::{IGiftFactory, IGiftFactoryDispatcherTrait}; +use core::poseidon::hades_permutation; +use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; +use snforge_std::{cheat_caller_address_global, cheat_chain_id_global}; +use starknet::get_tx_info; +use super::constants::DEPOSITOR; +use super::setup::{deploy_gifting_broken_erc20, GiftingSetup, deploy_gifting_normal}; + +#[test] +#[should_panic(expected: ('gift-fac/transfer-failed',))] +fn test_deposit_same_token_failing_transfer() { + let GiftingSetup { .., mock_eth, gift_factory, escrow_class_hash } = deploy_gifting_broken_erc20(); + gift_factory.deposit(escrow_class_hash, mock_eth.contract_address, 101, mock_eth.contract_address, 100, 12); +} + + +#[test] +fn test_a() { + let GiftingSetup { mock_strk, mock_eth, gift_factory, escrow_class_hash } = deploy_gifting_normal(); + cheat_caller_address_global(DEPOSITOR()); + assert(mock_eth.allowance(DEPOSITOR(), gift_factory.contract_address) == 1000, 'Failed to approve ETH'); + assert(mock_strk.allowance(DEPOSITOR(), gift_factory.contract_address) == 1000, 'Failed to approve ETH'); + gift_factory.deposit(escrow_class_hash, mock_eth.contract_address, 100, mock_strk.contract_address, 100, 42); +} From d214e1ff04c9cb9187caebe5ae1475e4dc4d7f63 Mon Sep 17 00:00:00 2001 From: Leonard Paturel Date: Mon, 1 Jul 2024 12:37:50 +0100 Subject: [PATCH 4/9] middle of fixing --- src/mocks/reentrant_erc20.cairo | 34 +++++++------- tests-integration/claim_external.test.ts | 60 ++++++++++++------------ 2 files changed, 48 insertions(+), 46 deletions(-) diff --git a/src/mocks/reentrant_erc20.cairo b/src/mocks/reentrant_erc20.cairo index e0ead61..3c1ead2 100644 --- a/src/mocks/reentrant_erc20.cairo +++ b/src/mocks/reentrant_erc20.cairo @@ -30,7 +30,7 @@ trait IMalicious { mod ReentrantERC20 { use argent_gifting::contracts::gift_data::{GiftData}; - use argent_gifting::contracts::gift_factory::{IGiftFactory, IGiftFactoryDispatcher, IGiftFactoryDispatcherTrait}; + use argent_gifting::contracts::escrow_account::{IEscrowAccount, IEscrowAccountDispatcher, IEscrowDispatcherTrait}; use argent_gifting::contracts::utils::{ETH_ADDRESS, StarknetSignature}; use openzeppelin::token::erc20::erc20::ERC20Component::InternalTrait; @@ -107,22 +107,22 @@ mod ReentrantERC20 { } fn transfer(ref self: ContractState, recipient: ContractAddress, amount: u256) -> bool { - // if (!self.has_reentered.read()) { - // self.has_reentered.write(true); - // let test_gift: TestGiftData = self.gift.read(); - // let gift = GiftData { - // factory: test_gift.factory, - // escrow_class_hash: test_gift.escrow_class_hash, - // sender: test_gift.sender, - // gift_token: test_gift.gift_token, - // gift_amount: test_gift.gift_amount, - // fee_token: test_gift.fee_token, - // fee_amount: test_gift.fee_amount, - // gift_pubkey: test_gift.gift_pubkey, - // }; - // IGiftFactoryDispatcher { contract_address: self.factory.read() } - // .claim_external(gift, self.receiver.read(), self.dust_receiver.read(), self.signature.read()); - // } + if (!self.has_reentered.read()) { + self.has_reentered.write(true); + let test_gift: TestGiftData = self.gift.read(); + let gift = GiftData { + factory: test_gift.factory, + escrow_class_hash: test_gift.escrow_class_hash, + sender: test_gift.sender, + gift_token: test_gift.gift_token, + gift_amount: test_gift.gift_amount, + fee_token: test_gift.fee_token, + fee_amount: test_gift.fee_amount, + gift_pubkey: test_gift.gift_pubkey, + }; + IEscr { contract_address: self.factory.read() } + .claim_external(gift, self.receiver.read(), self.dust_receiver.read(), self.signature.read()); + } self.erc20.transfer(recipient, amount) } diff --git a/tests-integration/claim_external.test.ts b/tests-integration/claim_external.test.ts index 271b502..ad39fde 100644 --- a/tests-integration/claim_external.test.ts +++ b/tests-integration/claim_external.test.ts @@ -1,4 +1,5 @@ import { expect } from "chai"; +import { byteArray, uint256 } from "starknet"; import { calculateEscrowAddress, cancelGift, @@ -10,6 +11,7 @@ import { manager, randomReceiver, setupGiftProtocol, + signExternalClaim, } from "../lib"; describe("Claim External", function () { @@ -135,33 +137,33 @@ describe("Claim External", function () { }); // Commented out to pass CI temporarily - // it.skip(`Not possible to gift more via reentrancy`, async function () { - // const { factory } = await setupGiftProtocol(); - // const receiver = randomReceiver(); - - // const reentrant = await manager.deployContract("ReentrantERC20", { - // unique: true, - // constructorCalldata: [ - // byteArray.byteArrayFromString("ReentrantUSDC"), - // byteArray.byteArrayFromString("RUSDC"), - // uint256.bnToUint256(100e18), - // deployer.address, - // factory.address, - // ], - // }); - // const { gift, giftPrivateKey } = await defaultDepositTestSetup({ - // factory, - // overrides: { giftTokenAddress: reentrant.address }, - // }); - - // const claimSig = await signExternalClaim({ gift, receiver, giftPrivateKey }); - - // reentrant.connect(deployer); - // const { transaction_hash } = await reentrant.set_gift_data(gift, receiver, "0x0", claimSig); - // await manager.waitForTransaction(transaction_hash); - - // await expectRevertWithErrorMessage("ERC20: insufficient balance", () => - // claimExternal({ gift, receiver, giftPrivateKey }), - // ); - // }); + it.only(`Not possible to gift more via reentrancy`, async function () { + const { factory } = await setupGiftProtocol(); + const receiver = randomReceiver(); + + const reentrant = await manager.deployContract("ReentrantERC20", { + unique: true, + constructorCalldata: [ + byteArray.byteArrayFromString("ReentrantUSDC"), + byteArray.byteArrayFromString("RUSDC"), + uint256.bnToUint256(100e18), + deployer.address, + factory.address, + ], + }); + const { gift, giftPrivateKey } = await defaultDepositTestSetup({ + factory, + overrides: { giftTokenAddress: reentrant.address }, + }); + + const claimSig = await signExternalClaim({ gift, receiver, giftPrivateKey }); + + reentrant.connect(deployer); + const { transaction_hash } = await reentrant.set_gift_data(gift, receiver, "0x0", claimSig); + await manager.waitForTransaction(transaction_hash); + + await expectRevertWithErrorMessage("ERC20: insufficient balance", () => + claimExternal({ gift, receiver, giftPrivateKey }), + ); + }); }); From 6322a580bb7f907ad7a08e057e4a485a83248f2e Mon Sep 17 00:00:00 2001 From: Leonard Paturel Date: Mon, 1 Jul 2024 13:02:46 +0100 Subject: [PATCH 5/9] Fixed reentrancy tests --- src/mocks/reentrant_erc20.cairo | 19 +++++++++++++------ tests-integration/claim_external.test.ts | 2 +- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/mocks/reentrant_erc20.cairo b/src/mocks/reentrant_erc20.cairo index 3c1ead2..3a0faeb 100644 --- a/src/mocks/reentrant_erc20.cairo +++ b/src/mocks/reentrant_erc20.cairo @@ -28,10 +28,11 @@ trait IMalicious { #[starknet::contract] mod ReentrantERC20 { - use argent_gifting::contracts::gift_data::{GiftData}; - - use argent_gifting::contracts::escrow_account::{IEscrowAccount, IEscrowAccountDispatcher, IEscrowDispatcherTrait}; - + use argent_gifting::contracts::escrow_account::{ + IEscrowAccount, IEscrowAccountDispatcher, IEscrowAccountDispatcherTrait + }; + use argent_gifting::contracts::gift_data::GiftData; + use argent_gifting::contracts::utils::calculate_escrow_account_address; use argent_gifting::contracts::utils::{ETH_ADDRESS, StarknetSignature}; use openzeppelin::token::erc20::erc20::ERC20Component::InternalTrait; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; @@ -120,8 +121,14 @@ mod ReentrantERC20 { fee_amount: test_gift.fee_amount, gift_pubkey: test_gift.gift_pubkey, }; - IEscr { contract_address: self.factory.read() } - .claim_external(gift, self.receiver.read(), self.dust_receiver.read(), self.signature.read()); + let escrow_account_address = calculate_escrow_account_address(gift); + let mut calldata: Array = array![]; + calldata.append_serde(gift); + calldata.append_serde(self.receiver.read()); + calldata.append_serde(self.dust_receiver.read()); + calldata.append_serde(self.signature.read()); + IEscrowAccountDispatcher { contract_address: escrow_account_address } + .execute_action(selector!("claim_external"), calldata); } self.erc20.transfer(recipient, amount) diff --git a/tests-integration/claim_external.test.ts b/tests-integration/claim_external.test.ts index ad39fde..e7603a0 100644 --- a/tests-integration/claim_external.test.ts +++ b/tests-integration/claim_external.test.ts @@ -137,7 +137,7 @@ describe("Claim External", function () { }); // Commented out to pass CI temporarily - it.only(`Not possible to gift more via reentrancy`, async function () { + it(`Not possible to gift more via reentrancy`, async function () { const { factory } = await setupGiftProtocol(); const receiver = randomReceiver(); From 3292286d7902219104711721eae41c792662037f Mon Sep 17 00:00:00 2001 From: Leonard Paturel Date: Mon, 1 Jul 2024 13:04:58 +0100 Subject: [PATCH 6/9] gift --> claim --- tests-integration/claim_external.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests-integration/claim_external.test.ts b/tests-integration/claim_external.test.ts index e7603a0..5959b08 100644 --- a/tests-integration/claim_external.test.ts +++ b/tests-integration/claim_external.test.ts @@ -137,7 +137,7 @@ describe("Claim External", function () { }); // Commented out to pass CI temporarily - it(`Not possible to gift more via reentrancy`, async function () { + it(`Not possible to claim more via reentrancy`, async function () { const { factory } = await setupGiftProtocol(); const receiver = randomReceiver(); From ecbc964571d34a4caf73339dd4d3bb0ae570a33c Mon Sep 17 00:00:00 2001 From: gaetbout Date: Mon, 1 Jul 2024 14:36:43 +0200 Subject: [PATCH 7/9] adding factory deposit tests --- tests/setup.cairo | 92 +++++++++++++++++++++++----------------- tests/test_deposit.cairo | 37 ++++++++-------- 2 files changed, 72 insertions(+), 57 deletions(-) diff --git a/tests/setup.cairo b/tests/setup.cairo index 2e1528f..19e187a 100644 --- a/tests/setup.cairo +++ b/tests/setup.cairo @@ -1,17 +1,16 @@ use argent_gifting::contracts::gift_factory::{IGiftFactory, IGiftFactoryDispatcher, IGiftFactoryDispatcherTrait}; use argent_gifting::contracts::utils::{STRK_ADDRESS, ETH_ADDRESS}; -use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; +use openzeppelin::token::erc20::interface::{IERC20Dispatcher, IERC20DispatcherTrait}; use openzeppelin::utils::serde::SerializedAppend; -use snforge_std::{ - declare, ContractClassTrait, ContractClass, cheat_caller_address_global, start_cheat_caller_address, - stop_cheat_caller_address, stop_cheat_caller_address_global -}; -use starknet::ClassHash; +use snforge_std::{declare, ContractClassTrait, ContractClass, start_cheat_caller_address, stop_cheat_caller_address}; +use starknet::{ClassHash, ContractAddress}; use super::constants::{OWNER, DEPOSITOR, CLAIMER}; +const ERC20_SUPPLY: u256 = 1_000_000_000_000_000_000; + pub struct GiftingSetup { pub mock_eth: IERC20Dispatcher, pub mock_strk: IERC20Dispatcher, @@ -19,13 +18,14 @@ pub struct GiftingSetup { pub escrow_class_hash: ClassHash, } +// This will return a valid ETH but a broken STRK at their respective addresses pub fn deploy_gifting_broken_erc20() -> GiftingSetup { - let broken_erc20 = declare("BrokenERC20").expect('Failed to declare broken ERC20'); - let mut broken_erc20_calldata: Array = array![]; - let (broken_erc20_address, _) = broken_erc20 - .deploy_at(@broken_erc20_calldata, ETH_ADDRESS()) - .expect('Failed to deploy broken ERC20'); - let broken_erc20 = IERC20Dispatcher { contract_address: broken_erc20_address }; + let mock_erc20 = declare("MockERC20").expect('Failed to declare ERC20'); + + // mock ETH contract + let mock_eth = deploy_erc20_at(mock_erc20, "ETHER", "ETH", ETH_ADDRESS()); + + let broken_erc20 = deploy_broken_erc20_at(STRK_ADDRESS()); // escrow contract let escrow_contract = declare("EscrowAccount").expect('Failed to declare escrow'); @@ -44,42 +44,51 @@ pub fn deploy_gifting_broken_erc20() -> GiftingSetup { let gift_factory = IGiftFactoryDispatcher { contract_address: factory_contract_address }; assert(gift_factory.get_latest_escrow_class_hash() == escrow_contract.class_hash, 'Incorrect factory setup'); - GiftingSetup { - mock_eth: broken_erc20, mock_strk: broken_erc20, gift_factory, escrow_class_hash: escrow_contract.class_hash - } + // Approving eth transfers + start_cheat_caller_address(mock_eth.contract_address, OWNER()); + mock_eth.transfer(DEPOSITOR(), 1000); + start_cheat_caller_address(mock_eth.contract_address, DEPOSITOR()); + mock_eth.approve(factory_contract_address, 1000); + stop_cheat_caller_address(mock_eth.contract_address); + + GiftingSetup { mock_eth, mock_strk: broken_erc20, gift_factory, escrow_class_hash: escrow_contract.class_hash } } -pub fn deploy_gifting_normal() -> GiftingSetup { - let erc20_supply = 1_000_000_000_000_000_000_u256; +pub fn deploy_broken_erc20_at(at: ContractAddress) -> IERC20Dispatcher { + let broken_erc20 = declare("BrokenERC20").expect('Failed to declare broken ERC20'); + let mut broken_erc20_calldata: Array = array![]; + let (broken_erc20_address, _) = broken_erc20 + .deploy_at(@broken_erc20_calldata, at) + .expect('Failed to deploy broken ERC20'); + IERC20Dispatcher { contract_address: broken_erc20_address } +} - let mock_erc20 = declare("MockERC20").expect('Failed to declare ERC20'); + +pub fn deploy_erc20_at( + mock_erc20: ContractClass, name: ByteArray, symbol: ByteArray, at: ContractAddress +) -> IERC20Dispatcher { // mock ETH contract let mut mock_eth_calldata: Array = array![]; - let name: ByteArray = "ETHER"; - let symbol: ByteArray = "ETH"; + mock_eth_calldata.append_serde(name); mock_eth_calldata.append_serde(symbol); - mock_eth_calldata.append_serde(erc20_supply); + mock_eth_calldata.append_serde(ERC20_SUPPLY); mock_eth_calldata.append_serde(OWNER()); mock_eth_calldata.append_serde(OWNER()); - let (mock_eth_address, _) = mock_erc20.deploy_at(@mock_eth_calldata, ETH_ADDRESS()).expect('Failed to deploy ETH'); - let mock_eth = IERC20Dispatcher { contract_address: mock_eth_address }; - assert(mock_eth.balance_of(OWNER()) == erc20_supply, 'Failed to mint ETH'); + let (mock_eth_address, _) = mock_erc20.deploy_at(@mock_eth_calldata, at).expect('Failed to deploy'); + IERC20Dispatcher { contract_address: mock_eth_address } +} + +pub fn deploy_gifting_normal() -> GiftingSetup { + let mock_erc20 = declare("MockERC20").expect('Failed to declare ERC20'); + + // mock ETH contract + let mock_eth = deploy_erc20_at(mock_erc20, "ETHER", "ETH", ETH_ADDRESS()); + assert(mock_eth.balance_of(OWNER()) == ERC20_SUPPLY, 'Failed to mint ETH'); // mock STRK contract - let mut mock_strk_calldata: Array = array![]; - let name: ByteArray = "STARK"; - let symbol: ByteArray = "STRK"; - mock_strk_calldata.append_serde(name); - mock_strk_calldata.append_serde(symbol); - mock_strk_calldata.append_serde(erc20_supply); - mock_strk_calldata.append_serde(OWNER()); - mock_strk_calldata.append_serde(OWNER()); - let (mock_strk_address, _) = mock_erc20 - .deploy_at(@mock_strk_calldata, STRK_ADDRESS()) - .expect('Failed to deploy STRK'); - let mock_strk = IERC20Dispatcher { contract_address: mock_strk_address }; - assert(mock_strk.balance_of(OWNER()) == erc20_supply, 'Failed to mint STRK'); + let mock_strk = deploy_erc20_at(mock_erc20, "STARK", "STRK", STRK_ADDRESS()); + assert(mock_strk.balance_of(OWNER()) == ERC20_SUPPLY, 'Failed to mint STRK'); // escrow contract let escrow_contract = declare("EscrowAccount").expect('Failed to declare escrow'); @@ -98,13 +107,16 @@ pub fn deploy_gifting_normal() -> GiftingSetup { let gift_factory = IGiftFactoryDispatcher { contract_address: factory_contract_address }; assert(gift_factory.get_latest_escrow_class_hash() == escrow_contract.class_hash, 'Incorrect factory setup'); - cheat_caller_address_global(OWNER()); + start_cheat_caller_address(mock_eth.contract_address, OWNER()); + start_cheat_caller_address(mock_strk.contract_address, OWNER()); mock_eth.transfer(DEPOSITOR(), 1000); mock_strk.transfer(DEPOSITOR(), 1000); - cheat_caller_address_global(DEPOSITOR()); + start_cheat_caller_address(mock_eth.contract_address, DEPOSITOR()); + start_cheat_caller_address(mock_strk.contract_address, DEPOSITOR()); mock_eth.approve(factory_contract_address, 1000); mock_strk.approve(factory_contract_address, 1000); - stop_cheat_caller_address_global(); + stop_cheat_caller_address(mock_eth.contract_address); + stop_cheat_caller_address(mock_strk.contract_address); assert(mock_eth.balance_of(DEPOSITOR()) == 1000, 'Failed to transfer ETH'); assert(mock_strk.balance_of(DEPOSITOR()) == 1000, 'Failed to transfer STRK'); diff --git a/tests/test_deposit.cairo b/tests/test_deposit.cairo index 2ea1c6c..1a07529 100644 --- a/tests/test_deposit.cairo +++ b/tests/test_deposit.cairo @@ -1,27 +1,30 @@ -use argent_gifting::contracts::claim_hash::{ - IStructHashRev1, StarknetDomain, MAINNET_FIRST_HADES_PERMUTATION, SEPOLIA_FIRST_HADES_PERMUTATION -}; -use argent_gifting::contracts::gift_factory::{IGiftFactory, IGiftFactoryDispatcherTrait}; -use core::poseidon::hades_permutation; -use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; -use snforge_std::{cheat_caller_address_global, cheat_chain_id_global}; -use starknet::get_tx_info; +use argent_gifting::contracts::gift_factory::{IGiftFactoryDispatcherTrait}; +use openzeppelin::token::erc20::interface::{IERC20Dispatcher, IERC20DispatcherTrait}; +use snforge_std::{start_cheat_caller_address}; use super::constants::DEPOSITOR; -use super::setup::{deploy_gifting_broken_erc20, GiftingSetup, deploy_gifting_normal}; +use super::setup::{GiftingSetup, deploy_gifting_broken_erc20}; #[test] #[should_panic(expected: ('gift-fac/transfer-failed',))] fn test_deposit_same_token_failing_transfer() { - let GiftingSetup { .., mock_eth, gift_factory, escrow_class_hash } = deploy_gifting_broken_erc20(); - gift_factory.deposit(escrow_class_hash, mock_eth.contract_address, 101, mock_eth.contract_address, 100, 12); + let GiftingSetup { .., mock_strk, gift_factory, escrow_class_hash } = deploy_gifting_broken_erc20(); + gift_factory.deposit(escrow_class_hash, mock_strk.contract_address, 101, mock_strk.contract_address, 100, 12); } +#[test] +#[should_panic(expected: ('gift-fac/transfer-gift-failed',))] +fn test_deposit_different_token_failing_gift_transfer() { + let GiftingSetup { mock_eth, mock_strk, gift_factory, escrow_class_hash } = deploy_gifting_broken_erc20(); + let broken_erc20 = mock_strk; + start_cheat_caller_address(gift_factory.contract_address, DEPOSITOR()); + gift_factory.deposit(escrow_class_hash, broken_erc20.contract_address, 100, mock_eth.contract_address, 100, 42); +} #[test] -fn test_a() { - let GiftingSetup { mock_strk, mock_eth, gift_factory, escrow_class_hash } = deploy_gifting_normal(); - cheat_caller_address_global(DEPOSITOR()); - assert(mock_eth.allowance(DEPOSITOR(), gift_factory.contract_address) == 1000, 'Failed to approve ETH'); - assert(mock_strk.allowance(DEPOSITOR(), gift_factory.contract_address) == 1000, 'Failed to approve ETH'); - gift_factory.deposit(escrow_class_hash, mock_eth.contract_address, 100, mock_strk.contract_address, 100, 42); +#[should_panic(expected: ('gift-fac/transfer-fee-failed',))] +fn test_deposit_different_token_failing_fee_transfer() { + let GiftingSetup { mock_eth, mock_strk, gift_factory, escrow_class_hash } = deploy_gifting_broken_erc20(); + let broken_erc20 = mock_strk; + start_cheat_caller_address(gift_factory.contract_address, DEPOSITOR()); + gift_factory.deposit(escrow_class_hash, mock_eth.contract_address, 100, broken_erc20.contract_address, 100, 42); } From c57036dc77569a67cbf2c762e69cc4ca24ba188d Mon Sep 17 00:00:00 2001 From: Leonard Paturel Date: Mon, 1 Jul 2024 14:45:56 +0100 Subject: [PATCH 8/9] use serialize --- src/mocks/reentrant_erc20.cairo | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/mocks/reentrant_erc20.cairo b/src/mocks/reentrant_erc20.cairo index 3a0faeb..19d748f 100644 --- a/src/mocks/reentrant_erc20.cairo +++ b/src/mocks/reentrant_erc20.cairo @@ -32,12 +32,11 @@ mod ReentrantERC20 { IEscrowAccount, IEscrowAccountDispatcher, IEscrowAccountDispatcherTrait }; use argent_gifting::contracts::gift_data::GiftData; - use argent_gifting::contracts::utils::calculate_escrow_account_address; + use argent_gifting::contracts::utils::{calculate_escrow_account_address, serialize}; use argent_gifting::contracts::utils::{ETH_ADDRESS, StarknetSignature}; use openzeppelin::token::erc20::erc20::ERC20Component::InternalTrait; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; use openzeppelin::token::erc20::{ERC20Component, ERC20HooksEmptyImpl}; - use openzeppelin::utils::serde::SerializedAppend; use starknet::{ get_caller_address, ContractAddress, get_contract_address, contract_address_const, syscalls::call_contract_syscall @@ -122,11 +121,11 @@ mod ReentrantERC20 { gift_pubkey: test_gift.gift_pubkey, }; let escrow_account_address = calculate_escrow_account_address(gift); - let mut calldata: Array = array![]; - calldata.append_serde(gift); - calldata.append_serde(self.receiver.read()); - calldata.append_serde(self.dust_receiver.read()); - calldata.append_serde(self.signature.read()); + let calldata = serialize(@( + gift, + self.receiver.read(), + self.dust_receiver.read(), + self.signature.read())); IEscrowAccountDispatcher { contract_address: escrow_account_address } .execute_action(selector!("claim_external"), calldata); } From 7fe1446c246ee4869549a9ff3033e82b4bf874fb Mon Sep 17 00:00:00 2001 From: Leonard Paturel Date: Mon, 1 Jul 2024 14:46:58 +0100 Subject: [PATCH 9/9] format --- src/mocks/reentrant_erc20.cairo | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/mocks/reentrant_erc20.cairo b/src/mocks/reentrant_erc20.cairo index 19d748f..4c98558 100644 --- a/src/mocks/reentrant_erc20.cairo +++ b/src/mocks/reentrant_erc20.cairo @@ -32,8 +32,8 @@ mod ReentrantERC20 { IEscrowAccount, IEscrowAccountDispatcher, IEscrowAccountDispatcherTrait }; use argent_gifting::contracts::gift_data::GiftData; - use argent_gifting::contracts::utils::{calculate_escrow_account_address, serialize}; use argent_gifting::contracts::utils::{ETH_ADDRESS, StarknetSignature}; + use argent_gifting::contracts::utils::{calculate_escrow_account_address, serialize}; use openzeppelin::token::erc20::erc20::ERC20Component::InternalTrait; use openzeppelin::token::erc20::interface::{IERC20, IERC20Dispatcher, IERC20DispatcherTrait}; use openzeppelin::token::erc20::{ERC20Component, ERC20HooksEmptyImpl}; @@ -121,11 +121,9 @@ mod ReentrantERC20 { gift_pubkey: test_gift.gift_pubkey, }; let escrow_account_address = calculate_escrow_account_address(gift); - let calldata = serialize(@( - gift, - self.receiver.read(), - self.dust_receiver.read(), - self.signature.read())); + let calldata = serialize( + @(gift, self.receiver.read(), self.dust_receiver.read(), self.signature.read()) + ); IEscrowAccountDispatcher { contract_address: escrow_account_address } .execute_action(selector!("claim_external"), calldata); }