Skip to content

Commit

Permalink
chore(contracts/solve): no enum reject reason (#2710)
Browse files Browse the repository at this point in the history
Switching to uint8 rather than enum, so we can add revert reasons
without
upgrading contracts.


issue: none
  • Loading branch information
kevinhalliday authored Dec 17, 2024
1 parent dd15937 commit d419b57
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 40 deletions.
4 changes: 2 additions & 2 deletions contracts/bindings/solveinbox.go

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions contracts/solve/.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
SolveInbox_accept_Test:test_accept_one_request() (gas: 449411)
SolveInbox_accept_Test:test_accept_reverts() (gas: 1135393)
SolveInbox_accept_Test:test_accept_reverts() (gas: 1135308)
SolveInbox_accept_Test:test_accept_skip_first() (gas: 757442)
SolveInbox_accept_Test:test_accept_two_requests() (gas: 781616)
SolveInbox_cancel_Test:test_cancel_multiToken() (gas: 604009)
SolveInbox_cancel_Test:test_cancel_nativeMultiToken() (gas: 694160)
SolveInbox_cancel_Test:test_cancel_oldest_request() (gas: 758515)
SolveInbox_cancel_Test:test_cancel_one_request() (gas: 450572)
SolveInbox_cancel_Test:test_cancel_rejected_nativeMultiToken_request() (gas: 737998)
SolveInbox_cancel_Test:test_cancel_rejected_nativeToken_request() (gas: 491776)
SolveInbox_cancel_Test:test_cancel_reverts() (gas: 1157381)
SolveInbox_cancel_Test:test_cancel_rejected_nativeMultiToken_request() (gas: 737913)
SolveInbox_cancel_Test:test_cancel_rejected_nativeToken_request() (gas: 491691)
SolveInbox_cancel_Test:test_cancel_reverts() (gas: 1157296)
SolveInbox_cancel_Test:test_cancel_singleToken() (gas: 482102)
SolveInbox_cancel_Test:test_cancel_two_requests() (gas: 760517)
SolveInbox_claim_Test:test_claim_multiDeposit() (gas: 819751)
Expand All @@ -17,11 +17,11 @@ SolveInbox_claim_Test:test_claim_singleNative() (gas: 569792)
SolveInbox_claim_Test:test_claim_singleToken() (gas: 600715)
SolveInbox_markFulfilled_Test:test_markFulfilled_reverts() (gas: 551134)
SolveInbox_markFulfilled_Test:test_markFulfilled_success() (gas: 493564)
SolveInbox_reject_Test:test_reject_nativeMultiToken() (gas: 657800)
SolveInbox_reject_Test:test_reject_oldest_request() (gas: 730796)
SolveInbox_reject_Test:test_reject_one_request() (gas: 422357)
SolveInbox_reject_Test:test_reject_reverts() (gas: 815736)
SolveInbox_reject_Test:test_reject_two_requests() (gas: 725261)
SolveInbox_reject_Test:test_reject_nativeMultiToken() (gas: 657715)
SolveInbox_reject_Test:test_reject_oldest_request() (gas: 730711)
SolveInbox_reject_Test:test_reject_one_request() (gas: 422272)
SolveInbox_reject_Test:test_reject_reverts() (gas: 815464)
SolveInbox_reject_Test:test_reject_two_requests() (gas: 725091)
SolveInbox_request_Test:test_request_multiToken() (gas: 584500)
SolveInbox_request_Test:test_request_nativeMultiToken() (gas: 643788)
SolveInbox_request_Test:test_request_reverts() (gas: 1018635)
Expand Down
11 changes: 0 additions & 11 deletions contracts/solve/src/Solve.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,6 @@ library Solve {
Claimed
}

/**
* @notice Reason for rejecting a request.
*/
enum RejectReason {
None,
DestCallReverts,
InsufficientFee,
InsufficientInventory,
InvalidTarget
}

/**
* @notice A request to execute a call on another chain, backed by a deposit.
* @param id ID for the request, globally unique per inbox.
Expand Down
3 changes: 2 additions & 1 deletion contracts/solve/src/SolveInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,9 @@ contract SolveInbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase, I
* @notice Reject an open request.
* @dev Only a whitelisted solver can reject.
* @param id ID of the request.
* @param reason Reason code for rejecting the request (see solver/app/reject.go for current codes)
*/
function reject(bytes32 id, Solve.RejectReason reason) external onlyRoles(SOLVER) nonReentrant {
function reject(bytes32 id, uint8 reason) external onlyRoles(SOLVER) nonReentrant {
Solve.Request storage req = _requests[id];
if (req.status != Solve.Status.Pending) revert NotPending();

Expand Down
7 changes: 4 additions & 3 deletions contracts/solve/src/interfaces/ISolveInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ interface ISolveInbox {
* @notice Emitted when a request is rejected.
* @param id ID of the request.
* @param by Address of the solver who rejected the request.
* @param reason Reason for rejecting the request.
* @param reason Reason code for rejecting the request (see solver/app/reject.go for current codes)
*/
event Rejected(bytes32 indexed id, address indexed by, Solve.RejectReason indexed reason);
event Rejected(bytes32 indexed id, address indexed by, uint8 indexed reason);

/**
* @notice Emitted when a request is cancelled.
Expand Down Expand Up @@ -83,8 +83,9 @@ interface ISolveInbox {
* @notice Reject an open request.
* @dev Only a whitelisted solver can reject.
* @param id ID of the request.
* @param reason Reason code for rejecting the request (see solver/app/reject.go for current codes)
*/
function reject(bytes32 id, Solve.RejectReason reason) external;
function reject(bytes32 id, uint8 reason) external;

/**
* @notice Cancel an open or rejected request and refund deposits.
Expand Down
2 changes: 1 addition & 1 deletion contracts/solve/test/Inbox_accept.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract SolveInbox_accept_Test is InboxBase {

// cannot accept rejected request
vm.startPrank(solver);
inbox.reject(id, Solve.RejectReason.None);
inbox.reject({ id: id, reason: 0 });
vm.expectRevert(SolveInbox.NotPending.selector);
inbox.accept(id);
vm.stopPrank();
Expand Down
6 changes: 3 additions & 3 deletions contracts/solve/test/Inbox_cancel.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract SolveInbox_cancel_Test is InboxBase {

// cannot double cancel rejected request
vm.prank(solver);
inbox.reject(id, Solve.RejectReason.None);
inbox.reject({ id: id, reason: 0 });
vm.startPrank(user);
inbox.cancel(id);
vm.expectRevert(SolveInbox.NotPendingOrRejected.selector);
Expand Down Expand Up @@ -230,7 +230,7 @@ contract SolveInbox_cancel_Test is InboxBase {

// reject request
vm.prank(solver);
inbox.reject(id, Solve.RejectReason.None);
inbox.reject({ id: id, reason: 0 });

// cancel rejected request
vm.prank(user);
Expand Down Expand Up @@ -265,7 +265,7 @@ contract SolveInbox_cancel_Test is InboxBase {

// reject request
vm.prank(solver);
inbox.reject(id, Solve.RejectReason.None);
inbox.reject({ id: id, reason: 0 });

// cancel rejected request
vm.prank(user);
Expand Down
18 changes: 9 additions & 9 deletions contracts/solve/test/Inbox_reject.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ contract SolveInbox_reject_Test is InboxBase {
// cannot reject non-existent request
vm.prank(solver);
vm.expectRevert(SolveInbox.NotPending.selector);
inbox.reject(bytes32(0), Solve.RejectReason.None);
inbox.reject({ id: bytes32(0), reason: 0 });

// needs to have solver role
vm.expectRevert(Ownable.Unauthorized.selector);
inbox.reject(bytes32(0), Solve.RejectReason.None);
inbox.reject({ id: bytes32(0), reason: 0 });

// create request to cancel before rejecting
vm.deal(user, 1 ether);
Expand All @@ -33,7 +33,7 @@ contract SolveInbox_reject_Test is InboxBase {
// cannot reject cancelled request
vm.prank(solver);
vm.expectRevert(SolveInbox.NotPending.selector);
inbox.reject(id, Solve.RejectReason.None);
inbox.reject({ id: id, reason: 0 });

// create request to accept before rejecting
vm.deal(user, 1 ether);
Expand All @@ -44,7 +44,7 @@ contract SolveInbox_reject_Test is InboxBase {
vm.startPrank(solver);
inbox.accept(id);
vm.expectRevert(SolveInbox.NotPending.selector);
inbox.reject(id, Solve.RejectReason.None);
inbox.reject({ id: id, reason: 0 });
vm.stopPrank();
}

Expand All @@ -58,7 +58,7 @@ contract SolveInbox_reject_Test is InboxBase {

// reject request
vm.prank(solver);
inbox.reject(id, Solve.RejectReason.None);
inbox.reject({ id: id, reason: 0 });

assertEq(address(inbox).balance, 1 ether, "address(inbox).balance");
assertEq(address(user).balance, 0, "address(user).balance");
Expand All @@ -82,8 +82,8 @@ contract SolveInbox_reject_Test is InboxBase {

// reject both requests
vm.startPrank(solver);
inbox.reject(id1, Solve.RejectReason.None);
inbox.reject(id2, Solve.RejectReason.None);
inbox.reject({ id: id1, reason: 0 });
inbox.reject({ id: id2, reason: 0 });
vm.stopPrank();

assertEq(address(inbox).balance, 2 ether, "address(inbox).balance");
Expand All @@ -109,7 +109,7 @@ contract SolveInbox_reject_Test is InboxBase {

// reject oldest request
vm.startPrank(solver);
inbox.reject(id1, Solve.RejectReason.None);
inbox.reject({ id: id1, reason: 0 });
vm.stopPrank();

assertEq(address(inbox).balance, 2 ether, "address(inbox).balance");
Expand Down Expand Up @@ -142,7 +142,7 @@ contract SolveInbox_reject_Test is InboxBase {

// reject request
vm.prank(solver);
inbox.reject(id, Solve.RejectReason.None);
inbox.reject({ id: id, reason: 0 });

assertEq(address(inbox).balance, 1 ether, "address(inbox).balance");
assertEq(address(user).balance, 0, "address(user).balance");
Expand Down

0 comments on commit d419b57

Please sign in to comment.