diff --git a/solidity/contracts/token/TokenBridgeCctpBase.sol b/solidity/contracts/token/TokenBridgeCctpBase.sol index b2f7cb509ea..f47c40337c0 100644 --- a/solidity/contracts/token/TokenBridgeCctpBase.sol +++ b/solidity/contracts/token/TokenBridgeCctpBase.sol @@ -366,6 +366,17 @@ abstract contract TokenBridgeCctpBase is // do not transfer to recipient as the CCTP transfer will do it } + /** + * @inheritdoc TokenRouter + * @dev Overrides to transfer fees directly from the router balance since CCTP handles token delivery. + */ + function _transferFee( + address _recipient, + uint256 _amount + ) internal override { + wrappedToken.safeTransfer(_recipient, _amount); + } + function _bridgeViaCircle( uint32 _destination, bytes32 _recipient, diff --git a/solidity/contracts/token/bridge/EverclearTokenBridge.sol b/solidity/contracts/token/bridge/EverclearTokenBridge.sol index 076c273e7cc..6339b6bb252 100644 --- a/solidity/contracts/token/bridge/EverclearTokenBridge.sol +++ b/solidity/contracts/token/bridge/EverclearTokenBridge.sol @@ -404,6 +404,17 @@ contract EverclearTokenBridge is EverclearBridge { // Do nothing (tokens transferred to recipient directly) } + /** + * @inheritdoc TokenRouter + * @dev Transfers fees directly from router balance using ERC20 transfer. + */ + function _transferFee( + address _recipient, + uint256 _amount + ) internal override { + wrappedToken._transferTo(_recipient, _amount); + } + /** * @notice Encodes the intent calldata for ETH transfers * @return The encoded calldata for the everclear intent. @@ -429,16 +440,17 @@ contract EverclearEthBridge is EverclearBridge { using Address for address payable; using TypeCasts for bytes32; + uint256 private constant SCALE = 1; + /** * @notice Constructor to initialize the Everclear ETH bridge * @param _everclearAdapter The address of the Everclear adapter contract */ constructor( IWETH _weth, - uint256 _scale, address _mailbox, IEverclearAdapter _everclearAdapter - ) EverclearBridge(_everclearAdapter, IERC20(_weth), _scale, _mailbox) {} + ) EverclearBridge(_everclearAdapter, IERC20(_weth), SCALE, _mailbox) {} /** * @inheritdoc EverclearBridge diff --git a/solidity/contracts/token/libs/TokenCollateral.sol b/solidity/contracts/token/libs/TokenCollateral.sol index 2435c467a3c..e9cf4f7b440 100644 --- a/solidity/contracts/token/libs/TokenCollateral.sol +++ b/solidity/contracts/token/libs/TokenCollateral.sol @@ -24,6 +24,7 @@ library NativeCollateral { /** * @title Handles deposits and withdrawals of WETH collateral. + * @dev TokenRouters must have `token() == address(0)` to use this library. */ library WETHCollateral { function _transferFromSender(IWETH token, uint256 _amount) internal { diff --git a/solidity/contracts/token/libs/TokenRouter.sol b/solidity/contracts/token/libs/TokenRouter.sol index a02fcee6853..7a96a3b1eb1 100644 --- a/solidity/contracts/token/libs/TokenRouter.sol +++ b/solidity/contracts/token/libs/TokenRouter.sol @@ -189,7 +189,7 @@ abstract contract TokenRouter is GasRouter, ITokenBridge { if (feeAmount > 0) { // transfer atomically so we don't need to keep track of collateral // and fee balances separately - _transferTo(_feeRecipient, feeAmount); + _transferFee(_feeRecipient, feeAmount); } remainingNativeValue = token() != address(0) ? _msgValue @@ -249,6 +249,7 @@ abstract contract TokenRouter is GasRouter, ITokenBridge { * param _recipient The address of the recipient on the destination chain. * param _amount The amount or identifier of tokens to be sent to the remote recipient * @return feeAmount The external fee amount. + * @dev This fee must be denominated in the `token()` defined by this router. * @dev The default implementation returns 0, meaning no external fees are charged. * This function is intended to be overridden by derived contracts that have additional fees. * Known overrides: @@ -362,6 +363,25 @@ abstract contract TokenRouter is GasRouter, ITokenBridge { uint256 _amountOrId ) internal virtual; + /** + * @dev Should transfer `_amount` of tokens from this token router to the fee recipient. + * @dev Called by `_calculateFeesAndCharge` when fee recipient is set and feeAmount > 0. + * @dev The default implementation delegates to `_transferTo`, which works for most token routers + * where tokens are held by the router (e.g., collateral routers, synthetic token routers). + * @dev Override this function for bridges where tokens are NOT held by the router but fees still + * need to be paid (e.g., CCTP, Everclear). In those cases, use direct token transfers from the + * router's balance collected via `_transferFromSender`. + * Known overrides: + * - TokenBridgeCctpBase: Directly transfers tokens from router balance. + * - EverclearTokenBridge: Directly transfers tokens from router balance. + */ + function _transferFee( + address _recipient, + uint256 _amount + ) internal virtual { + _transferTo(_recipient, _amount); + } + /** * @dev Scales local amount to message amount (up by scale factor). * Known overrides: diff --git a/solidity/test/token/EverclearTokenBridge.t.sol b/solidity/test/token/EverclearTokenBridge.t.sol index 4006acc3389..747a2fa75b8 100644 --- a/solidity/test/token/EverclearTokenBridge.t.sol +++ b/solidity/test/token/EverclearTokenBridge.t.sol @@ -28,6 +28,7 @@ import {IEverclearAdapter, IEverclear, IEverclearSpoke} from "../../contracts/in import {Quote} from "../../contracts/interfaces/ITokenBridge.sol"; import {TokenMessage} from "../../contracts/token/libs/TokenMessage.sol"; import {IWETH} from "contracts/token/interfaces/IWETH.sol"; +import {LinearFee} from "../../contracts/token/fees/LinearFee.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; /** @@ -400,6 +401,56 @@ contract EverclearTokenBridgeTest is Test { assertEq(sig, feeSignature); } + function testTransferRemoteWithFeeRecipient() public { + // Create a LinearFee contract as the fee recipient + // LinearFee(token, maxFee, halfAmount, owner) + address feeCollector = makeAddr("feeCollector"); + LinearFee feeContract = new LinearFee( + address(token), + 1e6, // maxFee + TRANSFER_AMT / 2, // halfAmount + feeCollector + ); + + // Set fee recipient to the LinearFee contract + vm.prank(OWNER); + bridge.setFeeRecipient(address(feeContract)); + + uint256 initialAliceBalance = token.balanceOf(ALICE); + uint256 initialFeeContractBalance = token.balanceOf( + address(feeContract) + ); + uint256 initialBridgeBalance = token.balanceOf(address(bridge)); + + // Get the expected fee from the feeContract + uint256 expectedFeeRecipientFee = feeContract + .quoteTransferRemote(DESTINATION, RECIPIENT, TRANSFER_AMT)[0].amount; + + vm.prank(ALICE); + bridge.transferRemote(DESTINATION, RECIPIENT, TRANSFER_AMT); + + // Check Alice paid the transfer amount + external fee + fee recipient fee + assertEq( + token.balanceOf(ALICE), + initialAliceBalance - + TRANSFER_AMT - + FEE_AMOUNT - + expectedFeeRecipientFee + ); + + // Check fee contract received the fee recipient fee (this tests the fix!) + assertEq( + token.balanceOf(address(feeContract)), + initialFeeContractBalance + expectedFeeRecipientFee + ); + + // Check bridge only holds the transfer amount + external fee, not the fee recipient fee + assertEq( + token.balanceOf(address(bridge)), + initialBridgeBalance + TRANSFER_AMT + FEE_AMOUNT + ); + } + function testTransferRemoteOutputAssetNotSet() public { vm.expectRevert("ETB: Output asset not set"); vm.prank(ALICE); @@ -797,10 +848,9 @@ contract EverclearTokenBridgeForkTest is BaseEverclearTokenBridgeForkTest { contract MockEverclearEthBridge is EverclearEthBridge { constructor( IWETH _weth, - uint256 _scale, address _mailbox, IEverclearAdapter _everclearAdapter - ) EverclearEthBridge(_weth, _scale, _mailbox, _everclearAdapter) {} + ) EverclearEthBridge(_weth, _mailbox, _everclearAdapter) {} bytes public lastIntent; function _createIntent( @@ -835,7 +885,6 @@ contract EverclearEthBridgeForkTest is BaseEverclearTokenBridgeForkTest { // Deploy ETH bridge implementation MockEverclearEthBridge implementation = new MockEverclearEthBridge( IWETH(ARBITRUM_WETH), - 1, address(0x979Ca5202784112f4738403dBec5D0F3B9daabB9), // Mailbox everclearAdapter ); @@ -927,7 +976,6 @@ contract EverclearEthBridgeForkTest is BaseEverclearTokenBridgeForkTest { function testEthBridgeConstructor() public { EverclearEthBridge newBridge = new EverclearEthBridge( IWETH(ARBITRUM_WETH), - 1, address(0x979Ca5202784112f4738403dBec5D0F3B9daabB9), // Mailbox everclearAdapter ); diff --git a/solidity/test/token/TokenBridgeCctp.t.sol b/solidity/test/token/TokenBridgeCctp.t.sol index 87d9c10d298..c097a1d72a5 100644 --- a/solidity/test/token/TokenBridgeCctp.t.sol +++ b/solidity/test/token/TokenBridgeCctp.t.sol @@ -339,6 +339,12 @@ contract TokenBridgeCctpV1Test is Test { vm.startPrank(user); tokenOrigin.approve(address(tbOrigin), charge); + uint256 initialUserBalance = tokenOrigin.balanceOf(user); + uint256 initialFeeContractBalance = tokenOrigin.balanceOf( + address(feeContract) + ); + uint256 initialBridgeBalance = tokenOrigin.balanceOf(address(tbOrigin)); + uint64 cctpNonce = tokenMessengerOrigin.nextNonce(); vm.expectCall( @@ -358,6 +364,27 @@ contract TokenBridgeCctpV1Test is Test { user.addressToBytes32(), amount ); + + // Verify fee recipient received the fee (tests the fix!) + assertEq( + tokenOrigin.balanceOf(address(feeContract)), + initialFeeContractBalance + feeRecipientFee, + "Fee contract should receive fee" + ); + + // Verify user was charged correctly + assertEq( + tokenOrigin.balanceOf(user), + initialUserBalance - charge, + "User should be charged transfer amount + fees" + ); + + // Verify bridge doesn't hold the fee + assertEq( + tokenOrigin.balanceOf(address(tbOrigin)), + initialBridgeBalance, + "Bridge should not hold fee recipient fee" + ); } function test_verify() public {