Skip to content

Commit

Permalink
feat(contracts/solve): modify TokenExpense handling for transfers (#2860
Browse files Browse the repository at this point in the history
)

Allowing the `spender` field in `TokenExpense` to be `bytes32(0)`
enables us to treat that like a transfer edge case so direct transfers
can be executed without an unnecessary approval.

issue: omni-network/ops#740
  • Loading branch information
Zodomo authored Jan 22, 2025
1 parent e0d86fc commit 05338f7
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 54 deletions.
2 changes: 1 addition & 1 deletion contracts/bindings/solvernetinbox.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/bindings/solvernetoutbox.go

Large diffs are not rendered by default.

77 changes: 39 additions & 38 deletions contracts/solve/.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -31,46 +31,47 @@ SolveInbox_request_Test:test_request_two() (gas: 816192)
SolveOutbox_fulfill_test:test_fulfillFee() (gas: 27996)
SolveOutbox_fulfill_test:test_fulfill_reverts() (gas: 673377)
SolveOutbox_fulfill_test:test_fulfill_succeeds() (gas: 274856)
SolverNet_E2E_Test:test_e2e_complete_order() (gas: 1655587)
SolverNet_E2E_Test:test_mixed_deposits_and_expenses() (gas: 1856906)
SolverNet_E2E_Test:test_multi_token_deposits_and_expenses() (gas: 2074947)
SolverNet_Inbox_Accept_Test:test_accept_reverts_not_solver() (gas: 996853)
SolverNet_Inbox_Accept_Test:test_accept_reverts_order_not_pending() (gas: 1049752)
SolverNet_Inbox_Accept_Test:test_accept_succeeds() (gas: 1119341)
SolverNet_Inbox_Cancel_Test:test_cancel_reverts_not_user() (gas: 1004077)
SolverNet_Inbox_Cancel_Test:test_cancel_reverts_order_not_pending_or_rejected() (gas: 1061411)
SolverNet_Inbox_Cancel_Test:test_cancel_succeeds() (gas: 1124026)
SolverNet_Inbox_Claim_Test:test_claim_native_succeeds() (gas: 1221823)
SolverNet_Inbox_Claim_Test:test_claim_reverts_not_solver() (gas: 1136287)
SolverNet_Inbox_Claim_Test:test_claim_reverts_order_not_filled() (gas: 1056015)
SolverNet_Inbox_Claim_Test:test_claim_reverts_zero_recipient() (gas: 1181981)
SolverNet_Inbox_Claim_Test:test_claim_succeeds() (gas: 1276031)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_reverts_not_outbox() (gas: 1094837)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_reverts_order_not_accepted() (gas: 1042640)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_reverts_wrong_fill_hash() (gas: 1097442)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_reverts_wrong_source_chain() (gas: 1097323)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_succeeds() (gas: 1198658)
SolverNet_Inbox_Open_Test:test_open_reverts_insufficient_erc20_allowance() (gas: 106389)
SolverNet_E2E_Test:test_e2e_complete_contract_order() (gas: 1656479)
SolverNet_E2E_Test:test_e2e_complete_token_transfer_order() (gas: 1414836)
SolverNet_E2E_Test:test_mixed_deposits_and_expenses() (gas: 1856966)
SolverNet_E2E_Test:test_multi_token_deposits_and_expenses() (gas: 2074968)
SolverNet_Inbox_Accept_Test:test_accept_reverts_not_solver() (gas: 996775)
SolverNet_Inbox_Accept_Test:test_accept_reverts_order_not_pending() (gas: 1049674)
SolverNet_Inbox_Accept_Test:test_accept_succeeds() (gas: 1119263)
SolverNet_Inbox_Cancel_Test:test_cancel_reverts_not_user() (gas: 1003999)
SolverNet_Inbox_Cancel_Test:test_cancel_reverts_order_not_pending_or_rejected() (gas: 1061333)
SolverNet_Inbox_Cancel_Test:test_cancel_succeeds() (gas: 1123948)
SolverNet_Inbox_Claim_Test:test_claim_native_succeeds() (gas: 1221745)
SolverNet_Inbox_Claim_Test:test_claim_reverts_not_solver() (gas: 1136209)
SolverNet_Inbox_Claim_Test:test_claim_reverts_order_not_filled() (gas: 1055937)
SolverNet_Inbox_Claim_Test:test_claim_reverts_zero_recipient() (gas: 1181903)
SolverNet_Inbox_Claim_Test:test_claim_succeeds() (gas: 1275953)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_reverts_not_outbox() (gas: 1094759)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_reverts_order_not_accepted() (gas: 1042562)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_reverts_wrong_fill_hash() (gas: 1097364)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_reverts_wrong_source_chain() (gas: 1097245)
SolverNet_Inbox_MarkFulfilled_Test:test_mark_fulfilled_succeeds() (gas: 1198580)
SolverNet_Inbox_Open_Test:test_open_reverts_insufficient_erc20_allowance() (gas: 106363)
SolverNet_Inbox_Open_Test:test_open_reverts_multiple_native_deposits() (gas: 58248)
SolverNet_Inbox_Open_Test:test_open_reverts_native_deposit_without_value() (gas: 51211)
SolverNet_Inbox_Open_Test:test_open_reverts_native_deposit_wrong_value() (gas: 57861)
SolverNet_Inbox_Open_Test:test_open_reverts_value_without_native_deposit() (gas: 162202)
SolverNet_Inbox_Open_Test:test_open_reverts_native_deposit_without_value() (gas: 51185)
SolverNet_Inbox_Open_Test:test_open_reverts_native_deposit_wrong_value() (gas: 57835)
SolverNet_Inbox_Open_Test:test_open_reverts_value_without_native_deposit() (gas: 162176)
SolverNet_Inbox_Open_Test:test_open_reverts_zero_amount_erc20_deposit() (gas: 52697)
SolverNet_Inbox_Open_Test:test_open_succeeds() (gas: 1072369)
SolverNet_Inbox_Reject_Test:test_reject_native_refund() (gas: 1082939)
SolverNet_Inbox_Reject_Test:test_reject_reverts_not_solver() (gas: 997056)
SolverNet_Inbox_Reject_Test:test_reject_reverts_order_not_pending() (gas: 1066777)
SolverNet_Inbox_Reject_Test:test_reject_succeeds() (gas: 1128847)
SolverNet_Inbox_Resolve_Test:test_resolveOrder_erc20_deposit_succeeds() (gas: 79352)
SolverNet_Inbox_Resolve_Test:test_resolveOrder_native_deposit_succeeds() (gas: 77263)
SolverNet_Inbox_Validate_Test:test_validate_reverts() (gas: 186770)
SolverNet_Inbox_Validate_Test:test_validate_succeeds() (gas: 31369)
SolverNet_Inbox_Open_Test:test_open_succeeds() (gas: 1072291)
SolverNet_Inbox_Reject_Test:test_reject_native_refund() (gas: 1082861)
SolverNet_Inbox_Reject_Test:test_reject_reverts_not_solver() (gas: 996978)
SolverNet_Inbox_Reject_Test:test_reject_reverts_order_not_pending() (gas: 1066699)
SolverNet_Inbox_Reject_Test:test_reject_succeeds() (gas: 1128769)
SolverNet_Inbox_Resolve_Test:test_resolveOrder_erc20_deposit_succeeds() (gas: 79326)
SolverNet_Inbox_Resolve_Test:test_resolveOrder_native_deposit_succeeds() (gas: 77237)
SolverNet_Inbox_Validate_Test:test_validate_reverts() (gas: 168204)
SolverNet_Inbox_Validate_Test:test_validate_succeeds() (gas: 31343)
SolverNet_Outbox_Executor_Test:test_all_reverts_not_outbox() (gas: 14265)
SolverNet_Outbox_Executor_Test:test_execute_reverts_call_failed() (gas: 20331)
SolverNet_Outbox_Fill_Test:test_fill_call_refund() (gas: 266808)
SolverNet_Outbox_Fill_Test:test_fill_overpayment_refund() (gas: 342901)
SolverNet_Outbox_Fill_Test:test_fill_reverts_already_filled() (gas: 420837)
SolverNet_Outbox_Fill_Test:test_fill_reverts_insufficient_fee() (gas: 405923)
SolverNet_Outbox_Fill_Test:test_fill_call_refund() (gas: 266782)
SolverNet_Outbox_Fill_Test:test_fill_overpayment_refund() (gas: 342913)
SolverNet_Outbox_Fill_Test:test_fill_reverts_already_filled() (gas: 420887)
SolverNet_Outbox_Fill_Test:test_fill_reverts_insufficient_fee() (gas: 405964)
SolverNet_Outbox_Fill_Test:test_fill_reverts_invalid_origin_data() (gas: 78579)
SolverNet_Outbox_Fill_Test:test_fill_reverts_wrong_chain() (gas: 258047)
SolverNet_Outbox_Fill_Test:test_fill_succeeds() (gas: 347706)
SolverNet_Outbox_Fill_Test:test_fill_reverts_wrong_chain() (gas: 258059)
SolverNet_Outbox_Fill_Test:test_fill_succeeds() (gas: 347718)
1 change: 0 additions & 1 deletion contracts/solve/src/ERC7683/SolverNetInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ contract SolverNetInbox is OwnableRoles, ReentrancyGuard, Initializable, Deploye
for (uint256 i; i < call.expenses.length; ++i) {
TokenExpense memory expense = call.expenses[i];
if (expense.token == bytes32(0)) revert NoExpenseToken();
if (expense.spender == bytes32(0)) revert NoExpenseSender();
if (expense.amount == 0) revert NoExpenseAmount();
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/solve/src/ERC7683/SolverNetOutbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ contract SolverNetOutbox is OwnableRoles, ReentrancyGuard, Initializable, Deploy

token.safeTransferFrom(msg.sender, address(_executor), expense.amount);
// We remotely set token approvals on executor so we don't need to reprocess Call expenses there.
_executor.approve(token, spender, expense.amount);
if (spender != address(0)) _executor.approve(token, spender, expense.amount);
}

_;
Expand All @@ -149,7 +149,7 @@ contract SolverNetOutbox is OwnableRoles, ReentrancyGuard, Initializable, Deploy
uint256 tokenBalance = token.balanceOf(address(_executor));

if (tokenBalance > 0) {
_executor.approve(token, expense.spender.toAddress(), 0);
if (expense.spender != bytes32(0)) _executor.approve(token, expense.spender.toAddress(), 0);
_executor.transfer(token, msg.sender, tokenBalance);
}
}
Expand Down
110 changes: 107 additions & 3 deletions contracts/solve/test/ERC7683/E2E.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ pragma solidity =0.8.24;
import "./TestBase.sol";

contract SolverNet_E2E_Test is TestBase {
function test_e2e_complete_order() public {
using AddrUtils for address;

function test_e2e_complete_contract_order() public {
// Prep: Set chainId to srcChainId
vm.chainId(srcChainId);

// 0. Generate order, validate it, resolve it, and prepare deposit tokens
IERC7683.OnchainCrossChainOrder memory order = randOrder();
assertTrue(inbox.validate(order));
assertTrue(inbox.validate(order), "order should be valid");
IERC7683.ResolvedCrossChainOrder memory resolvedOrder = inbox.resolve(order);
fundUser(resolvedOrder.minReceived);

Expand Down Expand Up @@ -41,7 +43,10 @@ contract SolverNet_E2E_Test is TestBase {
outbox.fill{ value: fillFee }(resolvedOrder.orderId, resolvedOrder.fillInstructions[0].originData, bytes(""));

assertVaultDeposit(resolvedOrder.orderId);
assertTrue(outbox.didFill(resolvedOrder.orderId, resolvedOrder.fillInstructions[0].originData));
assertTrue(
outbox.didFill(resolvedOrder.orderId, resolvedOrder.fillInstructions[0].originData),
"order should be filled"
);

// Prep: Set chainId back to srcChainId
vm.chainId(srcChainId);
Expand All @@ -64,6 +69,105 @@ contract SolverNet_E2E_Test is TestBase {
assertClaimedOrder(resolvedOrder.orderId);
}

function test_e2e_complete_token_transfer_order() public {
// Prep: Set chainId to srcChainId
vm.chainId(srcChainId);

// 0. Generate order, validate it, resolve it, and prepare deposit tokens
uint256 rand = vm.randomUint(1, 1000);
ISolverNet.Deposit[] memory deposit = new ISolverNet.Deposit[](1);
deposit[0] = ISolverNet.Deposit({ token: address(token1).toBytes32(), amount: rand * 1 ether });

ISolverNet.TokenExpense[] memory expense = new ISolverNet.TokenExpense[](1);
expense[0] =
ISolverNet.TokenExpense({ token: address(token2).toBytes32(), spender: bytes32(0), amount: rand * 1 ether });

ISolverNet.Call memory call = ISolverNet.Call({
chainId: destChainId,
target: address(token2).toBytes32(),
value: 0,
data: abi.encodeCall(ERC20.transfer, (user, rand * 1 ether)),
expenses: expense
});

ISolverNet.OrderData memory orderData = ISolverNet.OrderData({ call: call, deposits: deposit });

IERC7683.OnchainCrossChainOrder memory order = IERC7683.OnchainCrossChainOrder({
fillDeadline: uint32(block.timestamp + 1 minutes),
orderDataType: ORDER_DATA_TYPEHASH,
orderData: abi.encode(orderData)
});

assertTrue(inbox.validate(order), "order should be valid");
IERC7683.ResolvedCrossChainOrder memory resolvedOrder = inbox.resolve(order);

fundUser(resolvedOrder.minReceived);

// 1. Open order on srcChain
vm.prank(user);
inbox.open(order);

assertEq(
inbox.getLatestOrderIdByStatus(ISolverNetInbox.Status.Pending),
resolvedOrder.orderId,
"order should be opened"
);
assertEq(
token1.balanceOf(address(inbox)),
resolvedOrder.minReceived[0].amount,
"token1 should be deposited into inbox"
);

// 2. Accept order on srcChain
vm.prank(solver);
inbox.accept(resolvedOrder.orderId);

// Prep: Set chainId to destChainId and give solver some funds
vm.chainId(destChainId);
fundSolver(resolvedOrder.maxSpent);

// 3. Fill order on destChain
uint256 fillFee = outbox.fillFee(srcChainId);
bytes32 fillHash = fillHash(resolvedOrder.orderId, resolvedOrder.fillInstructions[0].originData);
vm.expectEmit(true, true, true, true);
emit ISolverNetOutbox.Filled(resolvedOrder.orderId, fillHash, solver);
vm.prank(solver);
outbox.fill{ value: fillFee }(resolvedOrder.orderId, resolvedOrder.fillInstructions[0].originData, bytes(""));

assertTrue(
outbox.didFill(resolvedOrder.orderId, resolvedOrder.fillInstructions[0].originData),
"order should be filled"
);

// Prep: Set chainId back to srcChainId
vm.chainId(srcChainId);

// 4. Mock markFulfilled call from destChain to srcChain
portal.mockXCall(
destChainId,
address(outbox),
address(inbox),
abi.encodeCall(ISolverNetInbox.markFilled, (resolvedOrder.orderId, fillHash)),
100_000
);

// 5. Claim order deposits on srcChain as solver
vm.prank(solver);
inbox.claim(resolvedOrder.orderId, solver);

assertEq(
inbox.getLatestOrderIdByStatus(ISolverNetInbox.Status.Claimed),
resolvedOrder.orderId,
"order should be claimed"
);
assertEq(token1.balanceOf(address(inbox)), 0, "token1 should be claimed from inbox");
assertEq(
token1.balanceOf(address(solver)), resolvedOrder.minReceived[0].amount, "token1 should be claimed by solver"
);
assertEq(token2.balanceOf(address(user)), resolvedOrder.maxSpent[0].amount, "token2 should be received by user");
assertEq(token2.balanceOf(address(solver)), 0, "token2 should be sent by solver");
}

function test_multi_token_deposits_and_expenses() public {
// Prep: Set chainId to srcChainId
vm.chainId(srcChainId);
Expand Down
1 change: 1 addition & 0 deletions contracts/solve/test/ERC7683/TestBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/trans
import { SolverNetInbox } from "src/ERC7683/SolverNetInbox.sol";
import { SolverNetOutbox } from "src/ERC7683/SolverNetOutbox.sol";

import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import { MockToken } from "test/utils/MockToken.sol";
import { MockVault } from "test/utils/MockVault.sol";
import { MockMultiTokenVault } from "test/utils/MockMultiTokenVault.sol";
Expand Down
8 changes: 0 additions & 8 deletions contracts/solve/test/ERC7683/inbox/Validate.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,6 @@ contract SolverNet_Inbox_Validate_Test is TestBase {
destChainId, address(vault).toBytes32(), 0, getVaultCalldata(user, rand * 1 ether), expenses, deposits
);

// `call.expenses[0].spender` cannot be the zero address
vm.expectRevert(ISolverNetInbox.NoExpenseSender.selector);
inbox.validate(order);
expenses[0].spender = address(vault).toBytes32();
order.orderData = getOrderDataBytes(
destChainId, address(vault).toBytes32(), 0, getVaultCalldata(user, rand * 1 ether), expenses, deposits
);

// `call.expenses[0].amount` cannot be 0
vm.expectRevert(ISolverNetInbox.NoExpenseAmount.selector);
inbox.validate(order);
Expand Down

0 comments on commit 05338f7

Please sign in to comment.