diff --git a/packages/abis/src/abis/RelayPool.sol/RelayPool.json b/packages/abis/src/abis/RelayPool.sol/RelayPool.json index 69f5acda..79a6aeab 100644 --- a/packages/abis/src/abis/RelayPool.sol/RelayPool.json +++ b/packages/abis/src/abis/RelayPool.sol/RelayPool.json @@ -40,6 +40,27 @@ "stateMutability": "nonpayable", "type": "constructor" }, + { + "inputs": [ + { + "internalType": "uint32", + "name": "chainId", + "type": "uint32" + }, + { + "internalType": "address", + "name": "bridge", + "type": "address" + }, + { + "internalType": "uint256", + "name": "outstandingDebt", + "type": "uint256" + } + ], + "name": "BridgeFeeChangeWithOutstandingDebt", + "type": "error" + }, { "inputs": [ { @@ -834,6 +855,30 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "uint32", + "name": "", + "type": "uint32" + }, + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "accumulatedFeesByOrigin", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { diff --git a/smart-contracts/contracts/RelayPool.sol b/smart-contracts/contracts/RelayPool.sol index 5be57eb3..a2de088c 100644 --- a/smart-contracts/contracts/RelayPool.sol +++ b/smart-contracts/contracts/RelayPool.sol @@ -174,6 +174,13 @@ contract RelayPool is ERC4626, Ownable { /// @dev Fees are held in the yield pool until they finish streaming uint256 public pendingBridgeFees = 0; + /// @notice Per-origin accumulated bridge fees not yet claimed + /// @dev Bounds the claim-time fee deduction. Per-message fees in handle() are + /// floored, and floor(A) + floor(B) <= floor(A+B), so the batch fee at + /// claim time can exceed the actually-accrued total without this cap. + mapping(uint32 => mapping(address => uint256)) + public accumulatedFeesByOrigin; + /// @notice All incoming assets are streamed (even though they are instantly deposited in the yield pool) /// @dev Total amount of assets currently being streamed uint256 public totalAssetsToStream = 0; @@ -589,6 +596,7 @@ contract RelayPool is ERC4626, Ownable { // Calculate fee using fractional basis points uint256 feeAmount = (message.amount * origin.bridgeFee) / FRACTIONAL_BPS_DENOMINATOR; + accumulatedFeesByOrigin[chainId][bridge] += feeAmount; pendingBridgeFees += feeAmount; // Check if origin settings are respected @@ -702,9 +710,10 @@ contract RelayPool is ERC4626, Ownable { uint256 feeAmount = 0; if (chargeFee) { - // The amount is the amount that was loaned + the fees - feeAmount = (amount * origin.bridgeFee) / + uint256 batchFee = (amount * origin.bridgeFee) / FRACTIONAL_BPS_DENOMINATOR; + feeAmount = Math.min(accumulatedFeesByOrigin[chainId][bridge], batchFee); + accumulatedFeesByOrigin[chainId][bridge] -= feeAmount; pendingBridgeFees -= feeAmount; // We need to account for it in a streaming fashion addToStreamingAssets(feeAmount); diff --git a/smart-contracts/ignition/deployments/BridgeProxy-81457-0x57B68c4EA221ee8Da6eb14ebdfcCEE5177567771-optimism-1/deployed_addresses.json b/smart-contracts/ignition/deployments/BridgeProxy-81457-0x57B68c4EA221ee8Da6eb14ebdfcCEE5177567771-optimism-1/deployed_addresses.json index ad39ccee..4d928fba 100644 --- a/smart-contracts/ignition/deployments/BridgeProxy-81457-0x57B68c4EA221ee8Da6eb14ebdfcCEE5177567771-optimism-1/deployed_addresses.json +++ b/smart-contracts/ignition/deployments/BridgeProxy-81457-0x57B68c4EA221ee8Da6eb14ebdfcCEE5177567771-optimism-1/deployed_addresses.json @@ -1,3 +1,3 @@ { "OPStackNativeBridgeProxy#OPStackNativeBridgeProxy": "0x3E1A97aF59Ec9a1636C52e1487A13C0AF34e49dC" -} \ No newline at end of file +} diff --git a/smart-contracts/ignition/deployments/BridgeProxy-81457-0x57B68c4EA221ee8Da6eb14ebdfcCEE5177567771-optimism-81457/deployed_addresses.json b/smart-contracts/ignition/deployments/BridgeProxy-81457-0x57B68c4EA221ee8Da6eb14ebdfcCEE5177567771-optimism-81457/deployed_addresses.json index 74eada5d..9a699b84 100644 --- a/smart-contracts/ignition/deployments/BridgeProxy-81457-0x57B68c4EA221ee8Da6eb14ebdfcCEE5177567771-optimism-81457/deployed_addresses.json +++ b/smart-contracts/ignition/deployments/BridgeProxy-81457-0x57B68c4EA221ee8Da6eb14ebdfcCEE5177567771-optimism-81457/deployed_addresses.json @@ -1,3 +1,3 @@ { "OPStackNativeBridgeProxy#OPStackNativeBridgeProxy": "0xf98D7ADA874D53a75BbDfB05D2A96C1525d426A7" -} \ No newline at end of file +} diff --git a/smart-contracts/ignition/deployments/bridges/81457/0x320357f5d8d107443BDF99ad325900c703Cf4b71/params.json b/smart-contracts/ignition/deployments/bridges/81457/0x320357f5d8d107443BDF99ad325900c703Cf4b71/params.json index 175ae593..99ba1b58 100644 --- a/smart-contracts/ignition/deployments/bridges/81457/0x320357f5d8d107443BDF99ad325900c703Cf4b71/params.json +++ b/smart-contracts/ignition/deployments/bridges/81457/0x320357f5d8d107443BDF99ad325900c703Cf4b71/params.json @@ -2,4 +2,4 @@ "assetAddress": "0x0000000000000000000000000000000000000000", "hyperlaneMailbox": "0x3a867fCfFeC2B790970eeBDC9023E75B0a172aa7", "proxyBridgeAddress": "0xf98D7ADA874D53a75BbDfB05D2A96C1525d426A7" -} \ No newline at end of file +} diff --git a/smart-contracts/test/RelayPool/fee-truncation.hardhat.ts b/smart-contracts/test/RelayPool/fee-truncation.hardhat.ts new file mode 100644 index 00000000..00c62ec7 --- /dev/null +++ b/smart-contracts/test/RelayPool/fee-truncation.hardhat.ts @@ -0,0 +1,248 @@ +import { expect } from 'chai' +import { ethers, ignition } from 'hardhat' +import RelayPoolModule from '../../ignition/modules/RelayPoolModule' +import OPStackNativeBridgeProxyModule from '../../ignition/modules/OPStackNativeBridgeProxyModule' +import { + MyToken, + MyWeth, + MyYieldPool, + OPStackNativeBridgeProxy, + RelayPool, +} from '../../typechain-types' +import { encodeData } from './hyperlane.hardhat' + +// fractionalBps denominator is 1e11. Pick a bridgeFee such that +// `amount * bridgeFee / 1e11` truncates to 0 for the per-message +// amount we send below, but produces a non-zero batch fee once those +// amounts are aggregated by the bridge. +// +// With bridgeFee = 1e9 (1%) and per-message amount = 50 wei: +// per-message: 50 * 1e9 / 1e11 = 0 (truncated) +// batch (1000 msgs of 50 wei): 50_000 * 1e9 / 1e11 = 500 +const relayBridgeOptimism = '0x0000000000000000000000000000000000000010' +const relayBridgeBase = '0x0000000000000000000000000000000000008453' +const portalProxy = '0x49048044D57e1C92A77f79988d21Fa8fAF74E97e' + +const bridgeFee = BigInt(1_000_000_000) // 1% in fractional bps +const FRACTIONAL_BPS_DENOMINATOR = BigInt(100_000_000_000) +const MESSAGE_COUNT = 1000n +const PER_MESSAGE_AMOUNT = 50n // wei — small enough to truncate to 0 fee + +describe('RelayPool: per-message fee truncation does not brick claim', () => { + let relayPool: RelayPool + let myToken: MyToken + let myWeth: MyWeth + let thirdPartyPool: MyYieldPool + let bridgeProxy: OPStackNativeBridgeProxy + let anotherBridgeProxy: OPStackNativeBridgeProxy + let userAddress: string + + before(async () => { + const [user] = await ethers.getSigners() + userAddress = await user.getAddress() + + myToken = await ethers.deployContract('MyToken', ['My Token', 'TOKEN']) + myWeth = await ethers.deployContract('MyWeth') + thirdPartyPool = await ethers.deployContract('MyYieldPool', [ + await myToken.getAddress(), + 'My Yield Pool', + 'YIELD', + ]) + + const parameters = { + RelayPool: { + asset: await myToken.getAddress(), + curator: userAddress, + // user is the mailbox so we can call handle() directly + hyperlaneMailbox: userAddress, + name: 'ERC20 RELAY POOL', + symbol: 'ERC20-REL', + thirdPartyPool: await thirdPartyPool.getAddress(), + weth: await myWeth.getAddress(), + }, + } + ;({ relayPool } = await ignition.deploy(RelayPoolModule, { parameters })) + + const bridgeProxyParameters = { + OPStackNativeBridgeProxy: { + l1BridgeProxy: ethers.ZeroAddress, + portalProxy, + relayPool: await relayPool.getAddress(), + relayPoolChainId: 31337, + }, + } + ;({ bridge: bridgeProxy } = await ignition.deploy( + OPStackNativeBridgeProxyModule, + { parameters: bridgeProxyParameters } + )) + ;({ bridge: anotherBridgeProxy } = await ignition.deploy( + OPStackNativeBridgeProxyModule, + { parameters: bridgeProxyParameters } + )) + + await relayPool.addOrigin({ + bridge: relayBridgeOptimism, + bridgeFee, + chainId: 10, + coolDown: 0, + curator: userAddress, + maxDebt: ethers.parseEther('10'), + proxyBridge: await bridgeProxy.getAddress(), + }) + + await relayPool.addOrigin({ + bridge: relayBridgeBase, + bridgeFee, + chainId: 8453, + coolDown: 0, + curator: userAddress, + maxDebt: ethers.parseEther('10'), + proxyBridge: await anotherBridgeProxy.getAddress(), + }) + + // Fund the pool with enough liquidity for the dust loans + const liquidity = ethers.parseUnits('100', 18) + await myToken.connect(user).mint(liquidity) + await myToken.connect(user).approve(await relayPool.getAddress(), liquidity) + await relayPool.connect(user).deposit(liquidity, userAddress) + }) + + it('per-message fees are truncated to zero for dust amounts', async () => { + expect( + (PER_MESSAGE_AMOUNT * bridgeFee) / FRACTIONAL_BPS_DENOMINATOR + ).to.equal(0n) + }) + + it('handles many dust messages and claim does not underflow', async () => { + // Sanity: pendingBridgeFees starts at 0 + expect(await relayPool.pendingBridgeFees()).to.equal(0) + expect( + await relayPool.accumulatedFeesByOrigin(10, relayBridgeOptimism) + ).to.equal(0) + + // Send many dust messages — per-message fee truncates to 0 each time. + for (let nonce = 1n; nonce <= MESSAGE_COUNT; nonce++) { + const recipient = ethers.Wallet.createRandom().address + await relayPool.handle( + 10, + ethers.zeroPadValue(relayBridgeOptimism, 32), + encodeData(nonce, recipient, PER_MESSAGE_AMOUNT) + ) + } + + // Per-message fees all truncate to 0, so both accumulators stay at 0. + expect(await relayPool.pendingBridgeFees()).to.equal(0) + expect( + await relayPool.accumulatedFeesByOrigin(10, relayBridgeOptimism) + ).to.equal(0) + + // The total outstanding debt is the sum of all dust amounts. + const totalBridged = MESSAGE_COUNT * PER_MESSAGE_AMOUNT + expect(await relayPool.outstandingDebt()).to.equal(totalBridged) + + // Compute what the legacy buggy code would have subtracted at claim time. + const batchFee = (totalBridged * bridgeFee) / FRACTIONAL_BPS_DENOMINATOR + expect(batchFee).to.be.greaterThan(0n) // batch fee IS non-zero + + // Bridge settles all the dust at once. + await myToken.transfer(await bridgeProxy.getAddress(), totalBridged) + + // Without the fix, this call would revert with an arithmetic underflow + // panic (0 - batchFee) inside _claim. With the fix, the subtraction is + // bounded by accumulatedFeesByOrigin (= 0 here), so the actual fee + // charged is 0 and the call succeeds. + await expect(relayPool.claim(10, relayBridgeOptimism)) + .to.emit(relayPool, 'BridgeCompleted') + .withArgs(10, relayBridgeOptimism, totalBridged, 0n) + + // No fee was actually accrued on chain; pendingBridgeFees stays at 0 + // and the outstanding debt clears to 0. + expect(await relayPool.pendingBridgeFees()).to.equal(0) + expect(await relayPool.outstandingDebt()).to.equal(0) + expect( + await relayPool.accumulatedFeesByOrigin(10, relayBridgeOptimism) + ).to.equal(0) + }) + + it('does not consume fees accrued for an unrelated origin', async () => { + // First, accumulate a real fee on the optimism origin via a normal-sized message. + const normalAmount = ethers.parseUnits('1') + const recipient = ethers.Wallet.createRandom().address + await relayPool.handle( + 10, + ethers.zeroPadValue(relayBridgeOptimism, 32), + encodeData(9999n, recipient, normalAmount) + ) + const opFee = (normalAmount * bridgeFee) / FRACTIONAL_BPS_DENOMINATOR + expect(opFee).to.be.greaterThan(0n) + expect(await relayPool.pendingBridgeFees()).to.equal(opFee) + expect( + await relayPool.accumulatedFeesByOrigin(10, relayBridgeOptimism) + ).to.equal(opFee) + expect( + await relayPool.accumulatedFeesByOrigin(8453, relayBridgeBase) + ).to.equal(0) + + // Now spam dust messages on the BASE origin — per-message fees truncate to 0. + for (let nonce = 1n; nonce <= MESSAGE_COUNT; nonce++) { + const r = ethers.Wallet.createRandom().address + await relayPool.handle( + 8453, + ethers.zeroPadValue(relayBridgeBase, 32), + encodeData(nonce, r, PER_MESSAGE_AMOUNT) + ) + } + // No fees accrued for BASE origin. + expect( + await relayPool.accumulatedFeesByOrigin(8453, relayBridgeBase) + ).to.equal(0) + // pendingBridgeFees still equals the optimism fee only. + expect(await relayPool.pendingBridgeFees()).to.equal(opFee) + + // Bridge settles BASE dust. Without per-origin tracking the BASE batch + // fee would be subtracted from pendingBridgeFees, silently consuming + // fees that were owed to the OP origin's claim. + const totalBased = MESSAGE_COUNT * PER_MESSAGE_AMOUNT + await myToken.transfer(await anotherBridgeProxy.getAddress(), totalBased) + + await expect(relayPool.claim(8453, relayBridgeBase)) + .to.emit(relayPool, 'BridgeCompleted') + .withArgs(8453, relayBridgeBase, totalBased, 0n) + + // Optimism fee must still be intact. + expect(await relayPool.pendingBridgeFees()).to.equal(opFee) + expect( + await relayPool.accumulatedFeesByOrigin(10, relayBridgeOptimism) + ).to.equal(opFee) + }) + + it('correctly drains accumulated per-origin fees on a full claim', async () => { + // Take a snapshot of the current state (carried over from the previous test). + const opFeeBefore = await relayPool.accumulatedFeesByOrigin( + 10, + relayBridgeOptimism + ) + const debtBefore = ( + await relayPool.authorizedOrigins(10, relayBridgeOptimism) + ).outstandingDebt + expect(opFeeBefore).to.be.greaterThan(0n) + expect(debtBefore).to.be.greaterThan(0n) + + // Bridge settles all the optimism debt. + await myToken.transfer(await bridgeProxy.getAddress(), debtBefore) + + const expectedFee = (debtBefore * bridgeFee) / FRACTIONAL_BPS_DENOMINATOR + // The batch fee should match what we accumulated for this origin + // (the only OP message was a normal-sized one earlier; there was no + // dust on this origin in this scenario). + expect(expectedFee).to.equal(opFeeBefore) + + await expect(relayPool.claim(10, relayBridgeOptimism)) + .to.emit(relayPool, 'BridgeCompleted') + .withArgs(10, relayBridgeOptimism, debtBefore, expectedFee) + + expect( + await relayPool.accumulatedFeesByOrigin(10, relayBridgeOptimism) + ).to.equal(0) + }) +})