Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion src/output/OutputSettlerBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ abstract contract OutputSettlerBase is IAttester, BaseInputOracle {
error PayloadTooSmall();
/// @dev Payload does not lead with a known domain magic
error InvalidPayloadMagic(bytes4 magic);
/// @dev `msg.value` does not cover the native output(s) paid out by the fill
error InsufficientNativeValue(uint256 required, uint256 provided);

/**
* @notice Sets outputs as filled by their solver identifier, such that outputs won't be filled twice.
Expand Down Expand Up @@ -288,12 +290,16 @@ abstract contract OutputSettlerBase is IAttester, BaseInputOracle {
}

/**
* @notice Refunds the unused native value to msg.sender.
* @notice Checks that `msg.value` covers the native paid out, and refunds the unused remainder to msg.sender.
* @dev The coverage check lives here rather than in `_fill` because `fillOrderOutputs` accumulates `nativeSent`
* across every output and settles once. A per-output check would pass on each individual output while the total
* overdrew the contract's balance.
* @param nativeSent Amount of native already paid out by `_fill`.
*/
function _refundNativeExcess(
uint256 nativeSent
) internal {
if (msg.value < nativeSent) revert InsufficientNativeValue(nativeSent, msg.value);
if (msg.value > nativeSent) Address.sendValue(payable(msg.sender), msg.value - nativeSent);
}

Expand Down
39 changes: 39 additions & 0 deletions test/output/OutputSettlerSimple.fill.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,45 @@ contract OutputSettlerSimpleTestFill is Test {
assertEq(sender.balance, senderBalanceBeforeSecond);
}

/// @notice A native fill must revert when `msg.value` does not cover the output, rather than paying it out of a
/// residual balance held by the settler.
/// @dev The settler is not meant to hold a balance, but it is not prevented from receiving one (for example via a
/// SELFDESTRUCT force-feed). Without the coverage check, a self-order with a native output and `msg.value = 0`
/// would let anyone sweep that balance.
function test_fill_native_token_insufficient_value_with_residual_balance(
bytes32 orderId,
bytes32 filler,
uint128 amount
) public {
vm.assume(filler != bytes32(0) && amount > 0);

address sender = makeAddr("sender");
// Residual balance on the settler, as a SELFDESTRUCT force-feed could produce.
vm.deal(outputSettlerCoinAddress, amount);

bytes memory fillerData = abi.encodePacked(filler);

// A self-order: the caller is also the recipient of the native output.
MandateOutput memory outputStruct = MandateOutput({
oracle: bytes32(0),
settler: bytes32(uint256(uint160(outputSettlerCoinAddress))),
chainId: block.chainid,
token: bytes32(0),
amount: amount,
recipient: bytes32(uint256(uint160(sender))),
callbackData: bytes(""),
context: bytes("")
});

vm.prank(sender);
vm.expectRevert(abi.encodeWithSignature("InsufficientNativeValue(uint256,uint256)", uint256(amount), 0));
outputSettlerCoin.fill{ value: 0 }(orderId, outputStruct, type(uint48).max, fillerData);

// The settler's balance was not swept.
assertEq(outputSettlerCoinAddress.balance, uint256(amount));
assertEq(sender.balance, 0);
}

function test_fill_native_token_with_callback(
bytes32 orderId,
uint256 amount,
Expand Down
111 changes: 111 additions & 0 deletions test/output/OutputSettlerSimple.fillOrderOutputs.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,117 @@ contract OutputSettlerSimpleTestfillOrderOutputs is Test {
outputSettlerCoin.fillOrderOutputs{ value: sentValue }(orderId, outputs, type(uint48).max, fillerData);
}

/// @notice A batch whose native outputs sum to more than `msg.value` must revert, even when the settler holds a
/// residual balance large enough to cover the difference.
/// @dev This is the case that a per-output check inside `_fill` would miss: `msg.value` covers each individual
/// output, but not their sum. Only the cumulative check in `_refundNativeExcess` catches it.
function test_fill_batch_native_token_total_exceeds_value(
bytes32 orderId,
bytes32 filler,
uint128 amount1,
uint128 amount2
) public {
vm.assume(filler != bytes32(0));
vm.assume(amount1 > 0 && amount2 > 0 && amount1 != amount2);

address sender = makeAddr("sender");
uint256 totalRequired = uint256(amount1) + uint256(amount2);
// Enough to cover either output on its own, but not both together.
uint256 sentValue = amount1 > amount2 ? uint256(amount1) : uint256(amount2);
vm.deal(sender, sentValue);
// Residual balance on the settler, as a SELFDESTRUCT force-feed could produce. Without it the second
// `Address.sendValue` would revert on an empty balance and mask the overdraw.
vm.deal(outputSettlerCoinAddress, totalRequired);

MandateOutput[] memory outputs = new MandateOutput[](2);

outputs[0] = MandateOutput({
oracle: bytes32(0),
settler: bytes32(uint256(uint160(outputSettlerCoinAddress))),
chainId: block.chainid,
token: bytes32(0), // native token
amount: amount1,
recipient: bytes32(uint256(uint160(swapper))),
callbackData: bytes(""),
context: bytes("")
});

outputs[1] = MandateOutput({
oracle: bytes32(0),
settler: bytes32(uint256(uint160(outputSettlerCoinAddress))),
chainId: block.chainid,
token: bytes32(0), // native token
amount: amount2,
recipient: bytes32(uint256(uint160(swapper))),
callbackData: bytes(""),
context: bytes("")
});

bytes memory fillerData = abi.encodePacked(filler);

vm.prank(sender);
vm.expectRevert(abi.encodeWithSignature("InsufficientNativeValue(uint256,uint256)", totalRequired, sentValue));
outputSettlerCoin.fillOrderOutputs{ value: sentValue }(orderId, outputs, type(uint48).max, fillerData);

// The revert unwound both sends, so the residual balance is intact.
assertEq(outputSettlerCoinAddress.balance, totalRequired);
}

/// @notice A correctly funded batch still fills and still refunds the excess, and leaves any residual balance
/// held by the settler untouched.
function test_fill_batch_native_token_excess_refund_leaves_residual(
bytes32 orderId,
bytes32 filler,
uint128 amount1,
uint128 amount2,
uint128 excess,
uint128 residual
) public {
vm.assume(filler != bytes32(0));
vm.assume(amount1 > 0 && amount2 > 0 && amount1 != amount2);

address sender = makeAddr("sender");
uint256 totalRequired = uint256(amount1) + uint256(amount2);
uint256 totalSent = totalRequired + uint256(excess);
vm.deal(sender, totalSent);
vm.deal(outputSettlerCoinAddress, residual);

MandateOutput[] memory outputs = new MandateOutput[](2);

outputs[0] = MandateOutput({
oracle: bytes32(0),
settler: bytes32(uint256(uint160(outputSettlerCoinAddress))),
chainId: block.chainid,
token: bytes32(0), // native token
amount: amount1,
recipient: bytes32(uint256(uint160(swapper))),
callbackData: bytes(""),
context: bytes("")
});

outputs[1] = MandateOutput({
oracle: bytes32(0),
settler: bytes32(uint256(uint160(outputSettlerCoinAddress))),
chainId: block.chainid,
token: bytes32(0), // native token
amount: amount2,
recipient: bytes32(uint256(uint160(swapper))),
callbackData: bytes(""),
context: bytes("")
});

bytes memory fillerData = abi.encodePacked(filler);

uint256 swapperBalanceBefore = swapper.balance;

vm.prank(sender);
outputSettlerCoin.fillOrderOutputs{ value: totalSent }(orderId, outputs, type(uint48).max, fillerData);

assertEq(swapper.balance, swapperBalanceBefore + totalRequired);
assertEq(sender.balance, uint256(excess)); // Excess refunded
assertEq(outputSettlerCoinAddress.balance, uint256(residual)); // Residual untouched
}

/// @notice Recipient of the first (native) output reenters `fill` while the batch fills a subsequent ERC20 output.
function test_fill_batch_native_then_erc20_excess_refund_with_reentering_recipient() public {
bytes32 orderId = keccak256(bytes("orderId"));
Expand Down