Consider enforcing fillDeadline at the Base7683 fill lifecycle boundary

[chenshj73]

Summary

Base7683.fill(...) moves an order from UNKNOWN to FILLED after _fillOrder(...) returns, but the base lifecycle boundary does not itself enforce fillDeadline. The refund paths do use fillDeadline to decide when an order is refundable, so deadline enforcement is part of the order lifecycle even though it is currently delegated to each _fillOrder implementation.

The existing BasicSwap7683 implementation does check fillDeadline, so I am not claiming that implementation is currently exploitable. The concern is that Base7683 makes this a per-inheritor obligation without enforcing or documenting the obligation at the abstract boundary.

Reviewed revision: 62a181ff597f486ad5dd167f2fd636cb958464c8

Origin open path enforces openDeadline in the base contract

solidity/src/Base7683.sol:139-160

139 function openFor(
140     GaslessCrossChainOrder calldata _order,
141     bytes calldata _signature,
142     bytes calldata _originFillerData
143 )
144     external
145     virtual
146 {
147     if (block.timestamp > _order.openDeadline) revert OrderOpenExpired();
148     if (_order.originSettler != address(this)) revert InvalidGaslessOrderSettler();
149     if (_order.originChainId != _localDomain()) revert InvalidGaslessOrderOrigin();
150
151     (ResolvedCrossChainOrder memory resolvedOrder, bytes32 orderId, uint256 nonce) = _resolveOrder(_order, _originFillerData);
152
153     openOrders[orderId] = abi.encode(_order.orderDataType, _order.orderData);
154     orderStatus[orderId] = OPENED;
155     _useNonce(_order.user, nonce);
156
157     _permitTransferFrom(resolvedOrder, _signature, _order.nonce, address(this));
158
159     emit Open(orderId, resolvedOrder);
160 }

This makes openDeadline a base-level lifecycle invariant.

Destination fill path does not enforce fillDeadline at the same base boundary

solidity/src/Base7683.sol:225-240

225 * @notice Fills a single leg of a particular order on the destination chain
226 * @param _orderId Unique order identifier for this order
227 * @param _originData Data emitted on the origin to parameterize the fill
228 * @param _fillerData Data provided by the filler to inform the fill or express their preferences. It should
229 * contain the bytes32 encoded address of the receiver which is used at settlement time
230 */
231 function fill(bytes32 _orderId, bytes calldata _originData, bytes calldata _fillerData) external payable virtual {
232     if (orderStatus[_orderId] != UNKNOWN) revert InvalidOrderStatus();
233
234     _fillOrder(_orderId, _originData, _fillerData);
235
236     orderStatus[_orderId] = FILLED;
237     filledOrders[_orderId] = FilledOrder(_originData, _fillerData);
238
239     emit Filled(_orderId, _originData, _fillerData);
240 }

After _fillOrder(...) returns, the base contract unconditionally records the destination status as FILLED. There is no base-level check here that the fill is still before the order's fillDeadline.

Refund path treats fillDeadline as the lifecycle boundary

Gasless order refund:

solidity/src/Base7683.sol:273-285

273 function refund(GaslessCrossChainOrder[] memory _orders) external payable {
274     bytes32[] memory orderIds = new bytes32[](_orders.length);
275     for (uint256 i = 0; i < _orders.length; i += 1) {
276         bytes32 orderId = _getOrderId(_orders[i]);
277         orderIds[i] = orderId;
278
279         if (orderStatus[orderId] != UNKNOWN) revert InvalidOrderStatus();
280         if (block.timestamp <= _orders[i].fillDeadline) revert OrderFillNotExpired();
281     }
282
283     _refundOrders(_orders, orderIds);
284
285     emit Refund(orderIds);

Onchain order refund:

solidity/src/Base7683.sol:295-305

295 function refund(OnchainCrossChainOrder[] memory _orders) external payable {
296     bytes32[] memory orderIds = new bytes32[](_orders.length);
297     for (uint256 i = 0; i < _orders.length; i += 1) {
298         bytes32 orderId = _getOrderId(_orders[i]);
299         orderIds[i] = orderId;
300
301         if (orderStatus[orderId] != UNKNOWN) revert InvalidOrderStatus();
302         if (block.timestamp <= _orders[i].fillDeadline) revert OrderFillNotExpired();
303     }
304
305     _refundOrders(_orders, orderIds);

This means an expired order is refundable only while its destination-side status remains UNKNOWN. If an inheriting _fillOrder implementation forgets to reject expired fills, Base7683.fill(...) can move an expired order to FILLED, which blocks the refund transition.

Current concrete implementation is safe because it repeats the deadline check

solidity/src/BasicSwap7683.sol:385-390

385 function _fillOrder(bytes32 _orderId, bytes calldata _originData, bytes calldata) internal override {
386     OrderData memory orderData = OrderEncoder.decode(_originData);
387
388     if (_orderId != OrderEncoder.id(orderData)) revert InvalidOrderId();
389     if (block.timestamp > orderData.fillDeadline) revert OrderFillExpired();
390     if (orderData.destinationDomain != _localDomain()) revert InvalidOrderDomain();

So the issue is not that BasicSwap7683 currently misses the check. The issue is that the base contract's lifecycle transition relies on every inheritor remembering to implement this check correctly.

Hook documentation does not state that obligation

solidity/src/Base7683.sol:440-446

440 * @notice Fills an order with specific origin and filler data.
441 * @dev To be implemented by the inheriting contract. Defines how to process the origin and filler data.
442 * @param _orderId The unique identifier for the order to fill.
443 * @param _originData Data emitted on the origin chain to parameterize the fill.
444 * @param _fillerData Data provided by the filler, including preferences and additional information.
445 */
446 function _fillOrder(bytes32 _orderId, bytes calldata _originData, bytes calldata _fillerData) internal virtual;

The hook docs say inheritors should process origin/filler data, but they do not explicitly say that inheritors must reject block.timestamp > fillDeadline before returning to the base fill(...) method.

Why this matters

For cross-chain intent systems, fillDeadline is the boundary between a valid destination fill and an origin-side refund. If that boundary is not enforced consistently, the lifecycle can drift:

UNKNOWN + expired + refund(...)  -> refundable
UNKNOWN + expired + fill(...)    -> FILLED, if inheritor forgot deadline check

That is the kind of implementation-level lifecycle mismatch that is easy to miss when each local function looks reasonable.

Suggested fix

Two possible approaches:

1. If Base7683 can reliably decode or resolve the deadline before calling _fillOrder, enforce fillDeadline directly inside fill(...), mirroring the base openDeadline check.
2. If the base contract must remain format-agnostic, add an explicit NatSpec warning on _fillOrder, plus a reusable internal helper or tests showing that every implementation must reject expired fills before fill(...) is allowed to mark the order FILLED.