Increase fault tolerance of BridgeReferralFees hooks

src/hooks/BridgeReferralFees.sol

@@ -6,6 +6,7 @@ import {BuilderCodes} from "builder-codes/BuilderCodes.sol";
 import {LibString} from "solady/utils/LibString.sol";
 
 import {CampaignHooks} from "../CampaignHooks.sol";
+import {Constants} from "../Constants.sol";
 import {Flywheel} from "../Flywheel.sol";
 
 /// @title BridgeReferralFees
@@ -15,15 +16,12 @@ import {Flywheel} from "../Flywheel.sol";
 ///         allows the builder to start receiving referral fees for each usage of the code during a bridge operation that
 ///         involves a transfer of tokens.
 contract BridgeReferralFees is CampaignHooks {
-    /// @notice ERC-7528 address for native token
-    address public constant NATIVE_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
-
-    /// @notice Maximum fee basis points
-    uint16 public immutable MAX_FEE_BASIS_POINTS;
-
     /// @notice Address of the BuilderCodes contract
     BuilderCodes public immutable BUILDER_CODES;
 
+    /// @notice Maximum fee basis points, capped at ~2.5% by uint8 size
+    uint8 public immutable MAX_FEE_BASIS_POINTS;
+
     /// @notice Address of the metadata manager
     address public immutable METADATA_MANAGER;
 
@@ -33,9 +31,6 @@ contract BridgeReferralFees is CampaignHooks {
     /// @notice Error thrown to enforce only one campaign can be initialized
     error InvalidCampaignInitialization();
 
-    /// @notice Error thrown when the balance is zero
-    error ZeroBridgedAmount();
-
     /// @notice Error thrown when the caller is not authorized
     error Unauthorized();
 
@@ -49,7 +44,7 @@ contract BridgeReferralFees is CampaignHooks {
     constructor(
         address flywheel,
         address builderCodes,
-        uint16 maxFeeBasisPoints,
+        uint8 maxFeeBasisPoints,
         address metadataManager,
         string memory uriPrefix_
     ) CampaignHooks(flywheel) {
@@ -70,29 +65,28 @@ contract BridgeReferralFees is CampaignHooks {
     }
 
     /// @inheritdoc CampaignHooks
+    /// @dev User can receive new funds sent into the campaign minus an optional fee for the referring builder code
     function _onSend(address sender, address campaign, address token, bytes calldata hookData)
         internal
         view
         override
         returns (Flywheel.Payout[] memory payouts, Flywheel.Distribution[] memory fees, bool sendFeesNow)
     {
-        (address user, bytes32 code, uint16 feeBps) = abi.decode(hookData, (address, bytes32, uint16));
+        (address user, string memory code, uint8 feeBps) = abi.decode(hookData, (address, string, uint8));
 
         // Calculate bridged amount as current balance minus total fees allocated and not yet sent
-        uint256 bridgedAmount = token == NATIVE_TOKEN ? campaign.balance : IERC20(token).balanceOf(campaign);
+        uint256 bridgedAmount = token == Constants.NATIVE_TOKEN ? campaign.balance : IERC20(token).balanceOf(campaign);
         bridgedAmount -= FLYWHEEL.totalAllocatedFees(campaign, token);
 
-        // Check bridged amount nonzero
-        if (bridgedAmount == 0) revert ZeroBridgedAmount();
-
-        // Set feeBps to 0 if builder code not registered
-        feeBps = BUILDER_CODES.isRegistered(BUILDER_CODES.toCode(uint256(code))) ? feeBps : 0;
-
         // Set feeBps to MAX_FEE_BASIS_POINTS if feeBps exceeds MAX_FEE_BASIS_POINTS
         feeBps = feeBps > MAX_FEE_BASIS_POINTS ? MAX_FEE_BASIS_POINTS : feeBps;
 
+        // Determine fallback key and payout address for builder code, zero-ing fees if failed to process
+        (bool success, bytes32 fallbackKey, address payoutAddress) = _processBuilderCode(code);
+        if (!success) feeBps = 0;
+
         // Prepare payout
-        uint256 feeAmount = (bridgedAmount * feeBps) / 1e4;
+        uint256 feeAmount = _safePercent(bridgedAmount, feeBps);
         payouts = new Flywheel.Payout[](1);
         payouts[0] = Flywheel.Payout({
             recipient: user,
@@ -105,8 +99,8 @@ contract BridgeReferralFees is CampaignHooks {
             sendFeesNow = true;
             fees = new Flywheel.Distribution[](1);
             fees[0] = Flywheel.Distribution({
-                key: code, // allow fee send to fallback to builder code
-                recipient: BUILDER_CODES.payoutAddress(uint256(code)), // if payoutAddress misconfigured, builder loses their fee
+                key: fallbackKey, // allow fee send to fallback to builder code
+                recipient: payoutAddress, // if payoutAddress misconfigured, builder loses their fee
                 amount: feeAmount,
                 extraData: ""
             });
@@ -122,12 +116,15 @@ contract BridgeReferralFees is CampaignHooks {
         override
         returns (Flywheel.Distribution[] memory distributions)
     {
-        bytes32 code = bytes32(hookData);
+        // Determine key and payout address for builder code, zero-ing fees if failed to process
+        (bool success, bytes32 fallbackKey, address payoutAddress) = _processBuilderCode(string(hookData));
+        if (!success) return distributions;
+
         distributions = new Flywheel.Distribution[](1);
         distributions[0] = Flywheel.Distribution({
-            recipient: BUILDER_CODES.payoutAddress(uint256(code)),
-            key: code,
-            amount: FLYWHEEL.allocatedFee(campaign, token, code),
+            key: fallbackKey,
+            recipient: payoutAddress,
+            amount: FLYWHEEL.allocatedFee(campaign, token, fallbackKey),
             extraData: ""
         });
     }
@@ -164,4 +161,42 @@ contract BridgeReferralFees is CampaignHooks {
         if (sender != METADATA_MANAGER) revert Unauthorized();
         if (hookData.length > 0) uriPrefix = string(hookData);
     }
+
+    /// @notice Processes a builder code and returns the key and payout address
+    ///
+    /// @param code Builder code
+    ///
+    /// @dev Wraps all calls to BuilderCodes in a try/catch to handle errors gracefully.
+    /// @dev Expected errors are if the code is not valid or registered.
+    ///
+    /// @return success True if the code is valid and registered
+    /// @return fallbackKey The fallback key to allocate fees to if fee distribution fails
+    /// @return payoutAddress The payout address for the builder code
+    function _processBuilderCode(string memory code)
+        internal
+        view
+        returns (bool success, bytes32 fallbackKey, address payoutAddress)
+    {
+        // Convert code to token ID for constant-size fallback key
+        try BUILDER_CODES.toTokenId(code) returns (uint256 tokenId) {
+            // Fetch payout address for token ID
+            try BUILDER_CODES.payoutAddress(tokenId) returns (address addr) {
+                return (true, bytes32(tokenId), addr);
+            } catch {
+                return (false, bytes32(0), address(0));
+            }
+        } catch {
+            return (false, bytes32(0), address(0));
+        }
+    }
+
+    /// @notice Calculates a percentage of an amount safely, avoiding overflow
+    ///
+    /// @param amount The amount to calculate the percentage of
+    /// @param basisPoints The basis points to calculate the percentage of
+    ///
+    /// @return value The percentage of the amount
+    function _safePercent(uint256 amount, uint8 basisPoints) internal pure returns (uint256 value) {
+        return (amount / 1e4) * basisPoints + ((amount % 1e4) * basisPoints) / 1e4;
+    }
 }

test/lib/BridgeReferralFeesTest.sol

@@ -17,7 +17,7 @@ contract BridgeReferralFeesTest is Test {
     uint256 internal constant BUILDER_PK = uint256(keccak256("builder"));
     string public constant CAMPAIGN_URI = "https://base.dev/campaign/bridge-rewards";
     string public constant URI_PREFIX = "https://base.dev/campaign/bridge-rewards";
-    uint16 public constant MAX_FEE_BASIS_POINTS = 10_000; // 100%
+    uint8 public constant MAX_FEE_BASIS_POINTS = 250; // 2.5%
 
     address public owner;
     address public user;
@@ -92,4 +92,25 @@ contract BridgeReferralFeesTest is Test {
         campaign = flywheel.createCampaign(address(hooks), 0, "");
         return (hooks, campaign);
     }
+
+    /// @dev Normal percentage calculation for testing
+    function _percent(uint256 amount, uint8 basisPoints) internal pure returns (uint256) {
+        return (amount * basisPoints) / 1e4;
+    }
+
+    /// @dev Safe percentage calculation matching contract implementation
+    function _safePercent(uint256 amount, uint8 basisPoints) internal pure returns (uint256) {
+        return (amount / 1e4) * basisPoints + ((amount % 1e4) * basisPoints) / 1e4;
+    }
+
+    /// @dev Bound user address for testing
+    function _boundUser(address user) internal {
+        vm.assume(user != address(0));
+        vm.assume(user != builder);
+        vm.assume(user != address(builderCodes));
+        vm.assume(user != address(bridgeReferralFees));
+        vm.assume(user != bridgeReferralFeesCampaign);
+        vm.assume(user.code.length == 0);
+        vm.assume(uint160(user) > 256);
+    }
 }

test/unit/hooks/BridgeReferralFees.t.sol

@@ -84,29 +84,17 @@ contract BridgeReferralFeesTest is Test {
         flywheel.createCampaign(address(bridgeReferralFees), 0, "invalid");
     }
 
-    function test_onSend_revert_zeroAmount(uint16 feeBps) public {
-        // Prepare hook data
-        bytes32 code = bytes32(builderCodes.toTokenId(TEST_CODE));
-        bytes memory hookData = abi.encode(user, code, feeBps);
-
-        // Should revert when campaign has zero balance
-        vm.expectRevert(abi.encodeWithSelector(BridgeReferralFees.ZeroBridgedAmount.selector));
-        flywheel.send(bridgeReferralFeesCampaign, address(usdc), hookData);
-    }
-
-    function test_onSend_success(uint256 bridgedAmount, uint16 feeBps) public {
+    function test_onSend_success(uint256 bridgedAmount, uint8 feeBps) public {
         // Fund the campaign
         vm.assume(bridgedAmount > 0);
         usdc.mint(bridgeReferralFeesCampaign, bridgedAmount);
 
-        // Prepare hook data with 1% fee
+        // Prepare hook data with fee
         vm.assume(feeBps > 0);
         vm.assume(feeBps <= bridgeReferralFees.MAX_FEE_BASIS_POINTS());
-        vm.assume(bridgedAmount < type(uint256).max / feeBps);
-        bytes32 code = bytes32(builderCodes.toTokenId(TEST_CODE));
-        bytes memory hookData = abi.encode(user, code, feeBps);
+        bytes memory hookData = abi.encode(user, TEST_CODE, feeBps);
 
-        uint256 feeAmount = (bridgedAmount * feeBps) / 1e4;
+        uint256 feeAmount = (bridgedAmount / 1e4) * feeBps + ((bridgedAmount % 1e4) * feeBps) / 1e4;
         uint256 userAmount = bridgedAmount - feeAmount;
 
         // Record balances before
@@ -128,9 +116,8 @@ contract BridgeReferralFeesTest is Test {
         usdc.mint(bridgeReferralFeesCampaign, bridgedAmount);
 
         // Prepare hook data with 0% fee
-        uint16 feeBps = 0;
-        bytes32 code = bytes32(builderCodes.toTokenId(TEST_CODE));
-        bytes memory hookData = abi.encode(user, code, feeBps);
+        uint8 feeBps = 0;
+        bytes memory hookData = abi.encode(user, TEST_CODE, feeBps);
 
         // Record balances before
         uint256 userBalanceBefore = usdc.balanceOf(user);
@@ -145,13 +132,13 @@ contract BridgeReferralFeesTest is Test {
         assertEq(usdc.balanceOf(bridgeReferralFeesCampaign), 0, "Campaign should be empty");
     }
 
-    function test_onSend_success_builderCodeNotRegistered(uint256 bridgedAmount, uint16 feeBps) public {
+    function test_onSend_success_builderCodeNotRegistered(uint256 bridgedAmount, uint8 feeBps) public {
         // Fund the campaign
         vm.assume(bridgedAmount > 0);
         usdc.mint(bridgeReferralFeesCampaign, bridgedAmount);
 
-        // Prepare hook data with 1% fee
-        bytes32 unregisteredCode = bytes32(builderCodes.toTokenId("unregistered"));
+        // Prepare hook data with fee
+        string memory unregisteredCode = "unregistered";
         bytes memory hookData = abi.encode(user, unregisteredCode, feeBps);
 
         // Record balances before
@@ -167,19 +154,17 @@ contract BridgeReferralFeesTest is Test {
         assertEq(usdc.balanceOf(bridgeReferralFeesCampaign), 0, "Campaign should be empty");
     }
 
-    function test_onSend_success_feeBasisPointsTooHigh(uint256 bridgedAmount, uint16 feeBps) public {
+    function test_onSend_success_feeBasisPointsTooHigh(uint256 bridgedAmount, uint8 feeBps) public {
         // Fund the campaign
         vm.assume(bridgedAmount > 0);
         usdc.mint(bridgeReferralFeesCampaign, bridgedAmount);
 
         // Use fee higher than maximum (2%)
-        uint16 maxFeeBps = bridgeReferralFees.MAX_FEE_BASIS_POINTS();
+        uint8 maxFeeBps = bridgeReferralFees.MAX_FEE_BASIS_POINTS();
         vm.assume(feeBps > maxFeeBps);
-        vm.assume(bridgedAmount < type(uint256).max / maxFeeBps);
-        bytes32 code = bytes32(builderCodes.toTokenId(TEST_CODE));
-        bytes memory hookData = abi.encode(user, code, feeBps);
+        bytes memory hookData = abi.encode(user, TEST_CODE, feeBps);
 
-        uint256 feeAmount = (bridgedAmount * maxFeeBps) / 1e4;
+        uint256 feeAmount = (bridgedAmount / 1e4) * maxFeeBps + ((bridgedAmount % 1e4) * maxFeeBps) / 1e4;
         uint256 userAmount = bridgedAmount - feeAmount;
 
         // Record balances before
@@ -195,24 +180,22 @@ contract BridgeReferralFeesTest is Test {
         assertEq(usdc.balanceOf(bridgeReferralFeesCampaign), 0, "Campaign should be empty");
     }
 
-    function test_onSend_allocatedFeesNotIncludedInAvailableBalance(uint256 bridgedAmount, uint16 feeBps) public {
+    function test_onSend_allocatedFeesNotIncludedInAvailableBalance(uint256 bridgedAmount, uint8 feeBps) public {
         // Fund the campaign
-        vm.assume(bridgedAmount > 0);
+        bridgedAmount = bound(bridgedAmount, 1, type(uint256).max / 2);
         vm.deal(bridgeReferralFeesCampaign, bridgedAmount);
 
         // Prepare mock account
         MockAccount mockAccount = new MockAccount(address(0), false); // reject native token initially
         vm.prank(builder);
         builderCodes.updatePayoutAddress(TEST_CODE, address(mockAccount));
 
-        // Prepare hook data with 1% fee
+        // Prepare hook data with fee
         vm.assume(feeBps > 0);
         vm.assume(feeBps <= bridgeReferralFees.MAX_FEE_BASIS_POINTS());
-        vm.assume(bridgedAmount < type(uint256).max / 2 / feeBps);
-        bytes32 code = bytes32(builderCodes.toTokenId(TEST_CODE));
-        bytes memory hookData = abi.encode(user, code, feeBps);
+        bytes memory hookData = abi.encode(user, TEST_CODE, feeBps);
 
-        uint256 feeAmount = (bridgedAmount * feeBps) / 1e4;
+        uint256 feeAmount = (bridgedAmount / 1e4) * feeBps + ((bridgedAmount % 1e4) * feeBps) / 1e4;
         uint256 userAmount = bridgedAmount - feeAmount;
 
         // Record balances before
@@ -230,6 +213,7 @@ contract BridgeReferralFeesTest is Test {
             flywheel.totalAllocatedFees(bridgeReferralFeesCampaign, Constants.NATIVE_TOKEN),
             "Campaign should only have total allocated fees left over"
         );
+        bytes32 code = bytes32(builderCodes.toTokenId(TEST_CODE));
         assertEq(
             flywheel.totalAllocatedFees(bridgeReferralFeesCampaign, Constants.NATIVE_TOKEN),
             flywheel.allocatedFee(bridgeReferralFeesCampaign, Constants.NATIVE_TOKEN, code),
@@ -250,7 +234,7 @@ contract BridgeReferralFeesTest is Test {
 
         // Execute send
         feeBps = 0;
-        hookData = abi.encode(user, code, feeBps);
+        hookData = abi.encode(user, TEST_CODE, feeBps);
         flywheel.send(bridgeReferralFeesCampaign, Constants.NATIVE_TOKEN, hookData);
 
         // Check balances after send with no fees
@@ -327,21 +311,19 @@ contract BridgeReferralFeesTest is Test {
     //                    NATIVE TOKEN TESTS
     // =============================================================
 
-    function test_send_nativeToken_succeeds(uint256 bridgedAmount, uint16 feeBps) public {
+    function test_send_nativeToken_succeeds(uint256 bridgedAmount, uint8 feeBps) public {
         // Fund campaign with native token
         vm.assume(bridgedAmount > 0);
         vm.deal(bridgeReferralFeesCampaign, bridgedAmount);
 
         // Prepare hook data (user, code, fee)
         vm.assume(feeBps > 0);
         vm.assume(feeBps <= bridgeReferralFees.MAX_FEE_BASIS_POINTS());
-        vm.assume(bridgedAmount < type(uint256).max / feeBps);
-        bytes32 code = bytes32(builderCodes.toTokenId(TEST_CODE));
-        bytes memory hookData = abi.encode(user, code, feeBps);
+        bytes memory hookData = abi.encode(user, TEST_CODE, feeBps);
 
         // Expected amounts based on contract logic
         uint256 startingBalance = bridgeReferralFeesCampaign.balance;
-        uint256 expectedFee = (startingBalance * feeBps) / 1e4;
+        uint256 expectedFee = (startingBalance / 1e4) * feeBps + ((startingBalance % 1e4) * feeBps) / 1e4;
         uint256 expectedUser = startingBalance - expectedFee;
 
         uint256 userBefore = user.balance;

test/unit/hooks/BridgeReferralFees/constructor.t.sol

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.29;
 
+import {BridgeReferralFees} from "../../../../src/hooks/BridgeReferralFees.sol";
 import {BridgeReferralFeesTest} from "../../../lib/BridgeReferralFeesTest.sol";
 
 contract ConstructorTest is BridgeReferralFeesTest {
@@ -28,4 +29,23 @@ contract ConstructorTest is BridgeReferralFeesTest {
     function test_setsMetadataManager() public {
         assertEq(address(bridgeReferralFees.METADATA_MANAGER()), address(owner));
     }
+
+    // ========================================
+    // NEW TESTS - UINT8 MAX_FEE_BASIS_POINTS
+    // ========================================
+
+    /// @dev Verifies maxFeeBasisPoints is stored as uint8 type
+    function test_success_maxFeeBasisPoints_uint8Type() public {
+        assertEq(bridgeReferralFees.MAX_FEE_BASIS_POINTS(), MAX_FEE_BASIS_POINTS);
+        assertLe(bridgeReferralFees.MAX_FEE_BASIS_POINTS(), type(uint8).max);
+    }
+
+    /// @dev Verifies maxFeeBasisPoints respects uint8 max value (255)
+    function test_edge_maxFeeBasisPoints_uint8Boundary() public {
+        uint8 maxUint8Value = type(uint8).max;
+        BridgeReferralFees testHook =
+            new BridgeReferralFees(address(flywheel), address(builderCodes), maxUint8Value, owner, CAMPAIGN_URI);
+        assertEq(testHook.MAX_FEE_BASIS_POINTS(), maxUint8Value);
+        assertEq(testHook.MAX_FEE_BASIS_POINTS(), 255);
+    }
 }