From 11aaafd36ffbb9526533ab6d59154ea6fe5627bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renaud?= Date: Thu, 30 Apr 2026 15:14:07 +0200 Subject: [PATCH 1/4] fix: prevent pendingBridgeFees underflow on truncated batch fees (DEC-820) Per-message fees in handle() are computed via integer division and can truncate to 0 for small message.amount values. The matching subtraction in claim() is computed off the *aggregated* claim amount, where the floor of the sum may exceed the sum of the per-message floors (floor(A) + floor(B) <= floor(A+B)). When the batch fee exceeds what was actually accumulated, pendingBridgeFees -= feeAmount underflows and reverts the claim, locking bridged funds for that origin until unrelated handle() calls happen to bring pendingBridgeFees back up. Track accumulated fees per (chainId, bridge) in addition to the global pendingBridgeFees, and at claim time bound the deduction by what was actually accrued for that origin. This also keeps origins from accidentally consuming each other's pending fees. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/abis/RelayPool.sol/RelayPool.json | 45 ++++ smart-contracts/contracts/RelayPool.sol | 24 +- .../test/RelayPool/fee-truncation.hardhat.ts | 248 ++++++++++++++++++ 3 files changed, 314 insertions(+), 3 deletions(-) create mode 100644 smart-contracts/test/RelayPool/fee-truncation.hardhat.ts 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..156cf41b 100644 --- a/smart-contracts/contracts/RelayPool.sol +++ b/smart-contracts/contracts/RelayPool.sol @@ -174,6 +174,15 @@ 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 Required because per-message integer truncation in handle() can sum + /// to less than the batch fee computed at claim time + /// (floor(A) + floor(B) <= floor(A+B)). Bounding the claim-time + /// subtraction by this value prevents pendingBridgeFees underflow. + /// @dev [chainId][bridgeAddress] => accumulated per-message fees + 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; @@ -586,9 +595,13 @@ contract RelayPool is ERC4626, Ownable { // Mark as processed if not messages[chainId][bridge][message.nonce] = data; - // Calculate fee using fractional basis points + // Calculate fee using fractional basis points. Per-message division + // truncates: many small messages can each produce feeAmount = 0 while + // the equivalent batch fee at claim time is non-zero. Track the + // truncated total per-origin so claim() can bound its subtraction. uint256 feeAmount = (message.amount * origin.bridgeFee) / FRACTIONAL_BPS_DENOMINATOR; + accumulatedFeesByOrigin[chainId][bridge] += feeAmount; pendingBridgeFees += feeAmount; // Check if origin settings are respected @@ -702,9 +715,14 @@ contract RelayPool is ERC4626, Ownable { uint256 feeAmount = 0; if (chargeFee) { - // The amount is the amount that was loaned + the fees - feeAmount = (amount * origin.bridgeFee) / + // The amount is the amount that was loaned + the fees. Bound the + // subtraction by the per-origin fees actually accrued in handle(), + // since the batch fee here can exceed the sum of truncated per-message + // fees and would otherwise underflow pendingBridgeFees. + 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/test/RelayPool/fee-truncation.hardhat.ts b/smart-contracts/test/RelayPool/fee-truncation.hardhat.ts new file mode 100644 index 00000000..e5c4ad03 --- /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) + }) +}) From e9d18afc2202bde043ad3138963f4e5620e51152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renaud?= Date: Thu, 30 Apr 2026 15:37:30 +0200 Subject: [PATCH 2/4] fix: prettier formatting in fee-truncation test Co-Authored-By: Claude Opus 4.7 (1M context) --- smart-contracts/test/RelayPool/fee-truncation.hardhat.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/smart-contracts/test/RelayPool/fee-truncation.hardhat.ts b/smart-contracts/test/RelayPool/fee-truncation.hardhat.ts index e5c4ad03..00c62ec7 100644 --- a/smart-contracts/test/RelayPool/fee-truncation.hardhat.ts +++ b/smart-contracts/test/RelayPool/fee-truncation.hardhat.ts @@ -108,9 +108,9 @@ describe('RelayPool: per-message fee truncation does not brick claim', () => { }) it('per-message fees are truncated to zero for dust amounts', async () => { - expect((PER_MESSAGE_AMOUNT * bridgeFee) / FRACTIONAL_BPS_DENOMINATOR).to.equal( - 0n - ) + expect( + (PER_MESSAGE_AMOUNT * bridgeFee) / FRACTIONAL_BPS_DENOMINATOR + ).to.equal(0n) }) it('handles many dust messages and claim does not underflow', async () => { From 1d94d70b851aac836129783221613b5c3025bf86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renaud?= Date: Thu, 7 May 2026 12:54:47 +0200 Subject: [PATCH 3/4] chore: trim verbose comments Co-Authored-By: Claude Opus 4.7 (1M context) --- smart-contracts/contracts/RelayPool.sol | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/smart-contracts/contracts/RelayPool.sol b/smart-contracts/contracts/RelayPool.sol index 156cf41b..a2de088c 100644 --- a/smart-contracts/contracts/RelayPool.sol +++ b/smart-contracts/contracts/RelayPool.sol @@ -175,11 +175,9 @@ contract RelayPool is ERC4626, Ownable { uint256 public pendingBridgeFees = 0; /// @notice Per-origin accumulated bridge fees not yet claimed - /// @dev Required because per-message integer truncation in handle() can sum - /// to less than the batch fee computed at claim time - /// (floor(A) + floor(B) <= floor(A+B)). Bounding the claim-time - /// subtraction by this value prevents pendingBridgeFees underflow. - /// @dev [chainId][bridgeAddress] => accumulated per-message fees + /// @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; @@ -595,10 +593,7 @@ contract RelayPool is ERC4626, Ownable { // Mark as processed if not messages[chainId][bridge][message.nonce] = data; - // Calculate fee using fractional basis points. Per-message division - // truncates: many small messages can each produce feeAmount = 0 while - // the equivalent batch fee at claim time is non-zero. Track the - // truncated total per-origin so claim() can bound its subtraction. + // Calculate fee using fractional basis points uint256 feeAmount = (message.amount * origin.bridgeFee) / FRACTIONAL_BPS_DENOMINATOR; accumulatedFeesByOrigin[chainId][bridge] += feeAmount; @@ -715,10 +710,6 @@ contract RelayPool is ERC4626, Ownable { uint256 feeAmount = 0; if (chargeFee) { - // The amount is the amount that was loaned + the fees. Bound the - // subtraction by the per-origin fees actually accrued in handle(), - // since the batch fee here can exceed the sum of truncated per-message - // fees and would otherwise underflow pendingBridgeFees. uint256 batchFee = (amount * origin.bridgeFee) / FRACTIONAL_BPS_DENOMINATOR; feeAmount = Math.min(accumulatedFeesByOrigin[chainId][bridge], batchFee); From b64c15dbd251dbf1e14cd072af15b8590ec3d831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renaud?= Date: Thu, 7 May 2026 13:02:13 +0200 Subject: [PATCH 4/4] chore: add trailing newlines to blast deployment json files Fixes prettier lint errors on these files that landed via #764. Files are not related to the DEC-820 fix in this PR; bundled here so CI goes green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../deployed_addresses.json | 2 +- .../deployed_addresses.json | 2 +- .../0x320357f5d8d107443BDF99ad325900c703Cf4b71/params.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 +}