Skip to content

Commit

Permalink
chore(contracts/core): allow bridge fee overpayment
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinhalliday committed Oct 10, 2024
1 parent 6dd5800 commit 720be7b
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 38 deletions.
2 changes: 1 addition & 1 deletion contracts/allocs/devnet.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contracts/allocs/staging.json

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions contracts/core/.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
Admin_Test:test_pause_unpause() (gas: 26104285)
Admin_Test:test_pause_unpause_bridge() (gas: 21279753)
Admin_Test:test_pause_unpause_xcall() (gas: 26058227)
Admin_Test:test_pause_unpause_xsubmit() (gas: 26057978)
Admin_Test:test_upgrade() (gas: 29993873)
Admin_Test:test_pause_unpause() (gas: 26108503)
Admin_Test:test_pause_unpause_bridge() (gas: 21283971)
Admin_Test:test_pause_unpause_xcall() (gas: 26062445)
Admin_Test:test_pause_unpause_xsubmit() (gas: 26062196)
Admin_Test:test_upgrade() (gas: 29998091)
AllocPredeploys_Test:test_num_allocs() (gas: 1181021629)
AllocPredeploys_Test:test_predeploys() (gas: 1180958103)
AllocPredeploys_Test:test_preinstalls() (gas: 1181593197)
Expand All @@ -24,13 +24,13 @@ FeeOracleV2_Test:test_setProtocolFee() (gas: 31508)
FeeOracleV2_Test:test_setToNativeRate() (gas: 43377)
InitializableHelper_Test:test_disableInitalizers() (gas: 181686)
InitializableHelper_Test:test_getInitialized() (gas: 178023)
OmniBridgeL1_Test:test_bridge() (gas: 252423)
OmniBridgeL1_Test:test_bridge() (gas: 233678)
OmniBridgeL1_Test:test_pauseAll() (gas: 49368)
OmniBridgeL1_Test:test_pauseBridging() (gas: 59610)
OmniBridgeL1_Test:test_pauseBridging() (gas: 59580)
OmniBridgeL1_Test:test_pauseWithdraws() (gas: 48975)
OmniBridgeL1_Test:test_stub() (gas: 143)
OmniBridgeL1_Test:test_withdraw() (gas: 1382984)
OmniBridgeNative_Test:test_bridge() (gas: 190446)
OmniBridgeL1_Test:test_withdraw() (gas: 1386987)
OmniBridgeNative_Test:test_bridge() (gas: 156805)
OmniBridgeNative_Test:test_claim() (gas: 287756)
OmniBridgeNative_Test:test_pauseAll() (gas: 52904)
OmniBridgeNative_Test:test_pauseBridging() (gas: 44449)
Expand Down
4 changes: 3 additions & 1 deletion contracts/core/src/token/OmniBridgeL1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ contract OmniBridgeL1 is OmniBridgeCommon {
bytes memory xcalldata =
abi.encodeCall(OmniBridgeNative.withdraw, (payor, to, amount, token.balanceOf(address(this)) + amount));

require(msg.value == omni.feeFor(omniChainId, xcalldata, XCALL_WITHDRAW_GAS_LIMIT), "OmniBridge: incorrect fee");
require(
msg.value >= omni.feeFor(omniChainId, xcalldata, XCALL_WITHDRAW_GAS_LIMIT), "OmniBridge: insufficient fee"
);
require(token.transferFrom(payor, address(this), amount), "OmniBridge: transfer failed");

omni.xcall{ value: msg.value }(
Expand Down
8 changes: 4 additions & 4 deletions contracts/core/src/token/OmniBridgeNative.sol
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,13 @@ contract OmniBridgeNative is OmniBridgeCommon {
require(to != address(0), "OmniBridge: no bridge to zero");
require(amount > 0, "OmniBridge: amount must be > 0");
require(amount <= l1BridgeBalance, "OmniBridge: no liquidity");

uint256 fee = bridgeFee(to, amount);
require(msg.value == amount + fee, "OmniBridge: incorrect funds");
require(msg.value >= amount + bridgeFee(to, amount), "OmniBridge: insufficient funds");

l1BridgeBalance -= amount;

omni.xcall{ value: fee }(
// if fee is overpaid, forward excess to portal.
// balance of this contract should continue to reflect funds bridged to L1.
omni.xcall{ value: msg.value - amount }(
l1ChainId,
ConfLevel.Finalized,
l1Bridge,
Expand Down
10 changes: 4 additions & 6 deletions contracts/core/test/token/OmniBridgeL1.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,8 @@ contract OmniBridgeL1_Test is Test {
vm.expectRevert("OmniBridge: no bridge to zero");
b.bridge(address(0), amount);

// value must equal fee
vm.expectRevert("OmniBridge: incorrect fee");
b.bridge{ value: fee + 1 }(to, amount);
vm.expectRevert("OmniBridge: incorrect fee");
// value must be greater than or equal fee
vm.expectRevert("OmniBridge: insufficient fee");
b.bridge{ value: fee - 1 }(to, amount);

// requires allowance
Expand Down Expand Up @@ -191,8 +189,8 @@ contract OmniBridgeL1_Test is Test {
// assert unpaused
assertFalse(b.isPaused(action));

// bridge succeeds
vm.expectRevert("OmniBridge: incorrect fee");
// bridge not paused (reverts, but not due to pause)
vm.expectRevert("OmniBridge: insufficient fee");
b.bridge(to, amount);
}

Expand Down
17 changes: 6 additions & 11 deletions contracts/core/test/token/OmniBridgeNative.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,9 @@ contract OmniBridgeNative_Test is Test {

b.setL1BridgeBalance(amount);

// requires msg.value == fee + amount
vm.expectRevert("OmniBridge: incorrect funds");
b.bridge{ value: amount }(to, amount);
vm.expectRevert("OmniBridge: incorrect funds");
b.bridge{ value: amount + 1 }(to, amount);

// must equal amount + fee
vm.expectRevert("OmniBridge: incorrect funds");
b.bridge{ value: amount + fee + 1 }(to, amount);
// requires msg.value >= fee + amount
vm.expectRevert("OmniBridge: insufficient funds");
b.bridge{ value: amount + fee - 1 }(to, amount);

// succeeds
//
Expand All @@ -92,9 +86,10 @@ contract OmniBridgeNative_Test is Test {
emit Bridge(address(this), to, amount);

// emits xcall
uint256 feeWithExcess = fee + 1; // test that bridge forwards excess fee to portal
vm.expectCall(
address(portal),
fee,
feeWithExcess,
abi.encodeCall(
IOmniPortal.xcall,
(
Expand All @@ -106,7 +101,7 @@ contract OmniBridgeNative_Test is Test {
)
)
);
b.bridge{ value: amount + fee }(to, amount);
b.bridge{ value: amount + feeWithExcess }(to, amount);

// decrements l1BridgeBalance
assertEq(b.l1BridgeBalance(), 0);
Expand Down
2 changes: 1 addition & 1 deletion halo/genutil/evm/testdata/TestMakeEVMGenesis.golden

Large diffs are not rendered by default.

0 comments on commit 720be7b

Please sign in to comment.