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 diff --git a/src/mocks/reentrant_erc20.cairo b/src/mocks/reentrant_erc20.cairo index e0ead61..4c98558 100644 --- a/src/mocks/reentrant_erc20.cairo +++ b/src/mocks/reentrant_erc20.cairo @@ -28,15 +28,15 @@ trait IMalicious { #[starknet::contract] 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, IEscrowAccountDispatcherTrait + }; + use argent_gifting::contracts::gift_data::GiftData; 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}; - use openzeppelin::utils::serde::SerializedAppend; use starknet::{ get_caller_address, ContractAddress, get_contract_address, contract_address_const, syscalls::call_contract_syscall @@ -107,22 +107,26 @@ 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, + }; + let escrow_account_address = calculate_escrow_account_address(gift); + 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); + } self.erc20.transfer(recipient, amount) } diff --git a/tests-integration/claim_external.test.ts b/tests-integration/claim_external.test.ts index 271b502..5959b08 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(`Not possible to claim 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 }), + ); + }); }); 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..19e187a 100644 --- a/tests/setup.cairo +++ b/tests/setup.cairo @@ -1,14 +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, start_cheat_caller_address, stop_cheat_caller_address}; -use starknet::ClassHash; +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, @@ -16,85 +18,105 @@ 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'); + // 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'); - 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_eth_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()); - let (mock_strk_address, _) = mock_erc20 - .deploy_at(@mock_eth_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'); + // 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_eth.contract_address, OWNER()); start_cheat_caller_address(mock_strk.contract_address, 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()); + 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(mock_eth_address); - stop_cheat_caller_address(mock_strk_address); + 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 new file mode 100644 index 0000000..1a07529 --- /dev/null +++ b/tests/test_deposit.cairo @@ -0,0 +1,30 @@ +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::{GiftingSetup, deploy_gifting_broken_erc20}; + +#[test] +#[should_panic(expected: ('gift-fac/transfer-failed',))] +fn test_deposit_same_token_failing_transfer() { + 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] +#[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); +}