-
Notifications
You must be signed in to change notification settings - Fork 7
feat: mock cctp relayer #299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… blocks before update
… funds from noble ica to neutron
WalkthroughThe changes introduce several modifications across multiple modules. In the Ethereum vault example, a parameter has been renamed and its usage adjusted, and the vault functionality was enhanced with additional account setup and CCTP relayer logic. Blockchain configuration parameters were updated in a JSON file. A new mock CCTP relayer module was added to simulate bidirectional cross-chain transfers. Additional utility functions were introduced, and various client libraries received minor updates, such as making fields public, adding an async method, and improving error conversions. Changes
Sequence Diagram(s)sequenceDiagram
participant Relayer as MockCctpRelayer
participant EthClient as Ethereum Client
participant NobleClient as Noble Client
Relayer->>EthClient: Poll Ethereum logs for burn events
EthClient-->>Relayer: Return event logs
Relayer->>NobleClient: Trigger mint on Noble (mint_noble)
NobleClient-->>Relayer: Acknowledge mint transaction
Relayer->>NobleClient: Poll Noble blocks for events
NobleClient-->>Relayer: Return block data
Relayer->>EthClient: Trigger mint on Ethereum (mint_evm)
EthClient-->>Relayer: Acknowledge mint transaction
Assessment against linked issues
Suggested reviewers
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (8)
e2e/src/utils/parse.rs (1)
7-35
: New helper function follows consistent pattern but has code duplicationThis new function effectively retrieves RPC addresses from logs, matching the pattern of the existing
get_grpc_address_from_logs
function. However, there's significant code duplication between these two functions.Consider extracting the common logic (file opening, JSON parsing, and chain searching) into a private helper function that both public functions can use:
+/// Internal helper to get field from chain configuration +fn get_field_from_chain_logs(target_chain_id: &str, field_name: &str) -> Result<String, Box<dyn Error>> { + // Open the logs file + let mut file = File::open(LOGS_FILE_PATH)?; + + // Read the file contents into a string + let mut contents = String::new(); + file.read_to_string(&mut contents)?; + + // Parse the string into a JSON value + let json: Value = serde_json::from_str(&contents)?; + + let chains = json["chains"] + .as_array() + .ok_or("'chains' field not found or not an array")?; + for chain in chains { + if let Some(chain_id) = chain["chain_id"].as_str() { + if chain_id == target_chain_id { + if let Some(field_value) = chain[field_name].as_str() { + return Ok(field_value.to_string()); + } else { + return Err(format!("{} not found for the specified chain", field_name).into()); + } + } + } + } + + Err(format!("Chain with ID '{}' not found in logs file", target_chain_id).into()) +} + /// Helper to get the rpc address of a chain from the local-ic logs file pub fn get_rpc_address_from_logs(target_chain_id: &str) -> Result<String, Box<dyn Error>> { - // Open the logs file - let mut file = File::open(LOGS_FILE_PATH)?; - - // Read the file contents into a string - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - - // Parse the string into a JSON value - let json: Value = serde_json::from_str(&contents)?; - - let chains = json["chains"] - .as_array() - .ok_or("'chains' field not found or not an array")?; - for chain in chains { - if let Some(chain_id) = chain["chain_id"].as_str() { - if chain_id == target_chain_id { - if let Some(rpc_address) = chain["rpc_address"].as_str() { - return Ok(rpc_address.to_string()); - } else { - return Err("rpc address not found for the specified chain".into()); - } - } - } - } - - Err(format!("Chain with ID '{}' not found in logs file", target_chain_id).into()) + get_field_from_chain_logs(target_chain_id, "rpc_address") }You can then refactor
get_grpc_address_from_logs
in a similar way.packages/chain-client-utils/src/ethereum.rs (1)
10-11
: Consider security implications of exposing the signer fieldMaking these fields public allows the Mock CCTP Relayer to access them directly, which is necessary for the implementation. However, exposing the
signer
field (which contains private key information) can have security implications if not handled carefully.Instead of making the
signer
field public, consider adding getter methods with appropriate access controls or implementing specific functionality that the relayer needs:-pub rpc_url: String, -pub signer: PrivateKeySigner, +pub rpc_url: String, +signer: PrivateKeySigner, + +// Add a method to perform specific operations that require the signer +pub fn sign_data(&self, data: &[u8]) -> Result<Signature, StrategistError> { + // Implementation that uses the signer internally +}Or if you need to expose the entire signer for legitimate reasons, document this design decision.
packages/chain-client-utils/src/cosmos/base_client.rs (2)
97-109
: Consider reusing HTTP client for efficiencyCreating a new
HttpClient
for each method call could be inefficient, especially if this method is called frequently.Consider providing a way to reuse an existing client or caching the client:
async fn block_results( &self, rpc_addr: &str, height: u32, ) -> Result<cosmrs::rpc::endpoint::block_results::Response, StrategistError> { - let client = HttpClient::new(rpc_addr)?; + // Option 1: Accept an existing client as a parameter + // let client = existing_client_or_create_new(rpc_addr)?; + + // Option 2: Use a client from the implementing struct if available + // let client = self.get_http_client(rpc_addr).await?; + + // Fallback to creating a new client + let client = HttpClient::new(rpc_addr)?; let height = Height::from(height); let results = client.block_results(height).await?; Ok(results) }
97-109
: Add documentation for the new methodThe method lacks documentation explaining its purpose, parameters, and return values.
Add documentation for the method:
+/// Retrieves block results at the specified height using the provided RPC address +/// +/// # Arguments +/// +/// * `rpc_addr` - The RPC endpoint address of the chain +/// * `height` - The block height to retrieve results for +/// +/// # Returns +/// +/// Block results containing transaction results and other block data async fn block_results( &self, rpc_addr: &str, height: u32, ) -> Result<cosmrs::rpc::endpoint::block_results::Response, StrategistError> { let client = HttpClient::new(rpc_addr)?; let height = Height::from(height); let results = client.block_results(height).await.unwrap(); Ok(results) }e2e/examples/eth_vault/vault.rs (2)
242-250
: Spawning and running the CCTP relayer in a separate runtime.Launching the relayer in parallel is a clean approach for simulating cross-chain event relaying. Consider ensuring graceful shutdown if needed, but for testing, this is acceptable.
544-545
: Fixed sleep for synchronization.Using a fixed sleep to wait on cross-chain events may introduce flaky behavior if block times or finality vary. Consider a polling-based or event-based approach for better reliability.
e2e/src/utils/mock_cctp_relayer.rs (2)
88-108
:start_relay
method for periodic polling.Spins in a loop to poll both Noble and Ethereum networks. The separate
poll_interval
is appropriate for a test environment. Consider bounded retries for production usage.
193-220
:poll_noble
method.Processes new Noble blocks from the last recorded height to the current. This approach is straightforward, though you may want to handle large block ranges carefully for performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
e2e/examples/eth_vault/program.rs
(2 hunks)e2e/examples/eth_vault/vault.rs
(7 hunks)e2e/hyperlane/config/config.json
(4 hunks)e2e/src/utils/mock_cctp_relayer.rs
(1 hunks)e2e/src/utils/mod.rs
(1 hunks)e2e/src/utils/parse.rs
(1 hunks)e2e/src/utils/vault.rs
(1 hunks)packages/chain-client-utils/Cargo.toml
(1 hunks)packages/chain-client-utils/src/cosmos/base_client.rs
(2 hunks)packages/chain-client-utils/src/cosmos/errors.rs
(1 hunks)packages/chain-client-utils/src/ethereum.rs
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/chain-client-utils/src/cosmos/base_client.rs (2)
packages/chain-client-utils/src/cosmos/errors.rs (10)
from
(11-13)from
(17-19)from
(23-25)from
(29-31)from
(35-37)from
(41-43)from
(47-49)from
(53-55)from
(59-61)from
(65-67)packages/chain-client-utils/src/cosmos/proto_timestamp.rs (1)
from
(31-33)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: setup-local-ic
- GitHub Check: build-and-deploy
🔇 Additional comments (27)
e2e/src/utils/mod.rs (1)
8-8
: Module declaration looks good!Adding the new
mock_cctp_relayer
module is appropriate and maintains consistency with other module declarations in this file.packages/chain-client-utils/src/cosmos/errors.rs (1)
10-14
: Good addition of error conversion for RPC errors!This implementation correctly converts
cosmrs::rpc::Error
toStrategistError::ClientError
, following the same pattern as other error conversions in this file. This is necessary for handling RPC errors when querying block results for CCTP events on the Noble blockchain.packages/chain-client-utils/Cargo.toml (1)
2-6
: Formatting improvements look good!The changes to align key-value pairs and improve array formatting for the features list enhance code consistency and readability.
Also applies to: 12-15, 18-25, 30-33
e2e/src/utils/vault.rs (1)
387-388
: Good simplification of receipt handling!The code now directly awaits the receipt without storing it in a variable, which is cleaner since the receipt details are no longer needed for logging. The log message has been simplified to focus on completion status rather than transaction details.
e2e/examples/eth_vault/program.rs (1)
126-126
: Parameter name change enhances clarityRenaming the parameter from
eth_admin_addr
toeth_withdraw_acc
better reflects its purpose in the code, making the function more understandable and maintaining consistency with its usage in line 175.e2e/examples/eth_vault/vault.rs (9)
32-32
: New import for CCTP relayer module.This addition cleanly integrates the mock CCTP relayer.
63-63
: Instantiation of EthereumClient with test mnemonic.This looks correct for a development/test environment. No further concerns at this time.
75-79
: Creation of separate deposit and withdrawal accounts.Introducing two distinct Valence accounts for deposit and withdrawal helps clarify ownership and improves clarity for cross-chain flows.
80-82
: Mock Token Messenger setup.The messenger contract is instantiated correctly. This aligns with the mock environment setup.
193-193
: Use of withdraw_acc_addr for Neutron libraries setup.Appropriately switches from the admin account to a dedicated withdrawal account, improving separation of concerns.
239-241
: Deployment of a mock USDC contract.The setup for the MockERC20 contract (named "USDC") is straightforward and consistent with typical test workflows.
511-511
: Completing withdraw request on vault.The invocation straightforwardly finalizes the user's withdrawal. No issues found.
521-539
: Pre-CCTP balance checks.Verifies the deposit account and Noble ICA balances before routing USDC. This is a good step to confirm initial states.
547-565
: Post-CCTP assertion checks.Ensures that balances are correctly updated after the relay. The zero-sum logic (deposit account reduced, ICA increased) matches expectations for a cross-chain transfer.
e2e/src/utils/mock_cctp_relayer.rs (8)
1-35
: Imports and constant definitions for cross-chain relayer logic.The dependencies (e.g., alloy, valence_chain_client_utils, noble & EVM clients) are organized logically. The synergy between these crates appears coherent for mock relaying.
37-47
: Struct definitions for MockCctpRelayer and RelayerState.Maintains shared relayer state (last processed block, processed events) in a mutex. This design appears robust for concurrency.
49-86
:MockCctpRelayer::new
constructor.
- Sets up NobleClient with the latest block height for referencing new events.
- Creates an EthereumClient with a derived mnemonic signer.
- Stores both in the shared state.
This is a solid initialization pattern.
110-166
:mint_evm
method for bridging tokens onto Ethereum.• Correctly queries pre-mint and post-mint balances.
• Properly mints via theMockERC20::mint
call.Recommended to ensure these minted tokens can be recognized in downstream logic.
168-191
:mint_noble
method for creating tokens on Noble.Deposits minted USDC into the specified address, then waits for transaction finality. The event-driven design is consistent with cross-chain bridging.
222-252
:poll_ethereum
method.• Uses a filter on the messenger address.
• Checks for new transaction hashes in a processed cache to avoid duplicates.Logical flow is correct for event-based bridging.
254-314
:process_noble_block
method.Identifies “circle.cctp.v1.DepositForBurn” events and extracts relevant attributes. The presence of multiple checks (amount, recipient, domain) ensures data correctness before minting.
317-330
:decode_mint_recipient_to_noble_address
utility.Converts hex format to a bech32 address. This is crucial for cross-chain address representation.
e2e/hyperlane/config/config.json (5)
17-19
: Updated Ethereum addresses for hyperlane components.Adjusts
interchainGasPaymaster
,mailbox
, andmerkleTreeHook
. Ensure these new addresses reflect the correct deployed contracts.Can you verify the correctness of these addresses using a known deployment reference or logs?
33-33
: Changing validatorAnnounce on Ethereum.Switches to a new address, presumably for the validator announcement. Looks consistent with a fresh deployment.
61-61
: Update for Neutron's interchainGasPaymaster.Be sure that any referencing code in the repository uses the new address.
63-64
: New mailbox and merkleTreeHook addresses on Neutron.These updated references should match the new on-chain deployments.
90-90
: validatorAnnounce changed for Neutron.In line with the other updates, this ensures continuity across the updated hyperlane environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
e2e/src/utils/persistence.rs (1)
44-47
: Consider using error propagation instead of expect().While the implementation correctly obtains the gRPC details through a two-step process, using
.expect()
will cause the program to panic if errors occur. For more robust error handling in production code, consider using the?
operator to propagate errors to the caller.- let grpc_addr = get_chain_field_from_local_ic_log(PERSISTENCE_CHAIN_ID, "grpc_address") - .expect("Failed to find grpc_address field for persistence chain"); - let (target_grpc_address, target_port) = get_grpc_address_and_port_from_url(&grpc_addr) - .expect("failed to unpack url and port from address {grpc_addr}"); + let grpc_addr = get_chain_field_from_local_ic_log(PERSISTENCE_CHAIN_ID, "grpc_address")?; + let (target_grpc_address, target_port) = get_grpc_address_and_port_from_url(&grpc_addr)?;e2e/examples/eth_vault/vault.rs (2)
239-250
: Starting the mock CCTP relayer
Instantiating an additional runtime to spawn the relayer works, but involves potential concurrency overhead. Also, consider using an event-based sync instead of hardcodedsleep(3)
.
544-546
: Fixed wait timing
Sleeping for 5 seconds to accommodate the relayer may suffice temporarily. For production or larger test scenarios, consider an event-based wait.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (8)
e2e/examples/eth_vault/neutron.rs
(2 hunks)e2e/examples/eth_vault/noble.rs
(1 hunks)e2e/examples/eth_vault/vault.rs
(7 hunks)e2e/examples/ica_libraries.rs
(3 hunks)e2e/src/utils/mock_cctp_relayer.rs
(1 hunks)e2e/src/utils/parse.rs
(3 hunks)e2e/src/utils/persistence.rs
(2 hunks)packages/chain-client-utils/src/cosmos/base_client.rs
(2 hunks)
🧰 Additional context used
🧬 Code Definitions (5)
e2e/examples/eth_vault/noble.rs (1)
e2e/src/utils/parse.rs (2)
get_chain_field_from_local_ic_log
(13-43)get_grpc_address_and_port_from_url
(45-60)
e2e/examples/eth_vault/neutron.rs (1)
e2e/src/utils/parse.rs (2)
get_chain_field_from_local_ic_log
(13-43)get_grpc_address_and_port_from_url
(45-60)
e2e/src/utils/persistence.rs (1)
e2e/src/utils/parse.rs (2)
get_chain_field_from_local_ic_log
(13-43)get_grpc_address_and_port_from_url
(45-60)
packages/chain-client-utils/src/cosmos/base_client.rs (2)
packages/chain-client-utils/src/cosmos/errors.rs (10)
from
(11-13)from
(17-19)from
(23-25)from
(29-31)from
(35-37)from
(41-43)from
(47-49)from
(53-55)from
(59-61)from
(65-67)packages/chain-client-utils/src/cosmos/proto_timestamp.rs (1)
from
(31-33)
e2e/examples/eth_vault/vault.rs (4)
e2e/src/utils/ethereum.rs (2)
setup_valence_account
(172-192)setup_deposit_erc20
(228-256)e2e/src/utils/vault.rs (1)
setup_mock_token_messenger
(499-515)e2e/src/utils/mock_cctp_relayer.rs (1)
new
(50-90)e2e/examples/eth_vault/strategist.rs (1)
cctp_route_usdc_from_eth
(496-519)
🔇 Additional comments (26)
e2e/src/utils/persistence.rs (1)
11-11
: Import statement update looks good.The change from a single import to two more specialized functions provides better separation of concerns for the gRPC address parsing.
packages/chain-client-utils/src/cosmos/base_client.rs (2)
13-19
: Import organization looks good.The imports are properly organized and include all necessary components for the new
block_results
method.
97-109
: Error handling improvement - good fix.The implementation of the
block_results
method now properly uses the?
operator instead ofunwrap()
, which addresses the previous review comment and prevents potential panics by properly propagating errors.This change ensures that RPC call failures are handled gracefully and returned to the caller as a
StrategistError
.e2e/examples/eth_vault/neutron.rs (2)
24-25
: Import statement update is appropriate.The updated imports align with the refactoring approach seen in other files, using more specialized functions for parsing gRPC addresses.
279-281
: Good error handling practice.This implementation correctly uses the
?
operator for error propagation, which is better practice than using.expect()
. The two-step process of first getting the gRPC address and then parsing it into URL and port components improves code readability and maintainability.e2e/examples/ica_libraries.rs (3)
16-18
: Import statement update is consistent.The import changes are consistent with the other files, reflecting a systematic refactoring approach throughout the codebase.
47-48
: Good error handling for noble chain client setup.The implementation properly uses the
?
operator to propagate errors when obtaining the gRPC details for the noble chain.
405-407
: Good error handling for neutron chain client setup.The implementation properly uses the
?
operator to propagate errors when obtaining the gRPC details for the neutron chain. This approach is consistent with the noble chain setup earlier in the file.e2e/examples/eth_vault/noble.rs (2)
9-10
: Imports for new parse functions
These added imports cleanly replace the older log-based approach, allowing more flexible retrieval of gRPC endpoints and other fields.
15-16
: Refactor from logs-based approach
Usingget_chain_field_from_local_ic_log
to retrieve the gRPC address explicitly is a good improvement in modularity and clarity.e2e/src/utils/parse.rs (3)
7-12
: More verbose doc comments
These doc comments are neatly structured, spelling out the functionality and expected fields. This clarity helps maintainers and collaborators.
13-16
: Enhanced function signature
Accepting a new param for the target field (rather than returning only the gRPC address) is a neat improvement that increases reusability.
45-46
: Refactored function for gRPC parsing
Renaming this function and passing the address explicitly is clearer than relying on a log-based approach.e2e/examples/eth_vault/vault.rs (5)
32-32
: New import for CCTP relayer
This import cleanly reflects the new usage of the mock CCTP relayer.
75-83
: Creation of deposit and withdraw accounts
Defining two separate base accounts for deposit and withdrawal scenarios is a clean design. Adding the mock CCTP messenger is also a logical step in preparing cross-chain flows.
302-309
: CCTP transfer forwarder setup
These parameters for setting up the forwarder look correct. Ensure that the inbound ICA address matches the intended cross-chain flow.
521-539
: Pre-CCTP balance checks
Verifying the deposit account’s USDC balance ensures correctness before routing. The assertions help validate the initial conditions.
547-566
: Post-CCTP balance checks
These assertions confirm that the deposit account is drained and the ICA balance is correctly updated, showing good test coverage for the cross-chain flow.e2e/src/utils/mock_cctp_relayer.rs (8)
1-41
: Imports and struct definition
All necessary dependencies are pulled in, and theMockCctpRelayer
struct is declared with a reference to both Ethereum and Noble clients. Initial scaffolding looks good.
43-47
: State tracking for the relayer
Storing the last processed block and cached events in aRelayerState
is a practical strategy to avoid double processing of events.
49-91
: Constructor logic
Creating a Noble client viaget_chain_field_from_local_ic_log
and an Ethereum client with a test mnemonic is acceptable for an e2e environment.
92-113
:start_relay
event loop
Polling both Noble and Ethereum in a loop is straightforward. Using a 2-second interval may suffice locally; watch out for performance in heavier environments.
173-196
:mint_noble
method
Nicely decodes the deposit log and callsmint_fiat
on the Noble chain. No issues spotted in logic flow.
198-225
:poll_noble
Incrementing fromlast_noble_block
to the current block in a loop covers missed events. The approach is simple and effective.
227-257
:poll_ethereum
Filtering logs, deduplicating events by transaction hash, and minting on Noble is straightforward. This concisely handles cross-chain triggers.
259-319
:process_noble_block
Captures theDepositForBurn
event and extracts attributes for cross-chain minting. The overall structure is logical and consistent with the rest of the relayer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
e2e/src/utils/parse.rs (1)
7-7
: Remove repeated “the” in documentation.Line 7’s comment has a small grammatical repetition (“in the the local-ic logs file”). Consider removing one of them for clarity.
- /// Helper to extract a given field from a given chain entry in the the local-ic logs file. + /// Helper to extract a given field from a given chain entry in the local-ic logs file.e2e/src/utils/mock_cctp_relayer.rs (2)
39-43
: Consider simplifying concurrency usage.If the relayer is not shared across multiple tasks, using
Arc<Mutex<RelayerState>>
may be unnecessary. You might simplify its design by making the relayer own the state. For multi-threaded usage, ensure all the concurrency operations are needed.
209-231
: Release the mutex lock outside the block-processing loop.Currently, the entire range is locked while iterating over multiple blocks, which may block other tasks from accessing shared state. Consider reading
noble_last_block
, dropping the lock, processing in a local scope, and reacquiring the lock only to update the last processed block at the end.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (4)
e2e/examples/eth_vault/vault.rs
(7 hunks)e2e/hyperlane/config/config.json
(4 hunks)e2e/src/utils/mock_cctp_relayer.rs
(1 hunks)e2e/src/utils/parse.rs
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/examples/eth_vault/vault.rs
- e2e/hyperlane/config/config.json
🧰 Additional context used
🧬 Code Definitions (1)
e2e/src/utils/mock_cctp_relayer.rs (3)
e2e/src/utils/parse.rs (2)
get_chain_field_from_local_ic_log
(13-45)get_grpc_address_and_port_from_url
(47-62)packages/chain-client-utils/src/ethereum.rs (2)
new
(15-24)signer
(36-38)packages/chain-client-utils/src/cosmos/base_client.rs (1)
latest_block_header
(76-95)
🔇 Additional comments (2)
e2e/src/utils/mock_cctp_relayer.rs (2)
148-148
: Validate the length of decoded recipient dataReferencing data at index 12 may panic or produce incorrect addresses if the decoded bytes are shorter than 12 bytes. Consider adding a safe length check.
let to = from_base64(mint_recipient)?; + if to.len() < 12 { + return Err("Recipient data is too short to parse an Ethereum address".into()); + } let dest_addr = Address::from_slice(&to[12..]);
338-350
: Use Noble chain prefix for Bech32 HRP.Using
NOBLE_CHAIN_ADMIN_ADDR
as the Bech32 HRP is likely incorrect. Typically, the prefix should be the Noble chain’s HRP, not the admin address. This may lead to invalid addresses.- let (hrp, _) = bech32::decode(NOBLE_CHAIN_ADMIN_ADDR)?; + // Use the chain prefix instead of the admin address + let (hrp, _) = bech32::decode(NOBLE_CHAIN_PREFIX)?;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
e2e/src/utils/mock_cctp_relayer.rs (2)
148-148
:⚠️ Potential issuePerform safe bounds checking before slicing at [12..].
This line could panic if the decoded byte array is shorter than 12 bytes. Consider verifying the length before slicing to prevent runtime errors.Apply this fix:
-let dest_addr = Address::from_slice(&to[12..]); +if to.len() < 12 { + return Err("Recipient data is too short to parse an Ethereum address".into()); +} +let dest_addr = Address::from_slice(&to[12..]);
337-349
: 🛠️ Refactor suggestionReplace usage of
NOBLE_CHAIN_ADMIN_ADDR
with the actual chain prefix.
UsingNOBLE_CHAIN_ADMIN_ADDR
as the Bech32 HRP is likely incorrect, leading to invalid addresses. Reference the chain prefix instead.-fn decode_mint_recipient_to_noble_address(mint_recipient_hex: &str) -> Result<String, Box<dyn Error>> { - let (hrp, _) = bech32::decode(NOBLE_CHAIN_ADMIN_ADDR)?; +fn decode_mint_recipient_to_noble_address( + mint_recipient_hex: &str, + noble_chain_prefix: &str, // pass in the actual chain prefix +) -> Result<String, Box<dyn Error>> { + let (hrp, _) = bech32::decode(noble_chain_prefix)?;
🧹 Nitpick comments (2)
e2e/src/utils/mock_cctp_relayer.rs (2)
59-63
: Consider referencing environment variables or config for mnemonic.
Currently, you're hardcoding a test mnemonic at [lines 86-88], which can be acceptable for local development but is risky if ever deployed beyond a dev environment. Storing private keys in environment variables or a secure secrets manager is a more secure practice, even in test code.
111-127
: Consider providing a shutdown or graceful stop mechanism.
The relayer loop runs indefinitely without any way to exit or stop unless the entire process is killed. Even if this is intentional for a mock relayer, you may want to provide a controlled shutdown path (e.g., by listening to a signal channel or exposing a stop function).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
e2e/hyperlane/config/config.json
(4 hunks)e2e/src/utils/mock_cctp_relayer.rs
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
e2e/src/utils/mock_cctp_relayer.rs (3)
e2e/src/utils/parse.rs (2)
get_chain_field_from_local_ic_log
(13-45)get_grpc_address_and_port_from_url
(47-62)packages/chain-client-utils/src/ethereum.rs (2)
new
(15-24)signer
(36-38)packages/chain-client-utils/src/cosmos/base_client.rs (1)
latest_block_header
(76-95)
🔇 Additional comments (1)
e2e/hyperlane/config/config.json (1)
17-19
: Verify the updated addresses for correctness.
The addresses forinterchainGasPaymaster
,mailbox
,merkleTreeHook
, andvalidatorAnnounce
have changed. If these are simply local or ephemeral addresses, using them is fine for test scenarios. Otherwise, ensure you are pointing to valid, expected addresses in the correct environment.Also applies to: 33-33, 61-61, 63-64, 90-90
closes #279
Description
This pr introduces the initial implementation of relaying CCTP requests between Ethereum and Noble chains.
On Ethereum side, the process tracks the events emitted by the
MockTokenMessenger
contract, processes them, and uses the event information to mint the corresponding funds on Noble.On Noble side, the process queries the block results searching for
circle.cctp.v1.DepositForBurn
event, processes it, and uses that information to mint the corresponding funds on Ethereum.Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Style