Skip to content

fix: add consistency of orderData #195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rymcol wants to merge 1 commit into from
Closed

fix: add consistency of orderData #195

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 32 additions & 15 deletions contracts/src/7683/T1ERC7683.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ import { IT1XChainReader } from "../libraries/xChain/IT1XChainReader.sol";
contract T1ERC7683 is IT1ERC7683, T1Permit2, AccessControlUpgradeable, EIP712 {
using SafeERC20 for IERC20;

/// Custom Errors
error InvalidSender(bytes32 provided, address expected);
error InvalidFillDeadline(uint32 provided, uint32 expected);
Comment on lines +30 to +32

@evchip evchip Sep 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

natspec comments please


/// @notice Role for pausing/unpausing open operations
bytes32 public constant OPEN_PAUSER_ROLE = keccak256("OPEN_PAUSER_ROLE");
/// @notice Role for pausing/unpausing settlement operations
Expand Down Expand Up @@ -108,7 +112,12 @@ contract T1ERC7683 is IT1ERC7683, T1Permit2, AccessControlUpgradeable, EIP712 {
function open(OnchainCrossChainOrder calldata _order) external payable override whenOpenNotPaused {
(ResolvedCrossChainOrder memory resolvedOrder, bytes32 orderId, uint256 nonce) = _resolveOrder(_order);

openOrders[orderId] = abi.encode(_order.orderDataType, _order.orderData);
// Encode a modified OrderData instead of _order.orderData to keep context clean across instances
OrderData memory orderData = OrderEncoder.decode(_order.orderData);
orderData.fillDeadline = _order.fillDeadline; // Use the resolved fillDeadline

@evchip evchip Sep 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't the resolved fillDeadline. that would be resolvedOrder.fillDeadline. Is this intentional?

orderData.sender = TypeCasts.addressToBytes32(msg.sender); // Use the resolved sender

@evchip evchip Sep 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't the resolved sender. that would be resolvedOrder.user


openOrders[orderId] = abi.encode(_order.orderDataType, orderData); // must not use _orderData for consistency
Comment on lines +115 to +120

@evchip evchip Sep 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this level of inline comments feels excessive. I feel that the code is fairly clear already.

orderStatus[orderId] = Status.OPENED;
_useNonce(msg.sender, nonce);

Expand Down Expand Up @@ -137,19 +146,19 @@ contract T1ERC7683 is IT1ERC7683, T1Permit2, AccessControlUpgradeable, EIP712 {
GaslessCrossChainOrder calldata _order,
bytes calldata _signature,
bytes calldata _originFillerData
)
external
override
whenOpenNotPaused
{
) external override whenOpenNotPaused {
if (block.timestamp > _order.openDeadline) revert OrderOpenExpired();
if (_order.originSettler != address(this)) revert InvalidGaslessOrderSettler();
if (_order.originChainId != localDomain) revert InvalidGaslessOrderOrigin();

(ResolvedCrossChainOrder memory resolvedOrder, bytes32 orderId, uint256 nonce) =
_resolveOrder(_order, _originFillerData);

openOrders[orderId] = abi.encode(_order.orderDataType, _order.orderData);
// Encode the modified OrderData (orderDataResolved) instead of orderDataOriginal
OrderData memory orderData = OrderEncoder.decode(_order.orderData);
orderData.fillDeadline = resolvedOrder.fillDeadline; // Use the resolved fillDeadline
orderData.sender = TypeCasts.addressToBytes32(_order.user); // Use the resolved sender
openOrders[orderId] = abi.encode(_order.orderDataType, OrderEncoder.encode(orderData));
Comment on lines +157 to +161

@evchip evchip Sep 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments here

orderStatus[orderId] = Status.OPENED;
_useNonce(_order.user, nonce);

Expand Down Expand Up @@ -197,14 +206,15 @@ contract T1ERC7683 is IT1ERC7683, T1Permit2, AccessControlUpgradeable, EIP712 {
function _resolveOrder(
GaslessCrossChainOrder memory _order,
bytes calldata
)
internal
view
returns (ResolvedCrossChainOrder memory, bytes32, uint256)
{
return _resolvedOrder(
_order.orderDataType, _order.user, _order.openDeadline, _order.fillDeadline, _order.orderData
);
) internal view returns (ResolvedCrossChainOrder memory, bytes32, uint256) {
OrderData memory orderData = OrderEncoder.decode(_order.orderData);
if (orderData.sender != TypeCasts.addressToBytes32(_order.user)) {
revert InvalidSender(orderData.sender, _order.user);
}
if (orderData.fillDeadline != _order.fillDeadline) {
revert InvalidFillDeadline(orderData.fillDeadline, _order.fillDeadline);
}
return _resolvedOrder(_order.orderDataType, _order.user, _order.openDeadline, _order.fillDeadline, _order.orderData);
}

/// @notice Resolves a OnchainCrossChainOrder.
Expand All @@ -217,6 +227,13 @@ contract T1ERC7683 is IT1ERC7683, T1Permit2, AccessControlUpgradeable, EIP712 {
view
returns (ResolvedCrossChainOrder memory, bytes32, uint256)
{
OrderData memory orderData = OrderEncoder.decode(_order.orderData);
if (orderData.sender != TypeCasts.addressToBytes32(msg.sender)) {
revert InvalidSender(orderData.sender, msg.sender);
}
if (orderData.fillDeadline != _order.fillDeadline) {
revert InvalidFillDeadline(orderData.fillDeadline, _order.fillDeadline);
}
return _resolvedOrder(_order.orderDataType, msg.sender, type(uint32).max, _order.fillDeadline, _order.orderData);
}

Expand Down
Loading