Skip to content

Commit

Permalink
cleaned up code and applied feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@Zodomo
Zodomo committed Nov 5, 2024
1 parent ad1831b commit 563e414
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 107 deletions.
40 changes: 20 additions & 20 deletions contracts/core/.gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,23 @@ FeeOracleV2_Test:test_setManager() (gas: 45775)
FeeOracleV2_Test:test_setProtocolFee() (gas: 32226)
FeeOracleV2_Test:test_setToNativeRate() (gas: 43640)
Inbox_accept_Test:test_accept_one_request() (gas: 388610)
Inbox_accept_Test:test_accept_reverts() (gas: 1026513)
Inbox_accept_Test:test_accept_reverts() (gas: 1027273)
Inbox_accept_Test:test_accept_skip_first() (gas: 694910)
Inbox_accept_Test:test_accept_two_requests() (gas: 718480)
Inbox_cancel_Test:test_cancel_multiToken() (gas: 542138)
Inbox_cancel_Test:test_cancel_nativeMultiToken() (gas: 631074)
Inbox_cancel_Test:test_cancel_oldest_request() (gas: 686650)
Inbox_cancel_Test:test_cancel_one_request() (gas: 389923)
Inbox_cancel_Test:test_cancel_rejected_nativeMultiToken_request() (gas: 639600)
Inbox_cancel_Test:test_cancel_rejected_nativeToken_request() (gas: 397962)
Inbox_cancel_Test:test_cancel_reverts() (gas: 1052678)
Inbox_cancel_Test:test_cancel_singleToken() (gas: 421456)
Inbox_cancel_Test:test_cancel_two_requests() (gas: 697764)
Inbox_reject_Test:test_reject_nativeMultiToken() (gas: 594705)
Inbox_reject_Test:test_reject_oldest_request() (gas: 658739)
Inbox_reject_Test:test_reject_one_request() (gas: 361505)
Inbox_reject_Test:test_reject_reverts() (gas: 731046)
Inbox_reject_Test:test_reject_two_requests() (gas: 662097)
Inbox_cancel_Test:test_cancel_multiToken() (gas: 542454)
Inbox_cancel_Test:test_cancel_nativeMultiToken() (gas: 631438)
Inbox_cancel_Test:test_cancel_oldest_request() (gas: 686856)
Inbox_cancel_Test:test_cancel_one_request() (gas: 390129)
Inbox_cancel_Test:test_cancel_rejected_nativeMultiToken_request() (gas: 640518)
Inbox_cancel_Test:test_cancel_rejected_nativeToken_request() (gas: 398722)
Inbox_cancel_Test:test_cancel_reverts() (gas: 1053644)
Inbox_cancel_Test:test_cancel_singleToken() (gas: 421693)
Inbox_cancel_Test:test_cancel_two_requests() (gas: 698176)
Inbox_reject_Test:test_reject_nativeMultiToken() (gas: 595259)
Inbox_reject_Test:test_reject_oldest_request() (gas: 659293)
Inbox_reject_Test:test_reject_one_request() (gas: 362059)
Inbox_reject_Test:test_reject_reverts() (gas: 731850)
Inbox_reject_Test:test_reject_two_requests() (gas: 663205)
Inbox_request_Test:test_request_multiToken() (gas: 545679)
Inbox_request_Test:test_request_nativeMultiToken() (gas: 603853)
Inbox_request_Test:test_request_reverts() (gas: 890398)
Expand Down Expand Up @@ -140,11 +140,11 @@ OmniPortal_xsubmit_gas_Test:test_xsubmit_guzzle5_10validators_succeeds() (gas: 2
OmniPortal_xsubmit_gas_Test:test_xsubmit_guzzle5_25validators_succeeds() (gas: 3510011)
OmniPortal_xsubmit_gas_Test:test_xsubmit_guzzle5_succeeds() (gas: 948663)
Omni_Test:test_constructor() (gas: 1008119)
Outbox_fulfill_Test:test_fulfillFee() (gas: 44052)
Outbox_fulfill_Test:test_fulfill_succeeds() (gas: 274736)
Outbox_fulfill_Test:test_markFulfilled_nativeMultiToken() (gas: 1119775)
Outbox_fulfill_Test:test_markFulfilled_singleNative() (gas: 906052)
Outbox_fulfill_Test:test_markFulfilled_singleToken() (gas: 1028483)
Outbox_fulfill_Test:test_fulfillFee() (gas: 43845)
Outbox_fulfill_Test:test_fulfill_succeeds() (gas: 276427)
Outbox_fulfill_Test:test_markFulfilled_nativeMultiToken() (gas: 1125071)
Outbox_fulfill_Test:test_markFulfilled_singleNative() (gas: 911348)
Outbox_fulfill_Test:test_markFulfilled_singleToken() (gas: 1033779)
PortalRegistry_Test:test_register() (gas: 1092038)
PortalRegistry_Test:test_stub() (gas: 143)
Preinstalls_Test:test_createX_deployCreate2_succeeds() (gas: 132705)
Expand Down
88 changes: 51 additions & 37 deletions contracts/core/src/solve/Inbox.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { OwnableRoles } from "solady/src/auth/OwnableRoles.sol";
import { ReentrancyGuard } from "solady/src/utils/ReentrancyGuard.sol";
import { Initializable } from "solady/src/utils/Initializable.sol";
import { XAppBase } from "../pkg/XAppBase.sol";
import { IInbox } from "./interfaces/IInbox.sol";

import { SafeTransferLib } from "solady/src/utils/SafeTransferLib.sol";
import { IConversionRateOracle } from "../interfaces/IConversionRateOracle.sol";
import { Solve } from "./Solve.sol";
Expand All @@ -13,7 +15,7 @@ import { Solve } from "./Solve.sol";
* @title Inbox
* @notice Entrypoint and alt-mempoool for user solve requests.
*/
contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase {
contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase, IInbox {
using SafeTransferLib for address;

error NoDeposits();
Expand Down Expand Up @@ -41,10 +43,11 @@ contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase {

/**
* @notice Emitted when a request is rejected.
* @param id ID of the request.
* @param by Address of the solver who rejected the request.
* @param id ID of the request.
* @param by Address of the solver who rejected the request.
* @param reason Reason for rejecting the request.
*/
event Rejected(bytes32 indexed id, address indexed by);
event Rejected(bytes32 indexed id, address indexed by, Solve.RejectReason indexed reason);

/**
* @notice Emitted when a request is cancelled.
Expand All @@ -54,11 +57,17 @@ contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase {

/**
* @notice Emitted when a request is fulfilled.
* @param guid ID of the request.
* @param id ID of the request.
* @param callHash Hash of the call executed on another chain.
* @param creditedTo Address of the recipient credited the funds by the solver.
*/
event Fulfilled(bytes32 indexed guid, bytes32 indexed callHash, address indexed creditedTo);
event Fulfilled(bytes32 indexed id, bytes32 indexed callHash, address indexed creditedTo);

/**
* @notice Emitted when a request is claimed.
* @param id ID of the request.
*/
event Claimed(bytes32 indexed id);

/**
* @notice Role for solvers.
Expand Down Expand Up @@ -105,7 +114,7 @@ contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase {
* @notice Suggest the amount of native currency to send with a request.
* @param call Details of the call to be executed on another chain.
* @param gasLimit Maximum gas limit for the call.
* @param gasPrice Gas price in wei.
* @param gasPrice Destination chain gas price in wei.
* @param fulfillFee Fee for the fulfill call, retrieved from the destination outbox.
*/
function suggestNativePayment(Solve.Call calldata call, uint64 gasLimit, uint64 gasPrice, uint256 fulfillFee)
Expand Down Expand Up @@ -168,14 +177,14 @@ contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase {
* @dev Only a whitelisted solver can reject.
* @param id ID of the request.
*/
function reject(bytes32 id) external onlyRoles(SOLVER) nonReentrant {
function reject(bytes32 id, Solve.RejectReason reason) external onlyRoles(SOLVER) nonReentrant {
Solve.Request storage req = _requests[id];
if (req.status != Solve.Status.Pending) revert RequestStateInvalid();

req.updatedAt = uint40(block.timestamp);
req.status = Solve.Status.Rejected;

emit Rejected(id, msg.sender);
emit Rejected(id, msg.sender, reason);
}

/**
Expand All @@ -188,7 +197,10 @@ contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase {
if (req.status != Solve.Status.Pending && req.status != Solve.Status.Rejected) revert RequestStateInvalid();
if (req.from != msg.sender) revert Unauthorized();

_cancelRequest(req);
req.updatedAt = uint40(block.timestamp);
req.status = Solve.Status.Reverted;

_transferDeposits(req.from, req.deposits);

emit Reverted(id);
}
Expand All @@ -197,34 +209,54 @@ contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase {
* @notice Fulfill a request.
* @dev Only callable by the outbox.
*/
function markFulfilled(bytes32 guid, bytes32 callHash, address creditTo) external xrecv nonReentrant {
function markFulfilled(bytes32 id, bytes32 callHash, address creditTo) external xrecv nonReentrant {
// Validate request state and fulfillment context
Solve.Request storage req = _requests[guid];
Solve.Request storage req = _requests[id];
if (req.status != Solve.Status.Accepted) revert RequestStateInvalid();
if (req.call.destChainId != xmsg.sourceChainId) revert IncorrectChain();
if (xmsg.sender != _outbox) revert Unauthorized();
// No need to check if msg.sender is OmniPortal as we use xrecv for source validation

// Validate call hash, ensuring the correct action is being fulfilled
bytes32 _callHash = keccak256(abi.encode(guid, req.call));
bytes32 _callHash = keccak256(abi.encode(id, block.chainid, req.call));
if (_callHash != callHash) revert InvalidCall();

// Update request state
req.updatedAt = uint40(block.timestamp);
req.status = Solve.Status.Fulfilled;
req.acceptedBy = creditTo;

emit Fulfilled(id, callHash, creditTo);
}

/**
* @notice Claim a fulfilled request.
* @param id ID of the request.
*/
function claim(bytes32 id) external nonReentrant {
Solve.Request storage req = _requests[id];
if (req.status != Solve.Status.Fulfilled) revert RequestStateInvalid();

// Transfer deposits to solver
Solve.Deposit[] memory deposits = req.deposits;
req.updatedAt = uint40(block.timestamp);
req.status = Solve.Status.Claimed;

_transferDeposits(req.acceptedBy, req.deposits);

emit Claimed(id);
}

/**
* @dev Transfer deposits to recipient. Used regardless of refund or claim.
*/
function _transferDeposits(address recipient, Solve.Deposit[] memory deposits) internal {
for (uint256 i; i < deposits.length; ++i) {
if (deposits[i].isNative) {
(bool success,) = payable(creditTo).call{ value: deposits[i].amount }("");
(bool success,) = payable(recipient).call{ value: deposits[i].amount }("");
if (!success) revert TransferFailed();
} else {
deposits[i].token.safeTransfer(creditTo, deposits[i].amount);
deposits[i].token.safeTransfer(recipient, deposits[i].amount);
}
}

emit Fulfilled(guid, callHash, creditTo);
}

/**
Expand Down Expand Up @@ -261,24 +293,6 @@ contract Inbox is OwnableRoles, ReentrancyGuard, Initializable, XAppBase {
}
}

function _cancelRequest(Solve.Request storage req) internal {
// Update state
req.updatedAt = uint40(block.timestamp);
req.status = Solve.Status.Reverted;

// Refund deposits
uint256 length = req.deposits.length;
for (uint256 i = 0; i < length; i++) {
Solve.Deposit memory deposit = req.deposits[i];
if (deposit.isNative) {
(bool success,) = payable(msg.sender).call{ value: deposit.amount }("");
if (!success) revert TransferFailed();
} else {
deposit.token.safeTransfer(msg.sender, deposit.amount);
}
}
}

/**
* @dev Increment and return _lastId.
*/
Expand Down
Loading

0 comments on commit 563e414

Please sign in to comment.