Skip to content

Commit

Permalink
feat(contracts/misc): bridge dynamic gas (#2985)
Browse files Browse the repository at this point in the history 
More gas is needed to send tokens to bridges with a lockbox, so these
changes allow for the gas calculation to reflect that so gas isn't
needlessly wasted by defaulting to the upper bound.

issue: none
  • Loading branch information
@Zodomo
Zodomo authored Feb 6, 2025
1 parent 16e572f commit b5c9101
Show file tree
Hide file tree
Showing 7 changed files with 316 additions and 258 deletions.
425 changes: 227 additions & 198 deletions contracts/bindings/bridge.go
View file

Large diffs are not rendered by default.

30 changes: 15 additions & 15 deletions contracts/misc/.gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,24 @@ DepositTest:test_depositTo_other_succeeds() (gas: 153450)
DepositTest:test_depositTo_self_succeeds() (gas: 142560)
DepositTest:test_deposit_succeeds() (gas: 142215)
DepositTest:test_deposits_reverts() (gas: 168237)
GeneralBridgeTest:test_initialize_reverts() (gas: 3180347)
GeneralBridgeTest:test_setRoutes_reverts() (gas: 66041)
GeneralBridgeTest:test_setRoutes_succeeds() (gas: 46447)
GeneralBridgeTest:test_initialize_reverts() (gas: 3257515)
GeneralBridgeTest:test_setRoutes_reverts() (gas: 66778)
GeneralBridgeTest:test_setRoutes_succeeds() (gas: 47325)
GeneralLockboxest:test_initialize_reverts() (gas: 1396931)
PledgeTest:test_pledge_reverts() (gas: 22501)
PledgeTest:test_pledge_success() (gas: 51984)
ReceiveTokenTest:test_receiveToken_reverts() (gas: 268994)
ReceiveTokenTest:test_receiveToken_succeeds_insolvent_lockbox() (gas: 191495)
ReceiveTokenTest:test_receiveToken_succeeds_no_lockbox() (gas: 135007)
ReceiveTokenTest:test_receiveToken_succeeds_paused() (gas: 297386)
ReceiveTokenTest:test_receiveToken_succeeds_solvent_lockbox() (gas: 212487)
SendTokenTest:test_sendToken_empty_lockbox_succeeds() (gas: 653132)
SendTokenTest:test_sendToken_fee_overpayment_refunded() (gas: 370256)
SendTokenTest:test_sendToken_reverts() (gas: 638532)
SendTokenTest:test_sendToken_withLockbox_token_succeeds() (gas: 319108)
SendTokenTest:test_sendToken_withLockbox_wrapper_succeeds() (gas: 298914)
SendTokenTest:test_sendToken_withoutLockbox_wrapper_succeeds() (gas: 311872)
SendTokenTest:test_sendToken_wrapper_overdrafted_lockbox_succeeds() (gas: 538308)
ReceiveTokenTest:test_receiveToken_reverts() (gas: 268726)
ReceiveTokenTest:test_receiveToken_succeeds_insolvent_lockbox() (gas: 191441)
ReceiveTokenTest:test_receiveToken_succeeds_no_lockbox() (gas: 134953)
ReceiveTokenTest:test_receiveToken_succeeds_paused() (gas: 297314)
ReceiveTokenTest:test_receiveToken_succeeds_solvent_lockbox() (gas: 212433)
SendTokenTest:test_sendToken_empty_lockbox_succeeds() (gas: 654802)
SendTokenTest:test_sendToken_fee_overpayment_refunded() (gas: 370772)
SendTokenTest:test_sendToken_reverts() (gas: 640742)
SendTokenTest:test_sendToken_withLockbox_token_succeeds() (gas: 319664)
SendTokenTest:test_sendToken_withLockbox_wrapper_succeeds() (gas: 299469)
SendTokenTest:test_sendToken_withoutLockbox_wrapper_succeeds() (gas: 312428)
SendTokenTest:test_sendToken_wrapper_overdrafted_lockbox_succeeds() (gas: 539420)
WithdrawTest:test_withdrawTo_other_succeeds() (gas: 151100)
WithdrawTest:test_withdrawTo_self_succeeds() (gas: 124918)
WithdrawTest:test_withdraw_succeeds() (gas: 124780)
Expand Down
52 changes: 31 additions & 21 deletions contracts/misc/src/bridge/Bridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@ contract Bridge is Initializable, AccessControlUpgradeable, PausableUpgradeable,
bytes32 public constant PAUSER_ROLE = 0x539440820030c4994db4e31b6b800deafd503688728f932addfe7a410515c14c;

/**
* @dev Default gas limit for xcalls.
* @dev Gas limit for xcalls to bridges without a lockbox.
*/
uint64 internal constant DEFAULT_GAS_LIMIT = 200_000;
uint64 internal constant RECEIVE_DEFAULT_GAS_LIMIT = 125_000;

/**
* @dev Gas limit for xcalls to bridges with a lockbox.
*/
uint64 internal constant RECEIVE_LOCKBOX_GAS_LIMIT = 180_000;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* STORAGE */
Expand All @@ -43,9 +48,9 @@ contract Bridge is Initializable, AccessControlUpgradeable, PausableUpgradeable,
address public lockbox;

/**
* @dev Mapping of destination chainId to bridge contract.
* @dev Mapping of destination chainId to bridge contract and config.
*/
mapping(uint64 chainId => address bridge) private _routes;
mapping(uint64 chainId => Route) private _routes;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* MODIFIERS */
Expand All @@ -56,7 +61,7 @@ contract Bridge is Initializable, AccessControlUpgradeable, PausableUpgradeable,
*/
modifier onlyBridge() {
if (msg.sender == address(omni)) {
if (_routes[xmsg.sourceChainId] != xmsg.sender) revert Unauthorized(xmsg.sourceChainId, xmsg.sender);
if (_routes[xmsg.sourceChainId].bridge != xmsg.sender) revert Unauthorized(xmsg.sourceChainId, xmsg.sender);
} else {
revert Unauthorized(uint64(block.chainid), msg.sender);
}
Expand Down Expand Up @@ -108,12 +113,14 @@ contract Bridge is Initializable, AccessControlUpgradeable, PausableUpgradeable,
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/**
* @dev Returns the bridge address for a given destination chainId.
* @dev Returns the bridge address and config for a given destination chainId.
* @param destChainId The chainId of the destination chain.
* @return bridge The bridge address.
* @return hasLockbox Whether the bridge has a lockbox.
*/
function routes(uint64 destChainId) external view returns (address bridge) {
return _routes[destChainId];
function getRoute(uint64 destChainId) external view returns (address bridge, bool hasLockbox) {
Route memory route = _routes[destChainId];
return (route.bridge, route.hasLockbox);
}

/**
Expand All @@ -122,8 +129,11 @@ contract Bridge is Initializable, AccessControlUpgradeable, PausableUpgradeable,
* @return fee The fee paid to the `OmniPortal` contract.
*/
function bridgeFee(uint64 destChainId) external view returns (uint256 fee) {
Route memory route = _routes[destChainId];
return feeFor(
destChainId, abi.encodeCall(Bridge.receiveToken, (TypeMax.Address, TypeMax.Uint256)), DEFAULT_GAS_LIMIT
destChainId,
abi.encodeCall(Bridge.receiveToken, (TypeMax.Address, TypeMax.Uint256)),
route.hasLockbox ? RECEIVE_LOCKBOX_GAS_LIMIT : RECEIVE_DEFAULT_GAS_LIMIT
);
}

Expand Down Expand Up @@ -158,18 +168,15 @@ contract Bridge is Initializable, AccessControlUpgradeable, PausableUpgradeable,

/**
* @dev Sets bridge routes for given chainIds.
* @param chainIds The chainIds to configure.
* @param bridgeAddrs The bridges addresses to configure.
* @param chainIds The chainIds to configure.
* @param routes The bridges addresses and configs to configure.
*/
function setRoutes(uint64[] calldata chainIds, address[] calldata bridgeAddrs)
external
onlyRole(DEFAULT_ADMIN_ROLE)
{
if (chainIds.length != bridgeAddrs.length) revert ArrayLengthMismatch();
function setRoutes(uint64[] calldata chainIds, Route[] calldata routes) external onlyRole(DEFAULT_ADMIN_ROLE) {
if (chainIds.length != routes.length) revert ArrayLengthMismatch();
for (uint256 i = 0; i < chainIds.length; i++) {
if (bridgeAddrs[i] == address(0)) revert ZeroAddress();
_routes[chainIds[i]] = bridgeAddrs[i];
emit BridgeConfigured(chainIds[i], bridgeAddrs[i]);
if (routes[i].bridge == address(0)) revert ZeroAddress();
_routes[chainIds[i]] = routes[i];
emit RouteConfigured(chainIds[i], routes[i].bridge, routes[i].hasLockbox);
}
}

Expand Down Expand Up @@ -199,7 +206,7 @@ contract Bridge is Initializable, AccessControlUpgradeable, PausableUpgradeable,
* @param wrap Whether the token is being wrapped.
*/
function _validateSend(uint64 destChainId, address to, uint256 value, bool wrap) internal view {
if (_routes[destChainId] == address(0)) revert InvalidRoute(destChainId);
if (_routes[destChainId].bridge == address(0)) revert InvalidRoute(destChainId);
if (to == address(0)) revert ZeroAddress();
if (value == 0) revert ZeroAmount();
if (wrap && lockbox == address(0)) revert CannotWrap();
Expand Down Expand Up @@ -237,8 +244,11 @@ contract Bridge is Initializable, AccessControlUpgradeable, PausableUpgradeable,
* @param value The amount of tokens to transfer.
*/
function _omniTransfer(uint64 destChainId, address to, uint256 value) internal {
Route memory route = _routes[destChainId];
bytes memory data = abi.encodeCall(Bridge.receiveToken, (to, value));
uint256 fee = xcall(destChainId, _routes[destChainId], data, DEFAULT_GAS_LIMIT);
uint256 fee = xcall(
destChainId, route.bridge, data, route.hasLockbox ? RECEIVE_LOCKBOX_GAS_LIMIT : RECEIVE_DEFAULT_GAS_LIMIT
);

if (msg.value < fee) revert InsufficientPayment();
if (msg.value > fee) msg.sender.safeTransferETH(msg.value - fee);
Expand Down
23 changes: 17 additions & 6 deletions contracts/misc/src/bridge/interfaces/IBridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ interface IBridge {
/**
* @dev Event emitted when a bridge route is configured.
*/
event BridgeConfigured(uint64 indexed destChainId, address indexed bridge);
event RouteConfigured(uint64 indexed destChainId, address indexed bridge, bool indexed hasLockbox);

/**
* @dev Event emitted when a crosschain token transfer is initiated.
Expand All @@ -69,6 +69,16 @@ interface IBridge {
/* STORAGE */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/**
* @dev Struct representing a bridge route.
* @param bridge The address of the bridge contract.
* @param hasLockbox Whether the bridge has a lockbox.
*/
struct Route {
address bridge;
bool hasLockbox;
}

/**
* @dev Address of the token being bridged.
*/
Expand All @@ -84,11 +94,12 @@ interface IBridge {
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/**
* @dev Returns the bridge address for a given destination chainId.
* @dev Returns the bridge address and config for a given destination chainId.
* @param destChainId The chainId of the destination chain.
* @return bridge The bridge address.
* @return hasLockbox Whether the bridge has a lockbox.
*/
function routes(uint64 destChainId) external view returns (address bridge);
function getRoute(uint64 destChainId) external view returns (address bridge, bool hasLockbox);

/**
* @dev Returns the fee for bridging a token to a destination chain.
Expand Down Expand Up @@ -123,8 +134,8 @@ interface IBridge {

/**
* @dev Sets bridge routes for given chainIds.
* @param chainIds The chainIds to configure.
* @param bridgeAddrs The bridges addresses to configure.
* @param chainIds The chainIds to configure.
* @param routes The bridges addresses and configs to configure.
*/
function setRoutes(uint64[] calldata chainIds, address[] calldata bridgeAddrs) external;
function setRoutes(uint64[] calldata chainIds, Route[] calldata routes) external;
}
12 changes: 6 additions & 6 deletions contracts/misc/test/bridge/TestBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ contract TestBase is Test {
function mockBridgeReceive(Bridge bridge, uint64 srcChainId, uint64 destChainId, address to, uint256 value)
internal
{
address destination = bridge.routes(destChainId);
(address destination,) = bridge.getRoute(destChainId);
bytes memory data = abi.encodeCall(Bridge.receiveToken, (to, value));

vm.chainId(destChainId);
Expand Down Expand Up @@ -175,17 +175,17 @@ contract TestBase is Test {

function _configureRoutes() internal {
uint64[] memory chainIds = new uint64[](1);
address[] memory bridges = new address[](1);
IBridge.Route[] memory routes = new IBridge.Route[](1);

vm.startPrank(admin);

chainIds[0] = DEST_CHAIN_ID;
bridges[0] = address(bridgeNoLockbox);
bridgeWithLockbox.setRoutes(chainIds, bridges);
routes[0] = IBridge.Route({ bridge: address(bridgeNoLockbox), hasLockbox: false });
bridgeWithLockbox.setRoutes(chainIds, routes);

chainIds[0] = SRC_CHAIN_ID;
bridges[0] = address(bridgeWithLockbox);
bridgeNoLockbox.setRoutes(chainIds, bridges);
routes[0] = IBridge.Route({ bridge: address(bridgeWithLockbox), hasLockbox: true });
bridgeNoLockbox.setRoutes(chainIds, routes);

vm.stopPrank();
}
Expand Down
18 changes: 9 additions & 9 deletions contracts/misc/test/bridge/bridge/General.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,26 +71,26 @@ contract GeneralBridgeTest is TestBase {
function test_setRoutes_reverts() public prank(admin) {
uint64[] memory chainIds = new uint64[](1);
chainIds[0] = DEST_CHAIN_ID + 1;
address[] memory bridgeAddrs;
IBridge.Route[] memory routes;

vm.expectRevert(IBridge.ArrayLengthMismatch.selector);
bridgeWithLockbox.setRoutes(chainIds, bridgeAddrs);
bridgeAddrs = new address[](1);
bridgeWithLockbox.setRoutes(chainIds, routes);
routes = new IBridge.Route[](1);

vm.expectRevert(IBridge.ZeroAddress.selector);
bridgeWithLockbox.setRoutes(chainIds, bridgeAddrs);
bridgeAddrs[0] = makeAddr("newBridge");
bridgeWithLockbox.setRoutes(chainIds, routes);
routes[0] = IBridge.Route({ bridge: makeAddr("newBridge"), hasLockbox: false });

// Configuration successful with valid inputs.
bridgeWithLockbox.setRoutes(chainIds, bridgeAddrs);
bridgeWithLockbox.setRoutes(chainIds, routes);
}

function test_setRoutes_succeeds() public prank(admin) {
uint64[] memory chainIds = new uint64[](1);
chainIds[0] = DEST_CHAIN_ID + 1;
address[] memory bridgeAddrs = new address[](1);
bridgeAddrs[0] = makeAddr("newBridge");
IBridge.Route[] memory routes = new IBridge.Route[](1);
routes[0] = IBridge.Route({ bridge: makeAddr("newBridge"), hasLockbox: false });

bridgeWithLockbox.setRoutes(chainIds, bridgeAddrs);
bridgeWithLockbox.setRoutes(chainIds, routes);
}
}
14 changes: 11 additions & 3 deletions e2e/xbridge/bridge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,23 +175,31 @@ func setRoutes(
return errors.Wrap(err, "bind opts")
}

canon, err := xtoken.Canonical(ctx, network.ID)
if err != nil {
return errors.Wrap(err, "canonical")
}

var destChainIDs []uint64
var destAddrs []common.Address
var routes []bindings.IBridgeRoute
for _, dest := range network.EVMChains() {
if dest.ID == chain.ID {
continue
}

destChainIDs = append(destChainIDs, dest.ID)
destAddrs = append(destAddrs, addr)
routes = append(routes, bindings.IBridgeRoute{
Bridge: addr,
HasLockbox: dest.ID == canon.ChainID,
})
}

bridge, err := bindings.NewBridge(addr, backend)
if err != nil {
return errors.Wrap(err, "new bridge")
}

tx, err := bridge.SetRoutes(txOpts, destChainIDs, destAddrs)
tx, err := bridge.SetRoutes(txOpts, destChainIDs, routes)
if err != nil {
return errors.Wrap(err, "set destinations")
}
Expand Down

0 comments on commit b5c9101

Please sign in to comment.