SlabBridge.sol

This document summarizes the security-focused review of SlabBridge.sol, which bridges Slab NFTs across chains using Chainlink CCIP. The review follows a line-by-line and entry-point analysis and uses the Chainlink CCIP documentation (see llms/chainlink-ccip.txt) for context on receiver behavior, gas limits, and best practices. Findings are ordered by severity; each includes context, impact, and a recommended fix where applicable.

[HIGH] No Upfront Check That msg.value Covers Total Bridge Fees

bridgeTokens() loops over _requests, and for each request it gets the fee with router.getFee(), burns the token, then calls router.ccipSend{value: fee_}(). Only at the end does it compute refund = msg.value - totalFeeUsed and refund the sender. There is no check at the start that msg.value >= getBridgeFee(_requests).

If the user sends less than the total fee required for all requests, the transaction will revert in a later iteration when ccipSend{value: fee_} is called (the contract does not have enough native currency to send). Because the entire transaction reverts, no tokens are permanently burned and no funds are lost. However, the revert happens only after the first token(s) have been burned and the first message(s) sent in the same transaction—so the failure point is late and the revert reason may be unclear (e.g. coming from the router or low-level send).

CCIP context: The docs state that users must pay fees (here in native currency) and that the gas limit in extraArgs must be sufficient for ccipReceive() on the destination. Failing mid-loop makes debugging and UX worse.

Recommended Solution

Add an upfront check so the transaction fails fast with a clear error and no burns:

function bridgeTokens(BridgeRequest[] calldata _requests)
    external
    payable
    whenNotPaused
    returns (bytes32[] memory messageIds_)
{
    uint length = _requests.length;
    if (length == 0) revert EmptyBridgeRequests();

    uint totalFeeRequired = this.getBridgeFee(_requests);
    if (msg.value < totalFeeRequired) revert InsufficientFee(msg.value, totalFeeRequired);

    messageIds_ = new bytes32[](length);
    uint totalFeeUsed;
    // ... rest of function
}

Introduce an InsufficientFee(uint sent, uint required) error (or equivalent) so integrators and users get a clear, early revert when funding is too low.

[MEDIUM] Refund via call{value} Can Revert and Block Completion

After processing all bridge requests, the contract refunds excess native currency with:

uint refund = msg.value - totalFeeUsed;
if (refund > 0) {
    (bool success,) = payable(msg.sender).call{value: refund}('');
    if (!success) revert RefundTransferFailed(msg.sender, refund);
}

If msg.sender is a contract that rejects incoming ether (no receive() or fallback(), or they revert), the refund fails and the whole transaction reverts. In that case, the user has already had their tokens burned and their CCIP messages sent; the only step that failed was the refund. So the user effectively paid the correct fee and the bridge did its job, but they get a revert and may believe the bridge failed. Their tokens are already burned on the source chain and the messages are in flight, so this is a serious UX and support issue.

CCIP context: The bridge correctly uses the “refund excess” pattern; the risk is that the refund recipient cannot accept native currency.

Recommended Solution

Consider one or both of the following:

1. Pull-based refund: Emit an event (e.g. RefundAvailable(address indexed recipient, uint amount)) and allow the recipient to call a function such as claimRefund() to pull their excess. Contracts can then implement claimRefund() and forward funds as they wish.
2. Try refund, else record: If call{value: refund}('') fails, do not revert; instead emit an event (e.g. RefundFailed(address recipient, uint amount)) and leave the excess in the contract so the owner can recover it via withdrawNative() and the user can be made whole off-chain or via a separate claim mechanism.

Either approach avoids a full revert after tokens are burned and messages are sent. Document the chosen behavior for integrators.

[MEDIUM] gasLimitForReceive Has No Bounds or Zero Check

The destination execution gas limit is set in extraArgs as gasLimitForReceive (default 500,000). CCIP uses this limit when executing ccipReceive() on the destination chain. The docs state that the gas limit must be carefully chosen so that the receiver has enough gas to complete; unspent gas is not refunded, and if the limit is too low, execution fails and the message may need to be manually executed.

• Zero: setGasLimitForReceive(0) is allowed. A zero gas limit would cause destination execution to fail for every message, forcing manual execution or leaving messages stuck.
• Upper bound: CCIP documents a maximum (e.g. 3,000,000 gas on EVM destinations). The contract does not enforce an upper bound; that is acceptable but worth documenting.
• 500k default: Whether 500,000 is enough for slab.safeMint(...) plus event and decoding depends on the Slab implementation (storage, URI, etc.). If it is too low, receives will fail and depend on manual execution.

Recommended Solution

• Enforce a minimum in setGasLimitForReceive (e.g. revert if _gasLimit == 0 or _gasLimit < MIN_GAS_LIMIT_RECEIVE).
• Document the default and the fact that it must cover _ccipReceive (decode, source/remote checks, slab.safeMint, event). If in doubt, increase the default or allow per-chain overrides in a future upgrade.

[MEDIUM] Unbounded Batch Size in bridgeTokens and getBridgeFee

Both bridgeTokens() and getBridgeFee() iterate over _requests with no cap on length. A very large array can cause the transaction to run out of gas and revert. For bridgeTokens, that means the user might have intended to bridge many tokens in one tx and instead gets a revert; for getBridgeFee, off-chain or on-chain callers may hit gas limits when estimating fees for large batches.

Recommended Solution

Introduce a maximum batch size (e.g. MAX_BRIDGE_REQUESTS) and revert if _requests.length > MAX_BRIDGE_REQUESTS in both bridgeTokens and getBridgeFee. Document the limit and the rationale (gas and CCIP service limits).

[LOW] No Explicit MessageId Deduplication in _ccipReceive

_ccipReceive decodes the payload, validates the source chain and trusted remote, then mints via slab.safeMint(bridgeMessage.recipient, ...). The contract does not track which messageIds have already been processed. CCIP is designed to deliver each message once; if execution reverts, the message can be retried via manual execution. So in normal operation, a given message should not be applied twice. Still, tracking received messageIds and reverting on duplicates would harden against any future CCIP behavior or misuse (e.g. same message delivered in two different ways).

Slab’s safeMint uses isMinted[externalContract][externalTokenId], so a second mint with the same external NFT data would revert with SlabAlreadyMinted. That gives some protection if the same logical token is somehow delivered twice with the same payload, but not if the same CCIP message (same messageId) is ever processed twice with different handling.

Recommended Solution

Maintain a mapping mapping(bytes32 messageId => bool received) and set received[_message.messageId] = true at the start of _ccipReceive (after source/remote checks). If received[_message.messageId] is already true, revert with a dedicated error. This aligns with CCIP best practices around defensive receivers and makes replay behavior explicit.

[LOW] Duplicate tokenId or destChainSelector+recipient in One Batch

If the same tokenId appears in more than one BridgeRequest in a single bridgeTokens() call, the first iteration will burn that token and send a message. The second iteration will revert when checking slab.ownerOf(_tokenId) (token no longer exists) or when attempting to burn it. The whole transaction reverts, so no funds or tokens are lost, but the error is confusing. Similarly, sending the same token to two different chains in one batch is valid and supported; the only problematic case is the same tokenId used twice in the same batch.

Recommended Solution

Either document that duplicate tokenIds in one batch cause a revert, or add a check (e.g. require that each tokenId appears at most once in _requests) and revert with a clear error such as DuplicateTokenIdInBatch(uint tokenId).

[INFORMATIONAL] SlabBridge Must Be Set as Valid Minter on Slab

_ccipReceive calls slab.safeMint(...). Slab’s safeMint is protected by onlyMinter, which requires validMinter[msg.sender]. So on each destination chain, the SlabBridge contract must be registered as a valid minter via Slab.setValidCaller(bridgeAddress, true). If this is not done, every incoming bridge message will revert in _ccipReceive with CallerCannotMint, and messages would depend on manual execution without fixing the configuration.

Recommended Solution

Document this in deployment and operations runbooks. Consider an initializer or admin function that verifies slab.validMinter(address(this)) (or the equivalent view) and reverts with a clear error if the bridge is not allowed to mint, so misconfiguration is detected early.

Summary

Addressing the high-severity item (insufficient-fee check) and the medium items (refund strategy, gas limit validation, batch cap) will improve safety and operability. The low and informational items strengthen robustness and deployment clarity. The implementation is already aligned with CCIP’s trust model (router-only ccipReceive, source chain and trusted-remote checks) and with using native currency for fees and refunds.