From 513f5436c08ef301f3b6a4f8f4f4cc687680db4b Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Tue, 19 Mar 2024 12:24:20 -0300 Subject: [PATCH 01/11] feat: claim payment batch --- contracts/starknet/src/escrow.cairo | 56 +++- contracts/starknet/src/lib.cairo | 3 +- .../src/tests/test_escrow_allowance.cairo | 23 -- .../src/tests/test_escrow_claim.cairo | 248 ++++++++++++++++++ 4 files changed, 297 insertions(+), 33 deletions(-) create mode 100644 contracts/starknet/src/tests/test_escrow_claim.cairo diff --git a/contracts/starknet/src/escrow.cairo b/contracts/starknet/src/escrow.cairo index d59af39f..5e37acab 100644 --- a/contracts/starknet/src/escrow.cairo +++ b/contracts/starknet/src/escrow.cairo @@ -243,18 +243,56 @@ mod Escrow { self.pausable.assert_not_paused(); let eth_transfer_contract_felt: felt252 = self.eth_transfer_contract.read().into(); assert(from_address == eth_transfer_contract_felt, 'Only PAYMENT_REGISTRY_CONTRACT'); - assert(self.orders_pending.read(order_id), 'Order claimed or nonexistent'); - let order = self.orders.read(order_id); - assert(order.recipient_address == recipient_address, 'recipient_address not match L1'); - assert(order.amount == amount, 'amount not match L1'); + _claim_payment(ref self, from_address, order_id, recipient_address, amount); + } - self.orders_pending.write(order_id, false); - let payment_amount = order.amount + order.fee; + #[l1_handler] + fn claim_payment_batch( + ref self: ContractState, + from_address: felt252, + order_ids: Array, + recipient_addresses: Array, + amounts: Array + ) { + let eth_transfer_contract_felt: felt252 = self.eth_transfer_contract.read().into(); + assert(from_address == eth_transfer_contract_felt, 'Only YAB_TRANSFER_CONTRACT'); + + assert(order_ids.len() == recipient_addresses.len(), 'Different lengths'); + assert(order_ids.len() == amounts.len(), 'Different lengths'); + + let mut idx = 0; - IERC20Dispatcher { contract_address: self.native_token_eth_starknet.read() } - .transfer(self.mm_starknet_wallet.read(), payment_amount); + loop { + if idx >= order_ids.len() { + break; + } - self.emit(ClaimPayment { order_id, address: self.mm_starknet_wallet.read(), amount }); + _claim_payment(ref self, from_address, *order_ids.at(idx), *recipient_addresses.at(idx), *amounts.at(idx)); + + idx += 1; + }; } + + fn _claim_payment( + ref self: ContractState, + from_address: felt252, + order_id: u256, + recipient_address: EthAddress, + amount: u256 + ) { + assert(self.orders_pending.read(order_id), 'Order withdrew or nonexistent'); + + let order = self.orders.read(order_id); + assert(order.recipient_address == recipient_address, 'recipient_address not match L1'); + assert(order.amount == amount, 'amount not match L1'); + + self.orders_pending.write(order_id, false); + let payment_amount = order.amount + order.fee; + + IERC20Dispatcher { contract_address: self.native_token_eth_starknet.read() } + .transfer(self.mm_starknet_wallet.read(), payment_amount); + + self.emit(ClaimPayment { order_id, address: self.mm_starknet_wallet.read(), amount }); + } } diff --git a/contracts/starknet/src/lib.cairo b/contracts/starknet/src/lib.cairo index 87cce74f..2e0fd740 100644 --- a/contracts/starknet/src/lib.cairo +++ b/contracts/starknet/src/lib.cairo @@ -10,7 +10,6 @@ mod mocks { mod mock_EVMFactsRegistry; mod mock_Escrow_changed_functions; mod mock_pausableEscrow; - } #[cfg(test)] @@ -19,6 +18,8 @@ mod tests { mod test_escrow_pause; mod test_escrow_upgrade; mod test_escrow_ownable; + mod test_escrow_claim; + mod utils { mod constants; } diff --git a/contracts/starknet/src/tests/test_escrow_allowance.cairo b/contracts/starknet/src/tests/test_escrow_allowance.cairo index f4febc7e..7b9e0b37 100644 --- a/contracts/starknet/src/tests/test_escrow_allowance.cairo +++ b/contracts/starknet/src/tests/test_escrow_allowance.cairo @@ -172,27 +172,4 @@ mod Escrow { let order_id = escrow.set_order(order); stop_prank(CheatTarget::One(escrow.contract_address)); } - - #[test] - fn test_fail_random_eth_user_calls_l1_handler() { - let (escrow, _) = setup(); - let data: Array = array![1, MM_ETHEREUM().into(), 3, 4]; - let mut payload_buffer: Array = ArrayTrait::new(); - data.serialize(ref payload_buffer); - let mut l1_handler = L1HandlerTrait::new( - contract_address: escrow.contract_address, - function_name: 'claim_payment', - ); - l1_handler.from_address = ETH_USER().into(); - - l1_handler.payload = payload_buffer.span(); - - // same as "Should Panic" but for the L1 handler function - match l1_handler.execute() { - Result::Ok(_) => panic_with_felt252('shouldve panicked'), - Result::Err(RevertedTransaction) => { - assert(*RevertedTransaction.panic_data.at(0) == 'Only PAYMENT_REGISTRY_CONTRACT', *RevertedTransaction.panic_data.at(0)); - } - } - } } diff --git a/contracts/starknet/src/tests/test_escrow_claim.cairo b/contracts/starknet/src/tests/test_escrow_claim.cairo new file mode 100644 index 00000000..99c9548a --- /dev/null +++ b/contracts/starknet/src/tests/test_escrow_claim.cairo @@ -0,0 +1,248 @@ +mod Escrow { + use core::to_byte_array::FormatAsByteArray; + use core::serde::Serde; + use core::traits::Into; + use starknet::{EthAddress, ContractAddress}; + use integer::BoundedInt; + + use snforge_std::{declare, ContractClassTrait, L1Handler, L1HandlerTrait}; + use snforge_std::{CheatTarget, start_prank, stop_prank, start_warp, stop_warp}; + + use yab::mocks::mock_Escrow_changed_functions::{IEscrow_mock_changed_functionsDispatcher, IEscrow_mock_changed_functionsDispatcherTrait}; + use yab::mocks::mock_pausableEscrow::{IEscrow_mockPausableDispatcher, IEscrow_mockPausableDispatcherTrait}; + use yab::interfaces::IERC20::{IERC20Dispatcher, IERC20DispatcherTrait}; + use yab::escrow::{IEscrowDispatcher, IEscrowDispatcherTrait, Order}; + use yab::interfaces::IEVMFactsRegistry::{ + IEVMFactsRegistryDispatcher, IEVMFactsRegistryDispatcherTrait + }; + + use yab::tests::utils::{ + constants::EscrowConstants::{ + USER, OWNER, MM_STARKNET, MM_ETHEREUM, ETH_TRANSFER_CONTRACT, ETH_USER + }, + }; + + use openzeppelin::{ + upgrades::{ + UpgradeableComponent, + interface::{IUpgradeable, IUpgradeableDispatcher, IUpgradeableDispatcherTrait} + }, + }; + + fn setup() -> (IEscrowDispatcher, IERC20Dispatcher) { + setup_general(BoundedInt::max(), BoundedInt::max()) + } + + // fn setup_approved(approved: u256) -> (IEscrowDispatcher, IERC20Dispatcher){ + // setup_general(BoundedInt::max(), approved) + // } + + // fn setup_balance(balance: u256) -> (IEscrowDispatcher, IERC20Dispatcher){ + // setup_general(balance, BoundedInt::max()) + // } + + fn setup_general(balance: u256, approved: u256) -> (IEscrowDispatcher, IERC20Dispatcher){ + let eth_token = deploy_erc20('ETH', '$ETH', BoundedInt::max(), OWNER()); + let escrow = deploy_escrow( + OWNER(), + ETH_TRANSFER_CONTRACT(), + MM_ETHEREUM(), + MM_STARKNET(), + eth_token.contract_address + ); + + start_prank(CheatTarget::One(eth_token.contract_address), OWNER()); + eth_token.transfer(USER(), balance); + stop_prank(CheatTarget::One(eth_token.contract_address)); + + start_prank(CheatTarget::One(eth_token.contract_address), USER()); + eth_token.approve(escrow.contract_address, approved); + stop_prank(CheatTarget::One(eth_token.contract_address)); + + (escrow, eth_token) + } + + fn deploy_escrow( + escrow_owner: ContractAddress, + eth_transfer_contract: EthAddress, + mm_ethereum_contract: EthAddress, + mm_starknet_contract: ContractAddress, + native_token_eth_starknet: ContractAddress + ) -> IEscrowDispatcher { + let escrow = declare('Escrow'); + let mut calldata: Array = ArrayTrait::new(); + calldata.append(escrow_owner.into()); + calldata.append(eth_transfer_contract.into()); + calldata.append(mm_ethereum_contract.into()); + calldata.append(mm_starknet_contract.into()); + calldata.append(native_token_eth_starknet.into()); + let address = escrow.deploy(@calldata).unwrap(); + return IEscrowDispatcher { contract_address: address }; + } + + fn deploy_erc20( + name: felt252, symbol: felt252, initial_supply: u256, recipent: ContractAddress + ) -> IERC20Dispatcher { + let erc20 = declare('ERC20'); + let mut calldata = array![name, symbol]; + Serde::serialize(@initial_supply, ref calldata); + calldata.append(recipent.into()); + let address = erc20.deploy(@calldata).unwrap(); + return IERC20Dispatcher { contract_address: address }; + } + + #[test] + fn test_happy_path() { + let (escrow, eth_token) = setup(); + + // check balance + assert(eth_token.balanceOf(escrow.contract_address) == 0, 'init: wrong balance'); + assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'init: wrong balance'); + + start_prank(CheatTarget::One(escrow.contract_address), USER()); + let order = Order { recipient_address: 12345.try_into().unwrap(), amount: 500, fee: 0 }; + let order_id = escrow.set_order(order); + stop_prank(CheatTarget::One(escrow.contract_address)); + + // check balance + assert(eth_token.balanceOf(escrow.contract_address) == 500, 'set_order: wrong balance '); + assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'set_order: wrong balance'); + + // check Order + assert(order_id == 0, 'wrong order_id'); + let order_save = escrow.get_order(order_id); + assert(order.recipient_address == order_save.recipient_address, 'wrong recipient_address'); + assert(order.amount == order_save.amount, 'wrong amount'); + assert(escrow.get_order_pending(order_id), 'wrong order used'); + + let mut l1_handler = L1HandlerTrait::new( + contract_address: escrow.contract_address, + function_name: 'claim_payment' + ); + + let mut payload_buffer: Array = ArrayTrait::new(); + Serde::serialize(@order_id, ref payload_buffer); + Serde::serialize(@order.recipient_address, ref payload_buffer); + Serde::serialize(@order.amount, ref payload_buffer); + + l1_handler.from_address = ETH_TRANSFER_CONTRACT().into(); + l1_handler.payload = payload_buffer.span(); + + l1_handler.execute().expect('Failed to execute l1_handler'); + + // check Order + assert(!escrow.get_order_pending(order_id), 'wrong order used'); + // check balance + assert(eth_token.balanceOf(escrow.contract_address) == 0, 'withdraw: wrong balance'); + assert(eth_token.balanceOf(MM_STARKNET()) == 500, 'withdraw: wrong balance'); + } + + #[test] + fn test_claim_batch_happy() { + let (escrow, eth_token) = setup(); + + // check balance + assert(eth_token.balanceOf(escrow.contract_address) == 0, 'init: wrong balance'); + assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'init: wrong balance'); + + let recipient_addresses = array![12345.try_into().unwrap(), 12346.try_into().unwrap(), 12347.try_into().unwrap()]; + let amounts = array![500, 501, 502]; + let fees = array![3, 2, 1]; + + let recipient_adresses_clone = recipient_addresses.clone(); + let amounts_clone = amounts.clone(); + + let mut order_ids = array![]; + + start_prank(CheatTarget::One(escrow.contract_address), USER()); + + let mut idx = 0; + loop { + if idx >= 3 { + break; + } + + let recipient_address = recipient_addresses.at(idx).clone(); + let amount = amounts.at(idx).clone(); + let fee = fees.at(idx).clone(); + + let order_id = _create_order(recipient_address, amount, fee, escrow); + + order_ids.append(order_id); + + idx += 1; + }; + + // check balance + assert(eth_token.balanceOf(escrow.contract_address) == 1509, 'set_order: wrong balance '); + assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'set_order: wrong balance'); + + // Call withdraw_batch l1_handler + let mut l1_handler = L1HandlerTrait::new( + contract_address: escrow.contract_address, + function_name: 'claim_payment_batch' + ); + + let mut payload_buffer: Array = ArrayTrait::new(); + Serde::serialize(@order_ids, ref payload_buffer); + Serde::serialize(@recipient_adresses_clone, ref payload_buffer); + Serde::serialize(@amounts_clone, ref payload_buffer); + + l1_handler.from_address = ETH_TRANSFER_CONTRACT().into(); + l1_handler.payload = payload_buffer.span(); + + l1_handler.execute().expect('Failed to execute l1_handler'); + + assert(eth_token.balanceOf(escrow.contract_address) == 0, 'withdraw: wrong balance'); + assert(eth_token.balanceOf(MM_STARKNET()) == 1509, 'withdraw: wrong balance'); + + // Check order_ids + let mut idx = 0; + loop { + if idx >= 3 { + break; + } + + let order_id = order_ids.at(idx).clone(); + assert(!escrow.get_order_pending(order_id), 'Order not used'); + idx += 1; + }; + } + + + #[test] + fn test_fail_random_eth_user_calls_l1_handler() { + let (escrow, _) = setup(); + let data: Array = array![1, MM_ETHEREUM().into(), 3, 4]; + let mut payload_buffer: Array = ArrayTrait::new(); + data.serialize(ref payload_buffer); + let mut l1_handler = L1HandlerTrait::new( + contract_address: escrow.contract_address, + function_name: 'claim_payment', + ); + l1_handler.from_address = ETH_USER().into(); + + l1_handler.payload = payload_buffer.span(); + + // same as "Should Panic" but for the L1 handler function + match l1_handler.execute() { + Result::Ok(_) => panic_with_felt252('shouldve panicked'), + Result::Err(RevertedTransaction) => { + assert(*RevertedTransaction.panic_data.at(0) == 'Only PAYMENT_REGISTRY_CONTRACT', *RevertedTransaction.panic_data.at(0)); + } + } + } + + fn _create_order( + recipient_address: EthAddress, + amount: u256, + fee: u256, + escrow: IEscrowDispatcher, + ) -> u256 { + start_prank(CheatTarget::One(escrow.contract_address), USER()); + let order = Order { recipient_address: recipient_address.try_into().unwrap(), amount: amount, fee: fee }; + let order_id = escrow.set_order(order); + stop_prank(CheatTarget::One(escrow.contract_address)); + return order_id; + } +} From 9e370f8e4bf46fbd38381cfa2f0ee9f50a82da75 Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Tue, 19 Mar 2024 12:51:45 -0300 Subject: [PATCH 02/11] feat (ethereum): claim payment batch) --- contracts/ethereum/src/PaymentRegistry.sol | 39 ++++++++++++++-- contracts/ethereum/test/Transfer.sol | 53 ++++++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/contracts/ethereum/src/PaymentRegistry.sol b/contracts/ethereum/src/PaymentRegistry.sol index 87168ed3..d0774bcf 100644 --- a/contracts/ethereum/src/PaymentRegistry.sol +++ b/contracts/ethereum/src/PaymentRegistry.sol @@ -60,14 +60,12 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { } function claimPayment(uint256 orderId, uint256 destAddress, uint256 amount) external payable onlyOwnerOrMM { - bytes32 index = keccak256(abi.encodePacked(orderId, destAddress, amount)); - TransferInfo storage transferInfo = transfers[index]; - require(transferInfo.isUsed == true, "Transfer not found."); + _claimPayment(orderId, destAddress, amount); uint256[] memory payload = new uint256[](5); payload[0] = uint128(orderId); // low payload[1] = uint128(orderId >> 128); // high - payload[2] = transferInfo.destAddress; + payload[2] = destAddress; payload[3] = uint128(amount); // low payload[4] = uint128(amount >> 128); // high @@ -77,6 +75,39 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { payload); } + function claimPaymentBatch(uint256[] calldata orderIds, uint256[] calldata destAddresses, uint256[] calldata amounts) external payable { + require(orderIds.length == destAddresses.length, "Invalid lengths."); + require(orderIds.length == amounts.length, "Invalid lengths."); + + uint256[] memory payload = new uint256[](5 * orderIds.length); + + for (uint32 idx = 0; idx < orderIds.length; idx++) { + uint256 orderId = orderIds[idx]; + uint256 destAddress = destAddresses[idx]; + uint256 amount = amounts[idx]; + + _claimPayment(orderId, destAddress, amount); + + uint32 base_idx = 5 * idx; + payload[base_idx] = uint128(orderId); // low + payload[base_idx + 1] = uint128(orderId >> 128); // high + payload[base_idx + 2] = destAddress; + payload[base_idx + 3] = uint128(amount); // low + payload[base_idx + 4] = uint128(amount >> 128); // high + } + + _snMessaging.sendMessageToL2{value: msg.value}( + _snEscrowAddress, + _snEscrowClaimPaymentSelector, + payload); + } + + function _claimPayment(uint256 orderId, uint256 destAddress, uint256 amount) internal { + bytes32 index = keccak256(abi.encodePacked(orderId, destAddress, amount)); + TransferInfo storage transferInfo = transfers[index]; + require(transferInfo.isUsed == true, "Transfer not found."); + } + function setEscrowAddress(uint256 snEscrowAddress) external onlyOwner { _snEscrowAddress = snEscrowAddress; emit ModifiedEscrowAddress(snEscrowAddress); diff --git a/contracts/ethereum/test/Transfer.sol b/contracts/ethereum/test/Transfer.sol index 9e48e239..86db3f7a 100644 --- a/contracts/ethereum/test/Transfer.sol +++ b/contracts/ethereum/test/Transfer.sol @@ -89,4 +89,57 @@ contract TransferTest is Test { hoax(marketMaker, 1 wei); yab_caller.claimPayment(1, 0x1, 1); } + + function testClaimPaymentBatch() public { + hoax(marketMaker, 3 wei); + yab_caller.transfer{value: 3}(1, 0x1, 3); + hoax(marketMaker, 2 wei); + yab_caller.transfer{value: 2}(2, 0x3, 2); + hoax(marketMaker, 1 wei); + yab_caller.transfer{value: 1}(3, 0x5, 1); + + uint256[] memory orderIds = new uint256[](3); + uint256[] memory destAddresses = new uint256[](3); + uint256[] memory amounts = new uint256[](3); + + orderIds[0] = 1; + orderIds[1] = 2; + orderIds[2] = 3; + + destAddresses[0] = 0x1; + destAddresses[1] = 0x3; + destAddresses[2] = 0x5; + + amounts[0] = 3; + amounts[1] = 2; + amounts[2] = 1; + + yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts); + } + + function testWithdrawBatchMissingTransfer() public { + hoax(marketMaker, 3 wei); + yab_caller.transfer{value: 3}(1, 0x1, 3); + hoax(marketMaker, 2 wei); + yab_caller.transfer{value: 2}(2, 0x3, 2); + + uint256[] memory orderIds = new uint256[](3); + uint256[] memory destAddresses = new uint256[](3); + uint256[] memory amounts = new uint256[](3); + + orderIds[0] = 1; + orderIds[1] = 2; + orderIds[2] = 3; + + destAddresses[0] = 0x1; + destAddresses[1] = 0x3; + destAddresses[2] = 0x5; + + amounts[0] = 3; + amounts[1] = 2; + amounts[2] = 1; + + vm.expectRevert("Transfer not found."); + yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts);(orderIds, destAddresses, amounts); + } } From ef53cf59e888e3280931fd1f510bf0b60844d52c Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Tue, 19 Mar 2024 13:05:02 -0300 Subject: [PATCH 03/11] fix (ethereum): add caller restrictions on claimPaymentBatch --- contracts/ethereum/src/PaymentRegistry.sol | 4 ++-- contracts/ethereum/test/Transfer.sol | 23 +++++++++++++++++++++- contracts/starknet/src/escrow.cairo | 2 +- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/contracts/ethereum/src/PaymentRegistry.sol b/contracts/ethereum/src/PaymentRegistry.sol index d0774bcf..6dc1a02a 100644 --- a/contracts/ethereum/src/PaymentRegistry.sol +++ b/contracts/ethereum/src/PaymentRegistry.sol @@ -75,7 +75,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { payload); } - function claimPaymentBatch(uint256[] calldata orderIds, uint256[] calldata destAddresses, uint256[] calldata amounts) external payable { + function claimPaymentBatch(uint256[] calldata orderIds, uint256[] calldata destAddresses, uint256[] calldata amounts) external payable onlyOwnerOrMM() { require(orderIds.length == destAddresses.length, "Invalid lengths."); require(orderIds.length == amounts.length, "Invalid lengths."); @@ -102,7 +102,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { payload); } - function _claimPayment(uint256 orderId, uint256 destAddress, uint256 amount) internal { + function _claimPayment(uint256 orderId, uint256 destAddress, uint256 amount) internal view { bytes32 index = keccak256(abi.encodePacked(orderId, destAddress, amount)); TransferInfo storage transferInfo = transfers[index]; require(transferInfo.isUsed == true, "Transfer not found."); diff --git a/contracts/ethereum/test/Transfer.sol b/contracts/ethereum/test/Transfer.sol index 86db3f7a..8df4e407 100644 --- a/contracts/ethereum/test/Transfer.sol +++ b/contracts/ethereum/test/Transfer.sol @@ -114,10 +114,11 @@ contract TransferTest is Test { amounts[1] = 2; amounts[2] = 1; + hoax(marketMaker); yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts); } - function testWithdrawBatchMissingTransfer() public { + function testClaimPaymentBatch_fail_MissingTransfer() public { hoax(marketMaker, 3 wei); yab_caller.transfer{value: 3}(1, 0x1, 3); hoax(marketMaker, 2 wei); @@ -140,6 +141,26 @@ contract TransferTest is Test { amounts[2] = 1; vm.expectRevert("Transfer not found."); + hoax(marketMaker); yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts);(orderIds, destAddresses, amounts); } + + function testClaimPaymentBatch_fail_notOwnerOrMM() public { + hoax(marketMaker, 3 wei); + yab_caller.transfer{value: 3}(1, 0x1, 3); + + uint256[] memory orderIds = new uint256[](1); + uint256[] memory destAddresses = new uint256[](1); + uint256[] memory amounts = new uint256[](1); + + orderIds[0] = 1; + + destAddresses[0] = 0x1; + + amounts[0] = 3; + + hoax(makeAddr("bob"), 100 wei); + vm.expectRevert("Only Owner or MM can call this function"); + yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts); + } } diff --git a/contracts/starknet/src/escrow.cairo b/contracts/starknet/src/escrow.cairo index 5e37acab..d36a7c6f 100644 --- a/contracts/starknet/src/escrow.cairo +++ b/contracts/starknet/src/escrow.cairo @@ -256,7 +256,7 @@ mod Escrow { amounts: Array ) { let eth_transfer_contract_felt: felt252 = self.eth_transfer_contract.read().into(); - assert(from_address == eth_transfer_contract_felt, 'Only YAB_TRANSFER_CONTRACT'); + assert(from_address == eth_transfer_contract_felt, 'Only PAYMENT_REGISTRY_CONTRACT'); assert(order_ids.len() == recipient_addresses.len(), 'Different lengths'); assert(order_ids.len() == amounts.len(), 'Different lengths'); From e1b361e99561167c89452fdcfdcf7a4ee07ed7b2 Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Tue, 19 Mar 2024 18:42:04 -0300 Subject: [PATCH 04/11] fix: call claim payment batch when batching from layer 1 + serialization issue --- contracts/ethereum/deploy.sh | 2 ++ contracts/ethereum/script/Deploy.s.sol | 9 ++++++- .../ethereum/set_claim_payment_selector.sh | 12 +++++++++ contracts/ethereum/src/PaymentRegistry.sol | 27 ++++++++++++++++--- contracts/ethereum/test/MM_ACL.sol | 2 +- contracts/ethereum/test/Transfer.sol | 2 +- contracts/starknet/src/escrow.cairo | 17 ++++++------ .../src/tests/test_escrow_claim.cairo | 10 +++---- 8 files changed, 59 insertions(+), 22 deletions(-) diff --git a/contracts/ethereum/deploy.sh b/contracts/ethereum/deploy.sh index 0c67d178..d344b683 100755 --- a/contracts/ethereum/deploy.sh +++ b/contracts/ethereum/deploy.sh @@ -5,6 +5,8 @@ cd contracts/ethereum printf "${GREEN}\n=> [ETH] Deploying ERC1967Proxy & PaymentRegistry ${COLOR_RESET}\n" +echo $MM_ETHEREUM_WALLET_ADDRESS + RESULT_LOG=$(forge script ./script/Deploy.s.sol --rpc-url $ETHEREUM_RPC --broadcast ${SKIP_VERIFY:---verify}) # echo "$RESULT_LOG" #uncomment this line for debugging in detail diff --git a/contracts/ethereum/script/Deploy.s.sol b/contracts/ethereum/script/Deploy.s.sol index a7225690..bb348618 100644 --- a/contracts/ethereum/script/Deploy.s.sol +++ b/contracts/ethereum/script/Deploy.s.sol @@ -13,11 +13,18 @@ contract Deploy is Script { address snMessagingAddress = vm.envAddress("STARKNET_MESSAGING_ADDRESS"); uint256 snEscrowAddress = 0x0; // this value is set in a call to the smart contract, once deployed uint256 snClaimPaymentSelector = 0x0; // this value is set in a call to the smart contract, once deployed + uint256 snClaimPaymentBatchSelector = 0x0; // this value is set in a call to the smart contract, once deployed address marketMaker = vm.envAddress("MM_ETHEREUM_WALLET_ADDRESS"); PaymentRegistry yab = new PaymentRegistry(); ERC1967Proxy proxy = new ERC1967Proxy(address(yab), ""); - PaymentRegistry(address(proxy)).initialize(snMessagingAddress, snEscrowAddress, snClaimPaymentSelector, marketMaker); + PaymentRegistry(address(proxy)).initialize( + snMessagingAddress, + snEscrowAddress, + snClaimPaymentSelector, + snClaimPaymentBatchSelector, + marketMaker + ); vm.stopBroadcast(); diff --git a/contracts/ethereum/set_claim_payment_selector.sh b/contracts/ethereum/set_claim_payment_selector.sh index e21783cf..ba682d32 100755 --- a/contracts/ethereum/set_claim_payment_selector.sh +++ b/contracts/ethereum/set_claim_payment_selector.sh @@ -11,6 +11,11 @@ if [ -z "$CLAIM_PAYMENT_NAME" ]; then echo "CLAIM_PAYMENT_NAME Variable is empty. Aborting execution.\n" exit 1 fi +if [ -z "$CLAIM_PAYMENT_BATCH_NAME" ]; then + printf "\n${RED}ERROR:${COLOR_RESET}\n" + echo "CLAIM_PAYMENT_BATCH_NAME Variable is empty. Aborting execution.\n" + exit 1 +fi printf "${GREEN}\n=> [ETH] Setting Starknet ClaimPayment Selector on ETH Smart Contract${COLOR_RESET}\n" echo "Smart contract being modified:" $PAYMENT_REGISTRY_PROXY_ADDRESS @@ -20,3 +25,10 @@ echo "New ClaimPayment Selector: ${CLAIM_PAYMENT_SELECTOR}" cast send --rpc-url $ETHEREUM_RPC --private-key $ETHEREUM_PRIVATE_KEY $PAYMENT_REGISTRY_PROXY_ADDRESS "setEscrowClaimPaymentSelector(uint256)" "${CLAIM_PAYMENT_SELECTOR}" | grep "transactionHash" echo "Done setting ClaimPayment selector" + +printf "${GREEN}\n=> [ETH] Setting Starknet ClaimPaymentBatch Selector on ETH Smart Contract${COLOR_RESET}\n" +CLAIM_PAYMENT_BATCH_SELECTOR=$(starkli selector $CLAIM_PAYMENT_BATCH_NAME) +echo "New ClaimPaymentBatch Selector: ${CLAIM_PAYMENT_BATCH_SELECTOR}" + +cast send --rpc-url $ETHEREUM_RPC --private-key $ETHEREUM_PRIVATE_KEY $PAYMENT_REGISTRY_PROXY_ADDRESS "setEscrowClaimPaymentBatchSelector(uint256)" "${CLAIM_PAYMENT_BATCH_SELECTOR}" | grep "transactionHash" +echo "Done setting ClaimPaymentBatch selector" diff --git a/contracts/ethereum/src/PaymentRegistry.sol b/contracts/ethereum/src/PaymentRegistry.sol index 6dc1a02a..1d15b130 100644 --- a/contracts/ethereum/src/PaymentRegistry.sol +++ b/contracts/ethereum/src/PaymentRegistry.sol @@ -16,12 +16,14 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { event Transfer(uint256 indexed orderId, address srcAddress, TransferInfo transferInfo); event ModifiedEscrowAddress(uint256 newEscrowAddress); event ModifiedEscrowClaimPaymentSelector(uint256 newEscrowClaimPaymentSelector); + event ModifiedEscrowClaimPaymentBatchSelector(uint256 newEscrowClaimPaymentSelector); mapping(bytes32 => TransferInfo) public transfers; address private _marketMaker; IStarknetMessaging private _snMessaging; uint256 private _snEscrowAddress; uint256 private _snEscrowClaimPaymentSelector; + uint256 private _snEscrowClaimPaymentBatchSelector; constructor() { _disableInitializers(); @@ -32,6 +34,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { address snMessaging, uint256 snEscrowAddress, uint256 snEscrowClaimPaymentSelector, + uint256 snEscrowClaimPaymentBatchSelector, address marketMaker) public initializer { __Ownable_init(msg.sender); __UUPSUpgradeable_init(); @@ -39,6 +42,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { _snMessaging = IStarknetMessaging(snMessaging); _snEscrowAddress = snEscrowAddress; _snEscrowClaimPaymentSelector = snEscrowClaimPaymentSelector; + _snEscrowClaimPaymentBatchSelector = snEscrowClaimPaymentBatchSelector; _marketMaker = marketMaker; } @@ -75,11 +79,17 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { payload); } - function claimPaymentBatch(uint256[] calldata orderIds, uint256[] calldata destAddresses, uint256[] calldata amounts) external payable onlyOwnerOrMM() { + function claimPaymentBatch( + uint256[] calldata orderIds, + uint256[] calldata destAddresses, + uint256[] calldata amounts + ) external payable onlyOwnerOrMM() { require(orderIds.length == destAddresses.length, "Invalid lengths."); require(orderIds.length == amounts.length, "Invalid lengths."); - uint256[] memory payload = new uint256[](5 * orderIds.length); + uint256[] memory payload = new uint256[](5 * orderIds.length + 1); + + payload[0] = orderIds.length; for (uint32 idx = 0; idx < orderIds.length; idx++) { uint256 orderId = orderIds[idx]; @@ -88,7 +98,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { _claimPayment(orderId, destAddress, amount); - uint32 base_idx = 5 * idx; + uint32 base_idx = 1 + 5 * idx; payload[base_idx] = uint128(orderId); // low payload[base_idx + 1] = uint128(orderId >> 128); // high payload[base_idx + 2] = destAddress; @@ -98,7 +108,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { _snMessaging.sendMessageToL2{value: msg.value}( _snEscrowAddress, - _snEscrowClaimPaymentSelector, + _snEscrowClaimPaymentBatchSelector, payload); } @@ -118,6 +128,11 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { emit ModifiedEscrowClaimPaymentSelector(snEscrowClaimPaymentSelector); } + function setEscrowClaimPaymentBatchSelector(uint256 snEscrowClaimPaymentBatchSelector) external onlyOwner { + _snEscrowClaimPaymentBatchSelector = snEscrowClaimPaymentBatchSelector; + emit ModifiedEscrowClaimPaymentBatchSelector(snEscrowClaimPaymentBatchSelector); + } + function getEscrowAddress() external view returns (uint256) { return _snEscrowAddress; } @@ -125,6 +140,10 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { function getEscrowClaimPaymentSelector() external view returns (uint256) { return _snEscrowClaimPaymentSelector; } + + function getEscrowClaimPaymentBatchSelector() external view returns (uint256) { + return _snEscrowClaimPaymentBatchSelector; + } //// MM ACL: diff --git a/contracts/ethereum/test/MM_ACL.sol b/contracts/ethereum/test/MM_ACL.sol index f5fa114c..91d91780 100644 --- a/contracts/ethereum/test/MM_ACL.sol +++ b/contracts/ethereum/test/MM_ACL.sol @@ -23,7 +23,7 @@ contract TransferTest is Test { yab = new PaymentRegistry(); proxy = new ERC1967Proxy(address(yab), ""); yab_caller = PaymentRegistry(address(proxy)); - yab_caller.initialize(STARKNET_MESSAGING_ADDRESS, snEscrowAddress, SN_ESCROW_CLAIM_PAYMENT_SELECTOR, marketMaker); + yab_caller.initialize(STARKNET_MESSAGING_ADDRESS, snEscrowAddress, SN_ESCROW_CLAIM_PAYMENT_SELECTOR, 0x0, marketMaker); vm.stopPrank(); } diff --git a/contracts/ethereum/test/Transfer.sol b/contracts/ethereum/test/Transfer.sol index 8df4e407..8ad527d5 100644 --- a/contracts/ethereum/test/Transfer.sol +++ b/contracts/ethereum/test/Transfer.sol @@ -23,7 +23,7 @@ contract TransferTest is Test { yab = new PaymentRegistry(); proxy = new ERC1967Proxy(address(yab), ""); yab_caller = PaymentRegistry(address(proxy)); - yab_caller.initialize(STARKNET_MESSAGING_ADDRESS, snEscrowAddress, SN_ESCROW_CLAIM_PAYMENT_SELECTOR, marketMaker); + yab_caller.initialize(STARKNET_MESSAGING_ADDRESS, snEscrowAddress, SN_ESCROW_CLAIM_PAYMENT_SELECTOR, 0x0, marketMaker); // Mock calls to Starknet Messaging contract vm.mockCall( diff --git a/contracts/starknet/src/escrow.cairo b/contracts/starknet/src/escrow.cairo index d36a7c6f..60cdf1c4 100644 --- a/contracts/starknet/src/escrow.cairo +++ b/contracts/starknet/src/escrow.cairo @@ -31,7 +31,8 @@ trait IEscrow { #[starknet::contract] mod Escrow { - use super::{IEscrow, Order}; + use core::traits::Into; +use super::{IEscrow, Order}; use openzeppelin::{ access::ownable::OwnableComponent, @@ -251,24 +252,21 @@ mod Escrow { fn claim_payment_batch( ref self: ContractState, from_address: felt252, - order_ids: Array, - recipient_addresses: Array, - amounts: Array + orders: Array<(u256, EthAddress, u256)> ) { let eth_transfer_contract_felt: felt252 = self.eth_transfer_contract.read().into(); assert(from_address == eth_transfer_contract_felt, 'Only PAYMENT_REGISTRY_CONTRACT'); - assert(order_ids.len() == recipient_addresses.len(), 'Different lengths'); - assert(order_ids.len() == amounts.len(), 'Different lengths'); - let mut idx = 0; loop { - if idx >= order_ids.len() { + if idx >= orders.len() { break; } - _claim_payment(ref self, from_address, *order_ids.at(idx), *recipient_addresses.at(idx), *amounts.at(idx)); + let (order_id, recipient_address, amount) = *orders.at(idx); + + _claim_payment(ref self, from_address, order_id, recipient_address, amount); idx += 1; }; @@ -290,6 +288,7 @@ mod Escrow { self.orders_pending.write(order_id, false); let payment_amount = order.amount + order.fee; + // TODO: Might be best to transfer all at once IERC20Dispatcher { contract_address: self.native_token_eth_starknet.read() } .transfer(self.mm_starknet_wallet.read(), payment_amount); diff --git a/contracts/starknet/src/tests/test_escrow_claim.cairo b/contracts/starknet/src/tests/test_escrow_claim.cairo index 99c9548a..c8b4319b 100644 --- a/contracts/starknet/src/tests/test_escrow_claim.cairo +++ b/contracts/starknet/src/tests/test_escrow_claim.cairo @@ -152,7 +152,7 @@ mod Escrow { let recipient_adresses_clone = recipient_addresses.clone(); let amounts_clone = amounts.clone(); - let mut order_ids = array![]; + let mut orders = array![]; start_prank(CheatTarget::One(escrow.contract_address), USER()); @@ -168,7 +168,7 @@ mod Escrow { let order_id = _create_order(recipient_address, amount, fee, escrow); - order_ids.append(order_id); + orders.append((order_id, recipient_address, amount)); idx += 1; }; @@ -184,9 +184,7 @@ mod Escrow { ); let mut payload_buffer: Array = ArrayTrait::new(); - Serde::serialize(@order_ids, ref payload_buffer); - Serde::serialize(@recipient_adresses_clone, ref payload_buffer); - Serde::serialize(@amounts_clone, ref payload_buffer); + Serde::serialize(@orders, ref payload_buffer); l1_handler.from_address = ETH_TRANSFER_CONTRACT().into(); l1_handler.payload = payload_buffer.span(); @@ -203,7 +201,7 @@ mod Escrow { break; } - let order_id = order_ids.at(idx).clone(); + let (order_id, _, _) = orders.at(idx).clone(); assert(!escrow.get_order_pending(order_id), 'Order not used'); idx += 1; }; From 632a756554751b0d70482fe76d39c383eef9976a Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Tue, 19 Mar 2024 18:55:36 -0300 Subject: [PATCH 05/11] fix: add missing CLAIM_PAYMENT_BATCH_NAME env variable to .env.test --- contracts/starknet/.env.example | 1 + contracts/starknet/.env.test | 1 + 2 files changed, 2 insertions(+) diff --git a/contracts/starknet/.env.example b/contracts/starknet/.env.example index 6b058d1f..515ae79a 100644 --- a/contracts/starknet/.env.example +++ b/contracts/starknet/.env.example @@ -7,5 +7,6 @@ STARKNET_RPC= STARKNET_ESCROW_OWNER= #in lowercase hexa with the 0x prefix MM_STARKNET_WALLET_ADDRESS= #in lowercase hexa with the 0x prefix CLAIM_PAYMENT_NAME= #must match the exact name of the function to claim the payment from the starknet smart contract +CLAIM_PAYMENT_BATCH_NAME= #must match the exact name of the function to claim the payment batch from the starknet smart contract MM_ETHEREUM_WALLET_ADDRESS= #in lowercase hexa with the 0x prefix NATIVE_TOKEN_ETH_STARKNET= #in lowercase hexa with the 0x prefix diff --git a/contracts/starknet/.env.test b/contracts/starknet/.env.test index 6dba7cfe..a7a6df4c 100644 --- a/contracts/starknet/.env.test +++ b/contracts/starknet/.env.test @@ -5,5 +5,6 @@ STARKNET_RPC=http://0.0.0.0:5050 STARKNET_ESCROW_OWNER=0x517ececd29116499f4a1b64b094da79ba08dfd54a3edaa316134c41f8160973 MM_STARKNET_WALLET_ADDRESS=0x5686a647a9cdd63ade617e0baf3b364856b813b508f03903eb58a7e622d5855 CLAIM_PAYMENT_NAME=claim_payment +CLAIM_PAYMENT_BATCH_NAME=claim_payment_batch MM_ETHEREUM_WALLET_ADDRESS=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 NATIVE_TOKEN_ETH_STARKNET=0x49d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7 From 70dbc28b6a5c37a91eaaf8973da4f8dc2aa57242 Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Fri, 22 Mar 2024 17:59:01 -0300 Subject: [PATCH 06/11] fix: set claim payment batch selector correctly --- contracts/ethereum/set_starknet_claim_payment_selector.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/ethereum/set_starknet_claim_payment_selector.sh b/contracts/ethereum/set_starknet_claim_payment_selector.sh index ed872044..4377f652 100755 --- a/contracts/ethereum/set_starknet_claim_payment_selector.sh +++ b/contracts/ethereum/set_starknet_claim_payment_selector.sh @@ -30,5 +30,5 @@ printf "${GREEN}\n=> [ETH] Setting Starknet ClaimPaymentBatch Selector on ETH Sm CLAIM_PAYMENT_BATCH_SELECTOR=$(starkli selector $CLAIM_PAYMENT_BATCH_NAME) echo "New ClaimPaymentBatch Selector: ${CLAIM_PAYMENT_BATCH_SELECTOR}" -cast send --rpc-url $ETHEREUM_RPC --private-key $ETHEREUM_PRIVATE_KEY $PAYMENT_REGISTRY_PROXY_ADDRESS "setEscrowClaimPaymentBatchSelector(uint256)" "${CLAIM_PAYMENT_BATCH_SELECTOR}" | grep "transactionHash" +cast send --rpc-url $ETHEREUM_RPC --private-key $ETHEREUM_PRIVATE_KEY $PAYMENT_REGISTRY_PROXY_ADDRESS "setStarknetClaimPaymentBatchSelector(uint256)" "${CLAIM_PAYMENT_BATCH_SELECTOR}" | grep "transactionHash" echo "Done setting ClaimPaymentBatch selector" From afa63ea46e63cb1ba712a7337bba490331fda65d Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Mon, 25 Mar 2024 14:32:56 -0300 Subject: [PATCH 07/11] test: permissons & missing order tests for batch --- .../ethereum/test/Transfer_Claim_SN.t.sol | 34 ++++++- .../src/tests/test_escrow_claim.cairo | 97 ++++++++++++++++++- .../starknet/src/tests/utils/constants.cairo | 8 ++ 3 files changed, 136 insertions(+), 3 deletions(-) diff --git a/contracts/ethereum/test/Transfer_Claim_SN.t.sol b/contracts/ethereum/test/Transfer_Claim_SN.t.sol index d856d7c3..34f3143c 100644 --- a/contracts/ethereum/test/Transfer_Claim_SN.t.sol +++ b/contracts/ethereum/test/Transfer_Claim_SN.t.sol @@ -108,6 +108,38 @@ contract TransferTest is Test { hoax(marketMaker); yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts); + + assertEq(address(0x1).balance, 3); + assertEq(address(0x3).balance, 2); + assertEq(address(0x5).balance, 1); + } + + function testClaimPaymentBatchPartial() public { + hoax(marketMaker, 3 wei); + yab_caller.transfer{value: 3}(1, 0x1, PaymentRegistry.Chain.Starknet); + hoax(marketMaker, 2 wei); + yab_caller.transfer{value: 2}(2, 0x3, PaymentRegistry.Chain.Starknet); + hoax(marketMaker, 1 wei); + yab_caller.transfer{value: 1}(3, 0x5, PaymentRegistry.Chain.Starknet); + + uint256[] memory orderIds = new uint256[](2); + uint256[] memory destAddresses = new uint256[](2); + uint256[] memory amounts = new uint256[](2); + + orderIds[0] = 1; + orderIds[1] = 2; + + destAddresses[0] = 0x1; + destAddresses[1] = 0x3; + + amounts[0] = 3; + amounts[1] = 2; + + hoax(marketMaker); + yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts); + + assertEq(address(0x1).balance, 3); + assertEq(address(0x3).balance, 2); } function testClaimPaymentBatch_fail_MissingTransfer() public { @@ -134,7 +166,7 @@ contract TransferTest is Test { vm.expectRevert("Transfer not found."); hoax(marketMaker); - yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts);(orderIds, destAddresses, amounts); + yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts); } function testClaimPaymentBatch_fail_notOwnerOrMM() public { diff --git a/contracts/starknet/src/tests/test_escrow_claim.cairo b/contracts/starknet/src/tests/test_escrow_claim.cairo index c8b4319b..bfee19d3 100644 --- a/contracts/starknet/src/tests/test_escrow_claim.cairo +++ b/contracts/starknet/src/tests/test_escrow_claim.cairo @@ -1,4 +1,5 @@ mod Escrow { + use core::array::ArrayTrait; use core::to_byte_array::FormatAsByteArray; use core::serde::Serde; use core::traits::Into; @@ -18,7 +19,7 @@ mod Escrow { use yab::tests::utils::{ constants::EscrowConstants::{ - USER, OWNER, MM_STARKNET, MM_ETHEREUM, ETH_TRANSFER_CONTRACT, ETH_USER + USER, OWNER, MM_STARKNET, MM_ETHEREUM, ETH_TRANSFER_CONTRACT, ETH_USER, ETH_USER_2, ETH_USER_3 }, }; @@ -145,7 +146,7 @@ mod Escrow { assert(eth_token.balanceOf(escrow.contract_address) == 0, 'init: wrong balance'); assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'init: wrong balance'); - let recipient_addresses = array![12345.try_into().unwrap(), 12346.try_into().unwrap(), 12347.try_into().unwrap()]; + let recipient_addresses = array![ETH_USER(), ETH_USER_2(), ETH_USER_3()]; let amounts = array![500, 501, 502]; let fees = array![3, 2, 1]; @@ -207,6 +208,38 @@ mod Escrow { }; } + #[test] + fn test_claim_batch_fail_missing_order_id() { + let (escrow, eth_token) = setup(); + + let mut orders = array![]; + + let amount = 500; + let order_id = _create_order(ETH_USER(), amount, 1, escrow); + + orders.append((order_id, ETH_USER(), amount)); + orders.append((order_id + 1, ETH_USER_2(), amount)); + + // Call withdraw_batch l1_handler + let mut l1_handler = L1HandlerTrait::new( + contract_address: escrow.contract_address, + function_name: 'claim_payment_batch' + ); + + let mut payload_buffer: Array = ArrayTrait::new(); + Serde::serialize(@orders, ref payload_buffer); + + l1_handler.from_address = ETH_TRANSFER_CONTRACT().into(); + l1_handler.payload = payload_buffer.span(); + + // same as "Should Panic" but for the L1 handler function + match l1_handler.execute() { + Result::Ok(_) => panic_with_felt252('shouldve panicked'), + Result::Err(RevertedTransaction) => { + assert(*RevertedTransaction.panic_data.at(0) == 'Order withdrew or nonexistent', *RevertedTransaction.panic_data.at(0)); + } + } + } #[test] fn test_fail_random_eth_user_calls_l1_handler() { @@ -231,6 +264,66 @@ mod Escrow { } } + #[test] + fn test_fail_random_eth_user_calls_l1_handler_batch() { + let (escrow, eth_token) = setup(); + + assert(eth_token.balanceOf(escrow.contract_address) == 0, 'init: wrong balance'); + assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'init: wrong balance'); + + let recipient_addresses = array![12345.try_into().unwrap(), 12346.try_into().unwrap(), 12347.try_into().unwrap()]; + let amounts = array![500, 501, 502]; + let fees = array![3, 2, 1]; + + let recipient_adresses_clone = recipient_addresses.clone(); + let amounts_clone = amounts.clone(); + + let mut orders = array![]; + + start_prank(CheatTarget::One(escrow.contract_address), USER()); + + let mut idx = 0; + loop { + if idx >= 3 { + break; + } + + let recipient_address = recipient_addresses.at(idx).clone(); + let amount = amounts.at(idx).clone(); + let fee = fees.at(idx).clone(); + + let order_id = _create_order(recipient_address, amount, fee, escrow); + + orders.append((order_id, recipient_address, amount)); + + idx += 1; + }; + + // check balance + assert(eth_token.balanceOf(escrow.contract_address) == 1509, 'set_order: wrong balance '); + assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'set_order: wrong balance'); + + // Call withdraw_batch l1_handler + let mut l1_handler = L1HandlerTrait::new( + contract_address: escrow.contract_address, + function_name: 'claim_payment_batch' + ); + + let mut payload_buffer: Array = ArrayTrait::new(); + Serde::serialize(@orders, ref payload_buffer); + + l1_handler.payload = payload_buffer.span(); + l1_handler.from_address = ETH_USER().into(); + + // same as "Should Panic" but for the L1 handler function + match l1_handler.execute() { + Result::Ok(_) => panic_with_felt252('shouldve panicked'), + Result::Err(RevertedTransaction) => { + assert(*RevertedTransaction.panic_data.at(0) == 'Only PAYMENT_REGISTRY_CONTRACT', *RevertedTransaction.panic_data.at(0)); + } + } + } + fn _create_order( recipient_address: EthAddress, amount: u256, diff --git a/contracts/starknet/src/tests/utils/constants.cairo b/contracts/starknet/src/tests/utils/constants.cairo index 02729ee5..7e9c638a 100644 --- a/contracts/starknet/src/tests/utils/constants.cairo +++ b/contracts/starknet/src/tests/utils/constants.cairo @@ -24,4 +24,12 @@ mod EscrowConstants { fn ETH_USER() -> EthAddress { 99.try_into().unwrap() } + + fn ETH_USER_2() -> EthAddress { + 100.try_into().unwrap() + } + + fn ETH_USER_3() -> EthAddress { + 101.try_into().unwrap() + } } From 96136fc262e7fafdfa5bf3cf2d429bef9dcf4b3d Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Mon, 25 Mar 2024 14:37:02 -0300 Subject: [PATCH 08/11] refactor: use constants for ETH_USER address --- contracts/starknet/src/tests/test_escrow_claim.cairo | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/starknet/src/tests/test_escrow_claim.cairo b/contracts/starknet/src/tests/test_escrow_claim.cairo index bfee19d3..585abf92 100644 --- a/contracts/starknet/src/tests/test_escrow_claim.cairo +++ b/contracts/starknet/src/tests/test_escrow_claim.cairo @@ -101,7 +101,7 @@ mod Escrow { assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'init: wrong balance'); start_prank(CheatTarget::One(escrow.contract_address), USER()); - let order = Order { recipient_address: 12345.try_into().unwrap(), amount: 500, fee: 0 }; + let order = Order { recipient_address: ETH_USER(), amount: 500, fee: 0 }; let order_id = escrow.set_order(order); stop_prank(CheatTarget::One(escrow.contract_address)); @@ -271,7 +271,7 @@ mod Escrow { assert(eth_token.balanceOf(escrow.contract_address) == 0, 'init: wrong balance'); assert(eth_token.balanceOf(MM_STARKNET()) == 0, 'init: wrong balance'); - let recipient_addresses = array![12345.try_into().unwrap(), 12346.try_into().unwrap(), 12347.try_into().unwrap()]; + let recipient_addresses = array![ETH_USER(), ETH_USER_2(), ETH_USER_3()]; let amounts = array![500, 501, 502]; let fees = array![3, 2, 1]; From 717aaa4cd6f2d57f042b98c83d275de0f6ef3fb3 Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Mon, 25 Mar 2024 15:12:26 -0300 Subject: [PATCH 09/11] feat: emit event on claim payment batch --- contracts/ethereum/src/PaymentRegistry.sol | 8 ++++++++ contracts/ethereum/test/Transfer_Claim_SN.t.sol | 3 +++ 2 files changed, 11 insertions(+) diff --git a/contracts/ethereum/src/PaymentRegistry.sol b/contracts/ethereum/src/PaymentRegistry.sol index 4ed7e7ec..c388f060 100644 --- a/contracts/ethereum/src/PaymentRegistry.sol +++ b/contracts/ethereum/src/PaymentRegistry.sol @@ -23,6 +23,8 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { event ModifiedStarknetEscrowAddress(uint256 newEscrowAddress); event ModifiedStarknetClaimPaymentSelector(uint256 newEscrowClaimPaymentSelector); event ModifiedStarknetClaimPaymentBatchSelector(uint256 newEscrowClaimPaymentSelector); + event ClaimPayment(uint256 orderId, uint256 destAddress, uint256 amount, Chain chainId); + event ClaimPaymentBatch(uint256[] orderIds, uint256[] destAddresses, uint256[] amounts, Chain chainId); mapping(bytes32 => TransferInfo) public transfers; address public marketMaker; @@ -93,6 +95,8 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { StarknetEscrowAddress, StarknetEscrowClaimPaymentSelector, payload); + + emit ClaimPayment(orderId, destAddress, amount, Chain.Starknet); } function claimPaymentBatch( @@ -126,6 +130,8 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { StarknetEscrowAddress, StarknetEscrowClaimPaymentBatchSelector, payload); + + emit ClaimPaymentBatch(orderIds, destAddresses, amounts, Chain.Starknet); } function _claimPaymentStarknet(uint256 orderId, uint256 destAddress, uint256 amount) internal view { @@ -161,6 +167,8 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { new bytes[](0), //factory dependencies msg.sender //refund recipient ); + + emit ClaimPayment(orderId, destAddress, amount, Chain.ZKSync); } function setStarknetEscrowAddress(uint256 newStarknetEscrowAddress) external onlyOwner { diff --git a/contracts/ethereum/test/Transfer_Claim_SN.t.sol b/contracts/ethereum/test/Transfer_Claim_SN.t.sol index 34f3143c..d7b97835 100644 --- a/contracts/ethereum/test/Transfer_Claim_SN.t.sol +++ b/contracts/ethereum/test/Transfer_Claim_SN.t.sol @@ -6,6 +6,7 @@ import "../src/PaymentRegistry.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; contract TransferTest is Test { + event ClaimPaymentBatch(uint256[] orderIds, uint256[] destAddresses, uint256[] amounts, PaymentRegistry.Chain chainId); address public deployer = makeAddr('deployer'); address public marketMaker = makeAddr("marketMaker"); @@ -107,6 +108,8 @@ contract TransferTest is Test { amounts[2] = 1; hoax(marketMaker); + vm.expectEmit(true, true, true, true); + emit ClaimPaymentBatch(orderIds, destAddresses, amounts, PaymentRegistry.Chain.Starknet); yab_caller.claimPaymentBatch(orderIds, destAddresses, amounts); assertEq(address(0x1).balance, 3); From 218253c54eaed2c51c1ab7999cbe2807514e7dd0 Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Wed, 27 Mar 2024 11:56:55 -0300 Subject: [PATCH 10/11] refactor: rename _claimPaymentStarknet -> _verifyTransferExistsStarknet to make it more descriptive --- contracts/ethereum/src/PaymentRegistry.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/ethereum/src/PaymentRegistry.sol b/contracts/ethereum/src/PaymentRegistry.sol index c388f060..7c85d1c6 100644 --- a/contracts/ethereum/src/PaymentRegistry.sol +++ b/contracts/ethereum/src/PaymentRegistry.sol @@ -82,7 +82,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { //TODO change name to claimPaymentStarknet function claimPayment(uint256 orderId, uint256 destAddress, uint256 amount) external payable onlyOwnerOrMM { - _claimPaymentStarknet(orderId, destAddress, amount); + _verifyTransferExistsStarknet(orderId, destAddress, amount); uint256[] memory payload = new uint256[](5); //TODO why array of 256 if then filled with 128? payload[0] = uint128(orderId); // low @@ -116,7 +116,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { uint256 destAddress = destAddresses[idx]; uint256 amount = amounts[idx]; - _claimPaymentStarknet(orderId, destAddress, amount); + _verifyTransferExistsStarknet(orderId, destAddress, amount); uint32 base_idx = 1 + 5 * idx; payload[base_idx] = uint128(orderId); // low @@ -134,7 +134,7 @@ contract PaymentRegistry is Initializable, OwnableUpgradeable, UUPSUpgradeable { emit ClaimPaymentBatch(orderIds, destAddresses, amounts, Chain.Starknet); } - function _claimPaymentStarknet(uint256 orderId, uint256 destAddress, uint256 amount) internal view { + function _verifyTransferExistsStarknet(uint256 orderId, uint256 destAddress, uint256 amount) internal view { bytes32 index = keccak256(abi.encodePacked(orderId, destAddress, amount, Chain.Starknet)); TransferInfo storage transferInfo = transfers[index]; require(transferInfo.isUsed == true, "Transfer not found."); From e76e5e4abfc0c95729731138ee18df718cceda40 Mon Sep 17 00:00:00 2001 From: Santos Rosati Date: Wed, 27 Mar 2024 12:02:21 -0300 Subject: [PATCH 11/11] refactor (starknet): rename tests & remove commented code --- contracts/starknet/src/tests/test_escrow_claim.cairo | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/contracts/starknet/src/tests/test_escrow_claim.cairo b/contracts/starknet/src/tests/test_escrow_claim.cairo index 585abf92..485ebe60 100644 --- a/contracts/starknet/src/tests/test_escrow_claim.cairo +++ b/contracts/starknet/src/tests/test_escrow_claim.cairo @@ -34,14 +34,6 @@ mod Escrow { setup_general(BoundedInt::max(), BoundedInt::max()) } - // fn setup_approved(approved: u256) -> (IEscrowDispatcher, IERC20Dispatcher){ - // setup_general(BoundedInt::max(), approved) - // } - - // fn setup_balance(balance: u256) -> (IEscrowDispatcher, IERC20Dispatcher){ - // setup_general(balance, BoundedInt::max()) - // } - fn setup_general(balance: u256, approved: u256) -> (IEscrowDispatcher, IERC20Dispatcher){ let eth_token = deploy_erc20('ETH', '$ETH', BoundedInt::max(), OWNER()); let escrow = deploy_escrow( @@ -93,7 +85,7 @@ mod Escrow { } #[test] - fn test_happy_path() { + fn test_claim_payment() { let (escrow, eth_token) = setup(); // check balance @@ -139,7 +131,7 @@ mod Escrow { } #[test] - fn test_claim_batch_happy() { + fn test_claim_payment_batch() { let (escrow, eth_token) = setup(); // check balance