feat: UnionTransfer EVM library #323
Conversation
WalkthroughThis update introduces the new "UnionTransfer" EVM library variant across Rust encoder code, Solidity contracts, and documentation. It adds the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Processor
participant UnionTransfer (Library/Contract)
participant zkGM (IUnion)
participant Recipient
User->>Processor: Initiate transfer request
Processor->>UnionTransfer: Call transfer(quoteAmount)
UnionTransfer->>UnionTransfer: Validate config and amounts
UnionTransfer->>zkGM: Approve tokens and send instruction (send)
zkGM-->>Recipient: Transfer tokens on destination chain
UnionTransfer-->>Processor: Return transfer result
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
packages/encoder-utils/src/libraries/union_transfer/mod.rs (1)
1-1: Exposesolidity_typessubmodule
The file correctly exposes thesolidity_typesmodule for Union Transfer. Consider adding a documentation comment above the module declaration to briefly explain its purpose and maintain consistency with other library modules.packages/encoder-utils/src/libraries/union_transfer/solidity_types.rs (1)
3-22: Well-structured Solidity type definitions for UnionTransfer.The UnionTransferConfig struct is comprehensive, capturing all necessary parameters for union transfer operations including protocol details, token information, addresses, amounts, and metadata.
The external function definition for transfer is correctly declared with the appropriate parameter.
However, I would recommend adding documentation comments to the struct fields to make their purpose more clear, especially for complex fields like
transferTokenUnwrappingPath.sol! { struct UnionTransferConfig { + /// Protocol version for the Union transfer uint8 protocolVersion; + /// Decimals of the token being transferred uint8 transferTokenDecimals; + /// Channel ID for the transfer uint32 channelId; + /// Timeout for the transfer operation in seconds uint64 timeout; + /// Account that provides input for the transfer address inputAccount; + /// Zero-knowledge Gateway Manager address address zkGM; + /// Amount of the transfer token to send uint256 amount; + /// Amount of the quote token requested in return uint256 quoteTokenAmount; + /// Path for unwrapping the transfer token (if needed) uint256 transferTokenUnwrappingPath; + /// Recipient address encoded as bytes bytes recipient; + /// Transfer token address encoded as bytes bytes transferToken; + /// Quote token address encoded as bytes bytes quoteToken; + /// Name of the transfer token string transferTokenName; + /// Symbol of the transfer token string transferTokenSymbol; } function transfer(uint256 _quoteAmount) external; }solidity/script/UnionTransfer.script.sol (1)
99-103: Use Forge’sassertEq/vm.expectReverthelpers for clearer failures
assertgives no context on failure and is optimised out in some build modes.
Forge provides richer helpers:-assert(ubbnBalanceBefore - ubbnBalanceAfter == tokenAmount); +assertEq( + ubbnBalanceBefore - ubbnBalanceAfter, + tokenAmount, + "Incorrect amount transferred" +);The same applies to the full‑balance assertion below.
Clear messages speed up debugging when the script is run in CI.docs/src/libraries/evm/union_transfer.md (1)
68-73: Minor wording & formatting nitThe sentence mixes singular/plural and has an unmatched parenthesis.
- * @param quoteToken The token requested in return on destination chain. Bytes conversion of the token. - * For example, the quote Token for WETH on Babylon would be `0x62626e31333030736530767775653737686e36733877706836346579366435357a616634386a72766567397761667371756e636e33653473637373677664` - * which bytes conversion of "bbn1300se0vwue77hn6s8wph64ey6d55zaf48jrveg9wafsquncn3e4scssgvd" because WETH is a CW20 token on Babylon. + * @param quoteToken Bytes representation of the token requested in return on the destination chain. + * Example (WETH on Babylon): + * `0x6262…73677664` + * (bytes of `"bbn1300se0vwue77hn6s8wph64ey6d55zaf48jrveg9wafsquncn3e4scssgvd"` – WETH is a CW20 token on Babylon).solidity/src/libraries/UnionTransfer.sol (1)
7-8: Remove unusedconsoleimport to avoid extra byte‑code and silencing of compiler warnings
console.solis only helpful while debugging; it is never referenced in this contract.
Removing unused imports slightly reduces deployment size and prevents potentialunused‑importlinter violations.-import {console} from "forge-std/src/console.sol";solidity/test/libraries/UnionTransfer.t.sol (2)
286-289: Variable shadowing:transferTokenDecimalsloses thenewprefix
The destructuring introduces a localtransferTokenDecimals, yet the assertion comment still says “Transfer token decimals should be updated.”
Renaming the variable tonewTransferTokenDecimalsimproves readability and consistency with the other names.
433-474:testDecodeFungibleAssetOrderTestcannot bepurebecause it emits cheat‑code logs
console.log*introduces external calls, making the function at bestview.
Mark itview(or remove the modifier) to satisfy the compiler when optimizations or stricter settings are enabled.- function testDecodeFungibleAssetOrderTest() public pure { + function testDecodeFungibleAssetOrderTest() public view {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
contracts/encoders/evm-encoder/src/lib.rs(3 hunks)contracts/encoders/evm-encoder/src/libraries/mod.rs(1 hunks)contracts/encoders/evm-encoder/src/libraries/union_transfer.rs(1 hunks)contracts/encoders/evm-encoder/src/tests.rs(2 hunks)docs/src/SUMMARY.md(1 hunks)docs/src/libraries/evm/union_transfer.md(1 hunks)packages/encoder-utils/src/libraries/mod.rs(1 hunks)packages/encoder-utils/src/libraries/union_transfer/mod.rs(1 hunks)packages/encoder-utils/src/libraries/union_transfer/solidity_types.rs(1 hunks)solidity/script/UnionTransfer.script.sol(1 hunks)solidity/src/libraries/UnionTransfer.sol(1 hunks)solidity/src/libraries/interfaces/union/IUnion.sol(1 hunks)solidity/test/libraries/IBCEurekaTransfer.t.sol(1 hunks)solidity/test/libraries/UnionTransfer.t.sol(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: solidity contracts
- GitHub Check: setup-local-ic
- GitHub Check: build-and-deploy
🔇 Additional comments (13)
solidity/test/libraries/IBCEurekaTransfer.t.sol (1)
26-26: Confirm updated source client ID
The test now uses"cosmoshub-0"instead of the previous"cosmoshub-4". Please verify that this change aligns with the actual client identifier expected byIBCEurekaTransferand update any related documentation or fixtures if necessary.docs/src/SUMMARY.md (1)
52-52: Include Union Transfer in EVM libraries summary
The new entry forUnion Transferis correctly inserted under the EVM libraries section and follows the established indentation and ordering.packages/encoder-utils/src/libraries/mod.rs (1)
12-12: Exportunion_transfermodule
The newpub mod union_transfer;declaration correctly exposes the Union Transfer submodule inencoder-utils. Ensure the corresponding directorylibraries/union_transferand its contents follow the package conventions.contracts/encoders/evm-encoder/src/libraries/mod.rs (1)
15-15: Registerunion_transferin encoder libraries
The addition ofpub mod union_transfer;integrates the Union Transfer encoding logic into the EVM encoder’s library registry. Ensure the correspondingunion_transfer.rsfile exists and adheres to the project's formatting and naming conventions.contracts/encoders/evm-encoder/src/lib.rs (3)
8-8: Clean import addition for the new UnionTransfer module.The import has been added in a consistent manner alongside other library imports.
34-34: Good implementation of the UnionTransfer enum variant.The UnionTransfer variant is appropriately added to the EVMLibrary enum, maintaining the existing pattern.
68-68: Well-structured handler for the UnionTransfer variant.The implementation follows the established pattern of calling the module's encode function when handling the UnionTransfer variant in the encode_message method.
contracts/encoders/evm-encoder/src/tests.rs (1)
536-571: Excellent test coverage for numeric type conversions.The test thoroughly validates the interoperability between
cosmwasm_std::Uint256andalloy_primitives::U256, which is crucial for the UnionTransfer library. The test covers multiple important scenarios:
- Small integer values
- Maximum u128 values
- Values larger than u128::MAX
- Bidirectional conversion (CosmWasm → Alloy and Alloy → CosmWasm)
This ensures that numeric values are preserved correctly during type conversions used in the encoding process.
solidity/src/libraries/interfaces/union/IUnion.sol (2)
5-35: Well-defined interface for Union protocol integration.The interface clearly defines the structures needed for interacting with the Union protocol:
- The
Instructionstruct with version, opcode, and operand- The
FungibleAssetOrderwith comprehensive token transfer parameters- The
sendfunction with appropriate parametersThis interface provides a solid foundation for the UnionTransfer library implementation.
1-3:❓ Verification inconclusive
Ensure proper attribution for external code.
The comment indicates this interface is taken from the Union Labs GitHub repository. While you've included the SPDX license identifier, it's important to ensure you're complying with the original license terms when incorporating external code.
Please verify that:
- The Apache-2.0 license is compatible with your project's license
- You're meeting all attribution requirements from the original repo
- Any modifications to the original code are clearly documented
🏁 Script executed:
#!/bin/bash # Check if there's a LICENSE file in the repo and examine its contents find . -name "LICENSE" -type f -exec echo "Found license file:" {} \; -exec cat {} \; -exec echo -e "\n---\n" \; # Check the original Union repository license if it's public echo "Checking original Union repository license:" curl -s https://raw.githubusercontent.com/unionlabs/union/main/LICENSE || echo "Could not fetch license from original repo"Length of output: 11684
Verify Original Union Labs License and Attribution
The project’s
LICENSEis Apache‑2.0 (Timewave Inc.), which is compatible, but we couldn’t fetch Union Labs’ LICENSE at the provided URL. Please confirm and satisfy all Apache‑2.0 requirements for the external code:
- Confirm Union Labs’ original repo is under Apache‑2.0 (or a compatible license) and fetch its LICENSE/NOTICE files.
- Include any required NOTICE text from Union Labs in your distribution (e.g., in a NOTICE file or documentation).
- In
solidity/src/libraries/interfaces/union/IUnion.sol, retain the original copyright header
and add a prominent “Modified by Timewave Inc.” notice if you’ve changed the code.- Document all modifications to the interface (either in-file comments or in your CHANGELOG).
solidity/script/UnionTransfer.script.sol (1)
48-54:⚠️ Potential issue
abi.encodePackedloses type information – usebytes20orbytesliteral
abi.encodePacked(UBBN_ADDR)produces a variable‑lengthbyteswhose runtime length must be inferred.
BecauseUnionTransferConfig.transferTokenultimately expects the raw 20‑byte address without extra ABI length‑prefixing, it is safer to cast directly:-bytes memory transferToken = abi.encodePacked(UBBN_ADDR); +bytes memory transferToken = abi.encode(bytes20(UBBN_ADDR));(or expose
bytes20in the struct).
This avoids silent mis‑encoding if additional data is ever appended.Likely an incorrect or invalid review comment.
contracts/encoders/evm-encoder/src/libraries/union_transfer.rs (1)
66-72: 🛠️ Refactor suggestion
unwrap_or_default()breaks the “use‑config‑value” contractSpec dictates: “If
quote_amountis not passed, fall back to the configured value”.
By defaulting to zero, the encoded call will always override the config with0, silently changing semantics when the config’squoteTokenAmount≠ 0.Proposed fix:
- let transfer_call = transferCall { - _quoteAmount: alloy_primitives::U256::from_be_bytes( - quote_amount.unwrap_or_default().to_be_bytes(), - ), - }; + let amount = quote_amount + .map(|v| v.to_be_bytes()) + .unwrap_or([0u8; 32]); // leave config un‑altered (contract interprets 0 as “use‑config”) + + let transfer_call = transferCall { + _quoteAmount: alloy_primitives::U256::from_be_bytes(amount), + };Likely an incorrect or invalid review comment.
solidity/src/libraries/UnionTransfer.sol (1)
199-207: UsesafeApprovepattern or reset allowance to 0 before re‑approving
Some ERC‑20 tokens (e.g. USDT, KNC) require the allowance to be set to 0 before it can be changed.
If_config.amountvaries between transfers, the second call may revert on such tokens.
Either reset to 0 first, or useSafeERC20.forceApprovefrom OpenZeppelin.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
solidity/test/libraries/UnionTransfer.t.sol (1)
23-24: MockzkGMstill points to an address with no code – success-path tests will revert
zkGMAddressis set toaddress(3)and later cast to theIUnioninterface, but no stub contract is deployed at that location. Any call that ultimately bubbles intoIUnion.send(or similar) will fail whenUnionTransfer.transfer()executes, breakingtestTransferSucceeds*and related tests.Re-use Foundry cheat codes to deploy / mock the call instead:
- address zkGMAddress = address(3); - IUnion mockZkGM; + IUnion mockZkGM; + + /// @dev Minimal stub that always returns true for `send()` + contract ZkGMStub { + function send(bytes calldata) external returns (bool) { + return true; + } + } ... - mockZkGM = IUnion(zkGMAddress); + mockZkGM = IUnion(address(new ZkGMStub()));Tagging as a duplicate of the earlier review because the underlying issue remains.
Also applies to: 41-42
🧹 Nitpick comments (1)
solidity/test/libraries/UnionTransfer.t.sol (1)
71-262: Test suite contains heavy duplication – extract a helper to build configsEvery negative-path test recreates almost identical
UnionTransferConfigstructs, changing only one field. Extracting a private_baseConfig()factory and cloning it with modified fields will cut ~200 lines, improve readability and ease future updates when the struct changes.Example:
function _baseConfig() internal view returns (UnionTransfer.UnionTransferConfig memory cfg) { cfg = UnionTransfer.UnionTransferConfig({ amount: 1000, inputAccount: inputAccount, recipient: recipient, protocolVersion: 1, zkGM: mockZkGM, transferToken: transferToken, transferTokenName: "TEST", transferTokenSymbol: "TEST", transferTokenDecimals: 18, transferTokenUnwrappingPath: 1, quoteToken: quoteToken, quoteTokenAmount: 980, channelId: channelId, timeout: timeout }); }Then each revert test becomes a three-liner:
UnionTransfer.UnionTransferConfig memory cfg = _baseConfig(); cfg.zkGM = IUnion(address(0)); vm.expectRevert("zkGM can't be zero address"); unionTransfer.updateConfig(abi.encode(cfg));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
solidity/test/libraries/UnionTransfer.t.sol(1 hunks)
🔇 Additional comments (1)
solidity/test/libraries/UnionTransfer.t.sol (1)
263-317: Nice coverage on the happy-path configuration updateThe assertions comprehensively verify every field exposed by
config(), ensuring that encoding/decoding round-trips correctly after mutation.
First UnionTransfer library (EVM side). Next one will be the CosmWasm one.
This library is slightly more complicated than other bridges due to the amount of parameters required and how some of these are encoded. The docs try to provide examples of how all parameters must be passed and also include links to all relevant sections.
As usual, e2e tests against the real protocol and unit tests to check all validations.
Summary by CodeRabbit
New Features
Documentation
Tests