Open
Conversation
📝 WalkthroughWalkthroughThe changes introduce subsidized relaying support to the Wormhole transceiver contract. A new feature flag enables relaying costs to be subsidized from contract balance instead of requiring message senders to pay delivery fees. The contract can now receive Ether via a fallback function and conditionally computes delivery payments based on subsidy status. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
evm/src/Transceiver/WormholeTransceiver/WormholeTransceiver.sol (1)
58-58: Consider adding a withdrawal mechanism for owner to recover subsidy funds.The
receive()function allows anyone to fund the subsidy pool, but there's no mechanism for the owner to withdraw funds. If subsidized relaying is disabled or the contract needs to be migrated, the ETH balance would be stuck.Example withdrawal function
/// `@notice` Withdraw subsidy funds from the contract. /// `@dev` This function is only callable by the `owner`. /// `@param` to The address to send the funds to. /// `@param` amount The amount of funds to withdraw. function withdrawSubsidyFunds(address payable to, uint256 amount) external onlyOwner { if (amount > address(this).balance) { revert InsufficientSubsidyBalance(amount, address(this).balance); } (bool success,) = to.call{value: amount}(""); require(success, "withdrawal failed"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/src/Transceiver/WormholeTransceiver/WormholeTransceiver.sol` at line 58, Add an owner-only withdrawal to recover ETH sent to the contract via receive(): implement a function (e.g., withdrawSubsidyFunds) gated by onlyOwner that accepts (address payable to, uint256 amount), checks amount <= address(this).balance and reverts with InsufficientSubsidyBalance if not, then performs a safe call to transfer the amount and reverts on failure; reference the existing receive() fallback, the onlyOwner modifier, and the InsufficientSubsidyBalance error to locate where to add and align behavior with the contract's ownership and error patterns.evm/test/IntegrationStandalone.t.sol (1)
300-330: Consider adding assertions to verify the subsidy was used and the transfer completed.The test validates that the quote is zero and the transfer call succeeds, but doesn't verify:
- The transceiver's balance decreased (proving the subsidy was consumed)
- The token transfer actually completed (userA balance decreased, NttManager received tokens)
Proposed enhancement to add balance assertions
vm.startPrank(userA); token1.approve(address(nttManagerChain1), sendingAmount); + + uint256 transceiverBalanceBefore = address(wormholeTransceiverChain1).balance; + uint256 userBalanceBefore = token1.balanceOf(address(userA)); + nttManagerChain1.transfer( sendingAmount, chainId2, bytes32(uint256(uint160(userB))), bytes32(uint256(uint160(userA))), false, instructions ); + + // Verify subsidy was consumed (wormhole message fee paid from contract) + assertLt( + address(wormholeTransceiverChain1).balance, + transceiverBalanceBefore, + "transceiver balance should decrease" + ); + + // Verify token transfer occurred + assertEq( + token1.balanceOf(address(userA)), + userBalanceBefore - sendingAmount, + "user balance should decrease" + ); + vm.stopPrank(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/test/IntegrationStandalone.t.sol` around lines 300 - 330, Extend test_subsidizedRelaying_allowsZeroValueTransfer to assert the subsidy was consumed and the transfer completed: before funding, record address(wormholeTransceiverChain1).balance and token1.balanceOf(userA) (and token1.balanceOf(address(nttManagerChain1)) if NttManager should hold the tokens), then after the transfer assert the transceiver balance decreased by the gas/fee amount and userA's token balance decreased by sendingAmount while nttManagerChain1's token balance increased accordingly; reference wormholeTransceiverChain1, token1 (DummyToken), nttManagerChain1, userA and userB to locate the variables and checks to add.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@evm/src/interfaces/IWormholeTransceiverState.sol`:
- Around line 34-38: The interface is missing the getter and setter declarations
for subsidized relaying and documents the wrong Topic0 hash; add function
declarations isSubsidizedRelayingEnabled() external view returns (bool) and
setIsSubsidizedRelayingEnabled(bool isEnabled) external to the interface
(matching the implementations in WormholeTransceiverState) and update the event
comment for SetIsSubsidizedRelayingEnabled(bool) to use the correct Topic0 hash
0x137fed7331b9937fa0c97e8a96a905d05e2efa75614a2b3cb247231bc6570b05.
---
Nitpick comments:
In `@evm/src/Transceiver/WormholeTransceiver/WormholeTransceiver.sol`:
- Line 58: Add an owner-only withdrawal to recover ETH sent to the contract via
receive(): implement a function (e.g., withdrawSubsidyFunds) gated by onlyOwner
that accepts (address payable to, uint256 amount), checks amount <=
address(this).balance and reverts with InsufficientSubsidyBalance if not, then
performs a safe call to transfer the amount and reverts on failure; reference
the existing receive() fallback, the onlyOwner modifier, and the
InsufficientSubsidyBalance error to locate where to add and align behavior with
the contract's ownership and error patterns.
In `@evm/test/IntegrationStandalone.t.sol`:
- Around line 300-330: Extend test_subsidizedRelaying_allowsZeroValueTransfer to
assert the subsidy was consumed and the transfer completed: before funding,
record address(wormholeTransceiverChain1).balance and token1.balanceOf(userA)
(and token1.balanceOf(address(nttManagerChain1)) if NttManager should hold the
tokens), then after the transfer assert the transceiver balance decreased by the
gas/fee amount and userA's token balance decreased by sendingAmount while
nttManagerChain1's token balance increased accordingly; reference
wormholeTransceiverChain1, token1 (DummyToken), nttManagerChain1, userA and
userB to locate the variables and checks to add.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
evm/src/Transceiver/WormholeTransceiver/WormholeTransceiver.solevm/src/Transceiver/WormholeTransceiver/WormholeTransceiverState.solevm/src/interfaces/IWormholeTransceiverState.solevm/test/IntegrationStandalone.t.sol
Comment on lines
+34
to
+38
| /// @notice Emitted when subsidized relaying is enabled or disabled. | ||
| /// @dev Topic0 | ||
| /// 0x1df7b0a641c11c6a0b6c1d1dbd5ed0a9f2e5f9df58f9cb6d3a15d5c7c80d9f15. | ||
| /// @param isEnabled A boolean indicating whether subsidized relaying is enabled. | ||
| event SetIsSubsidizedRelayingEnabled(bool isEnabled); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the interface and implementation files
find . -type f -name "IWormholeTransceiverState.sol" -o -name "WormholeTransceiverState.sol" | head -20Repository: wormhole-foundation/native-token-transfers
Length of output: 207
🏁 Script executed:
# Read the interface file to check for function declarations
cat -n ./evm/src/interfaces/IWormholeTransceiverState.solRepository: wormhole-foundation/native-token-transfers
Length of output: 7217
🏁 Script executed:
# Read the implementation file to check if these functions exist
cat -n ./evm/src/Transceiver/WormholeTransceiver/WormholeTransceiverState.solRepository: wormhole-foundation/native-token-transfers
Length of output: 14106
🏁 Script executed:
# Verify Topic0 hash for SetIsSubsidizedRelayingEnabled(bool)
# Using python with a keccak256 library since sha3sum may not be available
python3 <<'EOF'
from Crypto.Hash import keccak
signature = "SetIsSubsidizedRelayingEnabled(bool)"
k = keccak.new(digest_bits=256)
k.update(signature.encode('utf-8'))
topic0 = k.hexdigest()
print(f"Calculated Topic0: 0x{topic0}")
# Compare with the one in the code
expected = "0x1df7b0a641c11c6a0b6c1d1dbd5ed0a9f2e5f9df58f9cb6d3a15d5c7c80d9f15"
print(f"Expected Topic0: {expected}")
print(f"Match: {f'0x{topic0}' == expected}")
EOFRepository: wormhole-foundation/native-token-transfers
Length of output: 204
🏁 Script executed:
# Install pycryptodome and verify Topic0 hash
pip install pycryptodome -q
python3 <<'EOF'
from Crypto.Hash import keccak
signature = "SetIsSubsidizedRelayingEnabled(bool)"
k = keccak.new(digest_bits=256)
k.update(signature.encode('utf-8'))
topic0 = k.hexdigest()
print(f"Calculated Topic0: 0x{topic0}")
# Compare with the one in the code
expected = "0x1df7b0a641c11c6a0b6c1d1dbd5ed0a9f2e5f9df58f9cb6d3a15d5c7c80d9f15"
print(f"Expected Topic0: {expected}")
print(f"Match: {f'0x{topic0}' == expected}")
EOFRepository: wormhole-foundation/native-token-transfers
Length of output: 270
Add getter and setter declarations to the interface AND fix the Topic0 hash.
The interface is missing isSubsidizedRelayingEnabled() getter and setIsSubsidizedRelayingEnabled(bool) setter declarations. These functions are implemented in WormholeTransceiverState.sol (lines 224-226, 294-297) but absent from the interface, breaking the pattern established by similar features like isWormholeRelayingEnabled/setIsWormholeRelayingEnabled (lines 91-95, 121-125).
More critically, the documented Topic0 hash 0x1df7b0a641c11c6a0b6c1d1dbd5ed0a9f2e5f9df58f9cb6d3a15d5c7c80d9f15 is incorrect. The correct Topic0 for SetIsSubsidizedRelayingEnabled(bool) is 0x137fed7331b9937fa0c97e8a96a905d05e2efa75614a2b3cb247231bc6570b05.
Proposed fix
/// `@notice` Emitted when subsidized relaying is enabled or disabled.
/// `@dev` Topic0
- /// 0x1df7b0a641c11c6a0b6c1d1dbd5ed0a9f2e5f9df58f9cb6d3a15d5c7c80d9f15.
+ /// 0x137fed7331b9937fa0c97e8a96a905d05e2efa75614a2b3cb247231bc6570b05.
/// `@param` isEnabled A boolean indicating whether subsidized relaying is enabled.
event SetIsSubsidizedRelayingEnabled(bool isEnabled);
+ /// `@notice` Returns a boolean indicating whether subsidized relaying is enabled.
+ function isSubsidizedRelayingEnabled() external view returns (bool);
+
+ /// `@notice` Set whether subsidized relaying is enabled.
+ /// `@dev` This function is only callable by the `owner`.
+ /// `@param` isEnabled A boolean indicating whether subsidized relaying is enabled.
+ function setIsSubsidizedRelayingEnabled(bool isEnabled) external;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@evm/src/interfaces/IWormholeTransceiverState.sol` around lines 34 - 38, The
interface is missing the getter and setter declarations for subsidized relaying
and documents the wrong Topic0 hash; add function declarations
isSubsidizedRelayingEnabled() external view returns (bool) and
setIsSubsidizedRelayingEnabled(bool isEnabled) external to the interface
(matching the implementations in WormholeTransceiverState) and update the event
comment for SetIsSubsidizedRelayingEnabled(bool) to use the correct Topic0 hash
0x137fed7331b9937fa0c97e8a96a905d05e2efa75614a2b3cb247231bc6570b05.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests