Feat/multi network relayer #415
Conversation
Dev merge into feat/multi-network-relayer
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request introduces a new Solidity contract for batch transaction processing, updates environment and configuration files to support multiple networks and sender addresses, and refactors numerous utility modules. Key relayer functionalities have been enhanced with improved error handling, modularized network support, and updated testing. In addition, obsolete example files have been removed, and new error classes, interfaces, and event listeners have been added to better manage and log transaction relaying operations. Changes
Suggested labels
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 (
|
❌ Deploy Preview for veascan failed. Why did it fail? →
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (16)
contracts/src/utils/TransactionBatcher.sol (1)
6-23: Consider adding reentrancy protection.This contract makes external calls which could potentially lead to reentrancy attacks. Consider implementing reentrancy guards, especially since
batchSendUncheckedignores call results.+// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +// Adapted from https://github.com/daostack/web3-transaction-batcher/blob/1b88d2ea062f8f2d9fdfdf9bbe85d2bbef780151/contracts/Batcher.sol + +contract TransactionBatcher { + bool private locked; + + modifier nonReentrant() { + require(!locked, "ReentrancyGuard: reentrant call"); + locked = true; + _; + locked = false; + } + - function batchSend(address[] memory targets, uint256[] memory values, bytes[] memory datas) public payable { + function batchSend(address[] memory targets, uint256[] memory values, bytes[] memory datas) public payable nonReentrant { + require(targets.length == values.length && values.length == datas.length, "Input arrays must have the same length"); for (uint256 i = 0; i < targets.length; i++) { (bool success, ) = targets[i].call{value: values[i]}(datas[i]); if (!success) revert("transaction failed"); // All the calls must succeed. } } function batchSendUnchecked( address[] memory targets, uint256[] memory values, bytes[] memory datas - ) public payable { + ) public payable nonReentrant { + require(targets.length == values.length && values.length == datas.length, "Input arrays must have the same length"); for (uint256 i = 0; i < targets.length; i++) { targets[i].call{value: values[i]}(datas[i]); // Intentionally ignoring return value. } } }relayer-cli/src/relayer.ts (2)
30-69: Ensure robust error handling aroundinitializeNonceand network iteration.Inside the main loop (lines 33-69), if
initializeNonceor subsequent calls fail, the loop skips to the next network without logging or retry logic. Consider adding try/catch blocks or specialized exception handling to clarify errors and decide if retries or immediate shutdown is needed.
72-83: Optional: Provide a user-friendly message upon startup.When the file is run directly (line 72 onwards), consider logging a short message or instructions so users know the relayer is initializing and can watch for event-based logs. This can reduce confusion and improve the developer experience.
relayer-cli/src/utils/proof.ts (1)
88-93: ValidateRELAYER_SUBRAPGHenvironment variable.You use
process.env.RELAYER_SUBRAPGH(line 88) without confirming it’s set. Elsewhere, you throwMissingEnvironmentVariable. For consistency, consider throwing the same specialized error ifsubgraphis undefined.relayer-cli/src/utils/ethers.ts (7)
46-52: Ensure proper handling of unrecognized network.While adding
network: Networkhere is fine, if a new network value is passed (e.g., a future test environment), there's no default branch in the nested switch to handle it. This could result in silent fallthrough or returningundefined.Consider adding a default case to throw an error when the network is neither DEVNET nor TESTNET:
switch (network) { case Network.DEVNET: return VeaOutboxArbToEthDevnet__factory.connect(veaOutboxAddress, getWallet(privateKey, web3ProviderURL)); case Network.TESTNET: return VeaOutboxArbToEth__factory.connect(veaOutboxAddress, getWallet(privateKey, web3ProviderURL)); + default: + throw new Error(`Unsupported network: ${network}`); }
56-61: Prevent fallthrough whennetworkis not DEVNET or TESTNET.According to the static analysis tool, there is a potential fallthrough if no matching
networkis found. In practice, the code returns on DEVNET and TESTNET, but adding a default case or explicit error throw will make the logic more robust.🧰 Tools
🪛 Biome (1.9.4)
[error] 55-61: This case is falling through to the next case.
Add a
breakorreturnstatement to the end of this case to prevent fallthrough.(lint/suspicious/noFallthroughSwitchClause)
63-71: Extend error handling for chiado branch.Like the previous block, add a default case to avoid undefined return if an unsupported network is encountered on
chiado.
74-80: Similar network check recommended.
network: Networkis also introduced ingetVeaOutboxProvider. Consider a consistent approach to handle unknown networks by adding a default case in the nested switch statements.
84-89: Refine the testnet vs. devnet branching insepoliablock.As above, avoid falling through by returning or throwing for unknown networks to comply with best practices or lint rules.
🧰 Tools
🪛 Biome (1.9.4)
[error] 83-89: This case is falling through to the next case.
Add a
breakorreturnstatement to the end of this case to prevent fallthrough.(lint/suspicious/noFallthroughSwitchClause)
91-96: Refine the testnet vs. devnet branching inchiadoblock.Same reasoning: add a default switch case or explicit error for unrecognized networks.
🧰 Tools
🪛 Biome (1.9.4)
[error] 90-96: This case is falling through to the next case.
Add a
breakorreturnstatement to the end of this case to prevent fallthrough.(lint/suspicious/noFallthroughSwitchClause)
102-103: Consider validatingbatcherAddress.
getBatcherreturns a contract connection. If the address is invalid or the environment variable is not set, the call will proceed without warning. Optionally, throw an error ifbatcherAddressis empty or invalid to improve robustness.relayer-cli/src/utils/relay.test.ts (1)
16-28: Suggest stronger typing fornetwork.Here,
networkis cast asany(line 18). Consider usingNetworkor a more precise type to avoid type mismatches and improve maintainability.relayer-cli/src/consts/bridgeRoutes.ts (1)
61-63: Environment variable usage.Using non-null assertions (
!) on environment variables can lead to runtime errors if not set. Consider throwing a clearer error or defaulting to a fallback when these vars are missing.Also applies to: 69-71
relayer-cli/src/utils/relayerHelpers.ts (3)
43-43: Consider using asynchronous file operations.
Switching fromreadFileSyncto an asynchronous read may improve performance in high-load scenarios. That said, if synchronous reads are intentional here for simplicity, feel free to keep it.
53-61: Refine file path construction and conditional check inupdateStateFile.
WhennonceFromis not null, the file path is constructed with+, which may cause issues on certain platforms. Also consider more explicit null checks if needed.Below is a proposed change to leverage
path.joinfor consistency:- const chain_state_file = process.env.STATE_DIR + network + "_" + chainId + ".json"; + const chain_state_file = path.join(process.env.STATE_DIR || "", `${network}_${chainId}.json`);Also applies to: 71-78
137-167: Validate chain IDs before pushing torelayerNetworkConfig.
Currently, allchainIdsfrom the environment are parsed, but if they are invalid or non-numeric, runtime issues could arise. Adding a simple check (e.g., skipping or throwing ifNumber(chainId)isNaN) can improve stability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
contracts/src/utils/TransactionBatcher.sol(1 hunks)relayer-cli/.env.dist(1 hunks)relayer-cli/ecosystem.config.js(1 hunks)relayer-cli/jest.config.ts(1 hunks)relayer-cli/package.json(1 hunks)relayer-cli/src/consts/bridgeRoutes.ts(1 hunks)relayer-cli/src/devnetRelayExample.ts(0 hunks)relayer-cli/src/relayer.ts(1 hunks)relayer-cli/src/testnetRelayer.ts(0 hunks)relayer-cli/src/utils/botEvents.ts(2 hunks)relayer-cli/src/utils/errors.ts(1 hunks)relayer-cli/src/utils/ethers.ts(2 hunks)relayer-cli/src/utils/logger.ts(1 hunks)relayer-cli/src/utils/proof.ts(5 hunks)relayer-cli/src/utils/relay.test.ts(1 hunks)relayer-cli/src/utils/relay.ts(5 hunks)relayer-cli/src/utils/relayerHelpers.ts(6 hunks)relayer-cli/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (2)
- relayer-cli/src/devnetRelayExample.ts
- relayer-cli/src/testnetRelayer.ts
🧰 Additional context used
🧠 Learnings (1)
relayer-cli/src/relayer.ts (1)
Learnt from: mani99brar
PR: kleros/vea#344
File: relayer-cli/src/utils/relay.ts:78-78
Timestamp: 2025-03-20T18:46:08.116Z
Learning: In the `relayer-cli` project, the file `src/devnetRelayer.ts` is intended as an example, and suggestions to modify it can be disregarded unless specifically requested.
🧬 Code Definitions (3)
relayer-cli/src/relayer.ts (2)
relayer-cli/src/utils/relayerHelpers.ts (6)
RelayerNetworkConfig(180-180)ShutdownManager(179-179)setupExitHandlers(177-177)updateStateFile(176-176)delay(178-178)getNetworkConfig(174-174)relayer-cli/src/utils/relay.ts (2)
relayBatch(279-279)relayAllFrom(279-279)
relayer-cli/src/utils/relay.ts (4)
relayer-cli/src/consts/bridgeRoutes.ts (2)
Network(88-88)getBridgeConfig(88-88)relayer-cli/src/utils/errors.ts (3)
InvalidChainId(9-15)MissingEnvironmentVariable(17-23)DataError(25-31)relayer-cli/src/utils/ethers.ts (2)
getVeaOutbox(106-106)getBatcher(106-106)relayer-cli/src/utils/proof.ts (2)
getProofAtCount(131-131)getMessageDataToRelay(131-131)
relayer-cli/src/utils/relay.test.ts (1)
relayer-cli/src/utils/relay.ts (1)
relayBatch(279-279)
🪛 Biome (1.9.4)
relayer-cli/src/utils/ethers.ts
[error] 55-61: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 62-68: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 83-89: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 90-96: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: dependency-review
- GitHub Check: Analyze (javascript)
🔇 Additional comments (35)
relayer-cli/tsconfig.json (3)
3-5: Compiler Options Update: Module Resolution & Base Directory
The changes update the base module resolution by setting"baseUrl": ".","module": "commonjs", and"moduleResolution": "node". This configuration ensures that modules are resolved from the project root and are compatible with the Node environment, which is beneficial for the new multi-network relayer context.
7-9: Enhancements for Module Interoperability & Output Management
By adding"resolveJsonModule": trueand"allowSyntheticDefaultImports": true, the configuration improves JSON handling and simplifies default imports from modules with no default export. Additionally, specifying"outDir": "dist"directs all compiled files to a dedicated output directory. These additions help enforce strict TypeScript and build consistency.
11-13: Include Clause Addition: Explicit Source File Compilation
The addition of the"include": ["src/**/*"]array ensures that all files within thesrcdirectory are included in the compilation process. This change streamlines the build process by explicitly defining the source file scope.relayer-cli/jest.config.ts (1)
8-10: Good addition for module resolution in tests.The added
moduleNameMapperconfiguration improves Jest's ability to resolve path aliases, particularly for theconsts/imports. This is a good practice that will make tests more maintainable and supports the restructuring of the project's constants.relayer-cli/ecosystem.config.js (1)
4-11: Clean PM2 configuration update for multi-network support.The configuration has been properly updated to support the new multi-network relayer functionality:
- Renamed from "devnet-relayer" to "vea-relayer" to reflect its expanded capabilities
- Direct execution of TypeScript file instead of using yarn
- Proper TypeScript configuration with interpreter and required modules
- Appropriate environment variables for TypeScript project
This configuration aligns well with the PR objective of merging testnet and devnet relayers into a single relayer.
relayer-cli/.env.dist (2)
3-8: Well-structured sender address configuration for multi-network support.The updated sender address configuration nicely implements the PR objectives:
- Support for multiple sender addresses in devnet
- Zero address option to allow all senders
- Empty value option to disable a network
- Clear separation between devnet, testnet, and mainnet environments
This flexible approach will make the relayer more configurable and adaptable to different deployment scenarios.
13-13: Good update for multi-chain support.Replacing the single chain ID with a comma-separated list is an effective approach for supporting multiple chains, aligning with the multi-network enhancement goals.
relayer-cli/src/utils/botEvents.ts (2)
2-2: Updated comment to reflect the relayer context.The comment has been appropriately changed from "Bridger state" to "Relayer state" to better reflect the enhanced relayer functionality introduced in this PR.
16-19: Good addition of events for relay operations tracking.The new event constants for message relaying operations enhance monitoring and error handling capabilities:
RELAY_BATCHfor batch relaying operationsRELAY_ALL_FROMfor relaying all messages from a specific sourceMESSAGE_EXECUTION_FAILEDfor handling execution failuresThese events will improve logging, debugging, and error reporting for the multi-network relayer, which is essential for a system handling transactions across different networks.
relayer-cli/src/utils/logger.ts (1)
49-61: LGTM! New event listeners enhance logging for relayer operations.These new logging handlers are well-implemented and align with the PR objective of enhancing relayer functionality across multiple networks, with appropriate detail levels for batch operations, message relaying, and execution failures.
relayer-cli/package.json (2)
13-13: Script change aligns with unified relayer approach.The new
start-relayerscript replaces the previous network-specific scripts, consistent with the PR objective of mergingtestnetRelayeranddevnetRelayerinto a single relayer.
19-20:Details
❓ Verification inconclusive
Verify PM2 version compatibility.
The update from PM2 v5.2.2 to v6.0.5 represents a major version change that could potentially introduce breaking changes.
🌐 Web query:
Are there any breaking changes between PM2 v5.2.2 and v6.0.5?💡 Result:
PM2 introduced two notable breaking changes between versions 5.2.2 and 6.0.5:
Node.js Version Requirement
PM2 v6.x requires Node.js 12.x or higher[13]. This means environments using Node.js 10.x or below must upgrade to Node.js 12+ before migrating to PM2 v6.0.5.Disabled Git Parsing by Default
Git repository parsing is now disabled by default[1][10]. Projects relying on PM2’s automatic Git metadata extraction must explicitly enable it via configuration.Other key changes in v6.0.5 include enhanced Bun runtime support and FreeBSD compatibility fixes[1][10], but these are additive improvements rather than breaking changes. For most users, upgrading involves running:
npm install pm2@latest -g pm2 updateAlways validate your Node.js version and Git parsing dependencies before upgrading.
Citations:
- 1: https://github.com/Unitech/pm2/blob/master/CHANGELOG.md
- 2: https://nodejs.libhunt.com/pm2-changelog/2.6.0
- 3: https://joplinapp.org/help/about/changelog/server/
- 4: https://pm2.keymetrics.io/docs/usage/update-pm2/
- 5: https://www.npmjs.com/package/pm2/v/0.10.6?activeTab=versions
- 6: https://ww2.valleyair.org/media/gw5bacvj/2024-pm25-plan.pdf
- 7: https://nodejs.libhunt.com/pm2-changelog/2.5
- 8: https://www.jsdocs.io/package/pm2
- 9: https://www.epa.gov/sites/default/files/2019-06/documents/utilities_ria_final_cpp_repeal_and_ace_2019-06.pdf
- 10: https://github.com/Unitech/pm2/releases
- 11: https://deps.dev/npm/pm2/4.5.6/compare
- 12: https://www.myairdistrict.com/files/249e53056/Portola+Serious+SIP_FINAL_EQ_fix_20241101.pdf
- 13: https://www.npmjs.com/package/pm2
- 14: https://www.nhtsa.gov/sites/nhtsa.gov/files/2024-06/CAFE-2027-2031-HDPUV-2030-2035_Final-Environmental-Regulatory-Impact-Analysis.pdf
- 15: https://loopback.io/doc/en/lb4/changelog.cli.html
- 16: https://www.epa.gov/system/files/documents/2024-02/naaqs_pm_reconsideration_ria_final.pdf
- 17: https://security.snyk.io/package/npm/pm2
- 18: https://www.bluetooth.com/specifications/drafting-guidelines-html/
- 19: https://deps.dev/npm/pm2/4.5.1/compare
- 20: https://epa.illinois.gov/content/dam/soi/en/web/epa/public-notices/documents/general-notices/2024/regional-haze-sip-submittal20240118.pdf
Action: Confirm PM2 v6 Compatibility Requirements
The update from PM2 v5.2.2 to v6.0.5 is a major version bump that introduces two important breaking changes you need to verify:
- Node.js Version Requirement: PM2 v6 requires Node.js 12.x or higher. Please ensure that your deployment environment meets this requirement.
- Git Parsing Behavior: Git repository parsing is now disabled by default. If your project relies on PM2’s automatic Git metadata extraction, update your PM2 configuration to re-enable this feature explicitly.
File:
relayer-cli/package.json
Lines: 19-20"pm2": "^6.0.5", "typescript": "^4.9.5"Review these aspects to ensure compatibility across your environments.
relayer-cli/src/utils/errors.ts (1)
1-31: Well-structured custom error classes improve error handling.The new error classes follow a consistent pattern and provide descriptive error messages with relevant contextual information. This improves error traceability and debugging for the multi-network relayer functionality.
contracts/src/utils/TransactionBatcher.sol (1)
1-5: LGTM! Source attribution and license properly documented.The code includes appropriate SPDX license identifier and properly acknowledges the original source of the adapted implementation.
relayer-cli/src/relayer.ts (2)
57-63: Double-check the delay logic for non-DEVNET networks.You add a 100-second buffer (
+ 100 * 1000) when calculatingtimeLeft. Confirm that this buffer is intended and won’t cause unintended delay stacking over repeated runs. If you want a single buffer per epoch, that’s correct; otherwise, adjust accordingly.
1-14:Details
✅ Verification successful
Consider verifying ethers version for ZeroAddress usage.
You’re comparing
senders[0]withethers.ZeroAddressat line 42. Note that in ethers v5, the equivalent constant isethers.constants.AddressZero. If you are on ethers v6, you’re good. Otherwise, this may cause a runtime error.Run the following script to confirm your ethers version and search for the usage of ZeroAddress vs. AddressZero:
🏁 Script executed:
#!/bin/bash # Description: Verify if the codebase uses ethers v6 or v5 by checking package.json and searching references to ZeroAddress/AddressZero. # 1. Search package.json for ethers dependency fd package.json --exec cat {} | jq '.dependencies.ethers,.devDependencies.ethers' # 2. Search code references to ZeroAddress or AddressZero rg 'ZeroAddress|AddressZero' -A 2Length of output: 13270
Verified: Ethers v6 is in use and the usage of
ethers.ZeroAddressis correct.The codebase’s package.json specifies
"^6.13.5"for ethers, and our search confirms that all references useethers.ZeroAddressconsistently. There’s no need to switch toethers.constants.AddressZerosince ethers v6 is active.relayer-cli/src/utils/proof.ts (1)
27-38: Increase themessageSentsquery limit or confirm that 5 is sufficient.You have
messageSents(first: 5, where: {nonce: ${nonce}, inbox: "${inbox}"}). If more than five messages match the criteria, the query will miss them. If only a single message is expected per nonce, that’s fine. Otherwise, consider raising the limit or adding pagination.relayer-cli/src/utils/relay.ts (2)
45-50: Applaud usage of specialized error classes.Using
InvalidChainIdandMissingEnvironmentVariableclarifies failure points and makes debugging easier. This is a solid approach to explicit error handling.
257-277: Caution with batching up to 1000 messages in a single query.You query up to the first 1000 messages with
messageSents(first: 1000, ... ). If more than 1000 messages exist for a single sender, they won’t be returned. Confirm whether this limit is acceptable. If not, consider paginated queries or a larger limit.relayer-cli/src/utils/ethers.ts (3)
8-9: Add a brief rationale for new contract factories.The addition of
VeaOutboxArbToGnosisDevnet__factoryandTransactionBatcher__factoryis consistent with supporting multiple networks and introducing the TransactionBatcher for batch processing. This looks good.
11-11: Explicit import ofNetworkis helpful.Importing
NetworkfrombridgeRoutesclarifies whether a devnet or testnet is targeted. No issues here.
106-106: Export changes aligned with new multi-network approach.Removing obsolete exports and adding
getBatchermatches the updated architecture. Looks consistent with the rest of the code.relayer-cli/src/utils/relay.test.ts (5)
1-12: MockEmitter simplifies testing by suppressing unwanted logs.Replacing standard logging for
BotEventswith a custom emitter is a clean approach to reduce noise in tests. Nicely done.
30-34: Consistent usage of mocks.Centralizing mock definitions in the
beforeEachblock clarifies test setup. This improves readability and helps isolate test cases.
100-117: Good coverage for single-message relay flow.The new test ensures only one message is relayed, with correct gas estimation. This is a solid check for the minimal case.
143-174: Comprehensive multi-batch testing is thorough.Testing a 15-message scenario that splits into two batches shows strong coverage for chunked relays. Great job ensuring the logic handles bigger sets.
176-200: Skipping already-relayed messages is validated.This test covers the scenario of partial prior relays. Confirming that only unrelayed messages are included is crucial. Nicely done.
relayer-cli/src/consts/bridgeRoutes.ts (2)
18-19: Structured approach toveaContractsis clear.Switching from individual addresses to a
VeaContractsinterface for devnet/testnet helps keep references organized. This is a positive design choice.Also applies to: 23-26, 28-31
76-80: Improved error handling for unsupported chain IDs.Throwing errors when a chain ID is unknown or an epochPeriod is missing makes the code more robust. This aligns well with the multi-network strategy.
Also applies to: 83-85
relayer-cli/src/utils/relayerHelpers.ts (6)
2-2: No concerns with these new imports.
They properly bring in thepathmodule for cross-platform path handling and correctly reference the newly introduced error and network enum.Also applies to: 7-8
17-19: Verify handling ofnullreturn ininitialize.
The function now returnsPromise<number | null>, which could introduce ambiguity whennullis returned. Ensure that all callers handle thenullcase correctly (e.g., skipping relaying, throwing an error, etc.).Also applies to: 28-28
33-34: Good use ofpath.joinfor cross-platform consistency.
This ensures that thestateFilepath is constructed reliably across different environments.
84-90: Documentation improvements forsetupExitHandlerslook good.
Clear param descriptions make the function usage more transparent.
131-136:RelayerNetworkConfigtype addition is straightforward and well-structured.
The naming is consistent. No issues found.
173-181: Modular export structure looks consistent.
Exporting newly introduced functions and types ensures a clear API boundary.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
relayer-cli/src/utils/proof.ts (4)
27-27: Environment variable should have validationYou're directly using
process.env.RELAYER_SUBGRAPHwithout validating if it exists. If this environment variable is not set, it could lead to runtime errors.- const subgraph = process.env.RELAYER_SUBGRAPH; + const subgraph = process.env.RELAYER_SUBGRAPH; + if (!subgraph) { + throw new Error("RELAYER_SUBGRAPH environment variable is not set"); + }
45-45: Improve error handling with more contextSimply returning
undefinedwithout context makes debugging harder for callers. Consider adding more information about the error.- console.log(e); - return undefined; + console.error("Failed to get message data to relay:", e); + return undefined;
88-92: Environment variable validation needed here tooSimilar to the earlier instance, you're using
process.env.RELAYER_SUBGRAPHwithout validation.- const subgraph = process.env.RELAYER_SUBGRAPH; + const subgraph = process.env.RELAYER_SUBGRAPH; + if (!subgraph) { + throw new Error("RELAYER_SUBGRAPH environment variable is not set"); + }
98-101: Improve error handling with more contextLike in
getMessageDataToRelay, consider adding more context to the error logging to make debugging easier.- console.log(e); + console.error("Failed to get proof at count:", e);relayer-cli/src/utils/relay.ts (5)
167-214: Handle null lastNonce case more explicitly.The function could return null+1 if no messages are relayed, which evaluates to 1 but might not be the intended behavior.
- return lastNonce + 1; // return current nonce + return lastNonce !== null ? lastNonce + 1 : null; // return current nonce or null if no messages were relayed
126-130: Add null check for messageData in relayBatch.Similar to the check added in the relay function, you should verify messageData is not null before destructuring it.
const [proof, messageData] = await Promise.all([ fetchProofAtCount(chainId, nonce, count, veaInboxAddress), fetchMessageDataToRelay(chainId, veaInboxAddress, nonce), ]); + if (!messageData) { + emitter.emit(BotEvents.MESSAGE_EXECUTION_FAILED, nonce); + nonce++; + continue; + } const [to, data] = messageData;
192-196: Add null check for messageData in relayAllFrom.Add validation for messageData before destructuring to prevent potential runtime errors.
const [proof, messageData] = await Promise.all([ getProofAtCount(chainId, x, count, veaInboxAddress), getMessageDataToRelay(chainId, veaInboxAddress, x), ]); + if (!messageData) { + emitter.emit(BotEvents.MESSAGE_EXECUTION_FAILED, x); + continue; + } const [to, data] = messageData;
207-208: Consider adding a safety margin to gas estimation.For batch transactions, it's a good practice to add a small safety margin to the estimated gas to account for network conditions and slight variations in execution cost.
- const gasLimit = await batcher.batchSend.estimateGas(targets, values, datas); - const tx = await batcher.batchSend(targets, values, datas, { gasLimit }); + const estimatedGas = await batcher.batchSend.estimateGas(targets, values, datas); + const gasLimit = Math.floor(estimatedGas * 1.1); // Add 10% safety margin + const tx = await batcher.batchSend(targets, values, datas, { gasLimit });
145-146: Apply the same gas estimation safety margin here.Similar to the suggestion for relayAllFrom, consider adding a safety margin to the gas estimation in relayBatch.
- const gasLimit = await batcher.batchSend.estimateGas(targets, values, datas); - const tx = await batcher.batchSend(targets, values, datas, { gasLimit }); + const estimatedGas = await batcher.batchSend.estimateGas(targets, values, datas); + const gasLimit = Math.floor(estimatedGas * 1.1); // Add 10% safety margin + const tx = await batcher.batchSend(targets, values, datas, { gasLimit });relayer-cli/src/consts/bridgeRoutes.ts (3)
23-26: Consider specifying a more precise type instead ofany.Using
anyforveaInboxandveaOutboxproperties reduces type safety benefits. Consider creating a specific interface or type that reflects the structure of these contract artifacts.type VeaContracts = { - veaInbox: any; - veaOutbox: any; + veaInbox: { + address: string; + abi: any[]; + // other relevant properties + }; + veaOutbox: { + address: string; + abi: any[]; + // other relevant properties + }; };
82-86: Verify chain ID availability before accessing properties.The error check could be simplified and made more robust by checking for the bridge's existence first:
const getEpochPeriod = (chainId: number): number => { const bridge = bridges[chainId]; - if (!bridge.epochPeriod) throw new Error(`Unsupported chainId: ${chainId}`); + if (!bridge) throw new Error(`Unsupported chainId: ${chainId}`); + if (!bridge.epochPeriod) throw new Error(`No epoch period defined for chainId: ${chainId}`); return bridge.epochPeriod; };
3-12: Consider adding more descriptive comments for imports.With multiple contract imports that have similar names, adding comments to describe the purpose or context of each import would improve readability and maintainability.
// File for handling contants and configurations require("dotenv").config(); +// Arbitrum to Ethereum bridge contracts - Devnet import veaInboxArbToEthDevnet from "@kleros/vea-contracts/deployments/arbitrumSepolia/VeaInboxArbToEthDevnet.json"; import veaOutboxArbToEthDevnet from "@kleros/vea-contracts/deployments/sepolia/VeaOutboxArbToEthDevnet.json"; +// Arbitrum to Ethereum bridge contracts - Testnet import veaInboxArbToEthTestnet from "@kleros/vea-contracts/deployments/arbitrumSepolia/VeaInboxArbToEthTestnet.json"; import veaOutboxArbToEthTestnet from "@kleros/vea-contracts/deployments/sepolia/VeaOutboxArbToEthTestnet.json"; +// Arbitrum to Gnosis bridge contracts - Devnet import veaInboxArbToGnosisDevnet from "@kleros/vea-contracts/deployments/arbitrumSepolia/VeaInboxArbToGnosisDevnet.json"; import veaOutboxArbToGnosisDevnet from "@kleros/vea-contracts/deployments/chiado/VeaOutboxArbToGnosisDevnet.json"; +// Arbitrum to Gnosis bridge contracts - Testnet import veaInboxArbToGnosisTestnet from "@kleros/vea-contracts/deployments/arbitrumSepolia/VeaInboxArbToGnosisTestnet.json"; import veaOutboxArbToGnosisTestnet from "@kleros/vea-contracts/deployments/chiado/VeaOutboxArbToGnosisTestnet.json";relayer-cli/src/relayer.ts (2)
1-3: Consider providing a fallback if.envis missing.
Currently, all environment variables rely on loading from.env. If this file is absent, the relayer might crash or behave unexpectedly. Implementing fallback checks or default values can help improve robustness.
84-95: Consider separating the CLI entry point.
While it’s convenient to have the relayer automatically start in the same file, placing this block in a dedicated CLI or main file can keep the module lean and easier to test or reuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (6)
relayer-cli/.env.dist(1 hunks)relayer-cli/src/consts/bridgeRoutes.ts(1 hunks)relayer-cli/src/relayer.ts(1 hunks)relayer-cli/src/utils/ethers.ts(2 hunks)relayer-cli/src/utils/proof.ts(5 hunks)relayer-cli/src/utils/relay.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- relayer-cli/src/utils/ethers.ts
🧰 Additional context used
🧠 Learnings (1)
relayer-cli/src/relayer.ts (1)
Learnt from: mani99brar
PR: kleros/vea#344
File: relayer-cli/src/utils/relay.ts:78-78
Timestamp: 2025-03-20T18:46:08.116Z
Learning: In the `relayer-cli` project, the file `src/devnetRelayer.ts` is intended as an example, and suggestions to modify it can be disregarded unless specifically requested.
🧬 Code Definitions (2)
relayer-cli/src/utils/relay.ts (4)
relayer-cli/src/consts/bridgeRoutes.ts (2)
Network(88-88)getBridgeConfig(88-88)relayer-cli/src/utils/errors.ts (3)
InvalidChainId(9-15)MissingEnvironmentVariable(17-23)DataError(25-31)relayer-cli/src/utils/ethers.ts (2)
getVeaOutbox(117-117)getBatcher(117-117)relayer-cli/src/utils/proof.ts (2)
getProofAtCount(131-131)getMessageDataToRelay(131-131)
relayer-cli/src/relayer.ts (2)
relayer-cli/src/utils/relayerHelpers.ts (6)
RelayerNetworkConfig(180-180)ShutdownManager(179-179)delay(178-178)setupExitHandlers(177-177)updateStateFile(176-176)getNetworkConfig(174-174)relayer-cli/src/utils/relay.ts (2)
relayBatch(255-255)relayAllFrom(255-255)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (30)
relayer-cli/.env.dist (3)
3-8: Ensure Consistency in Sender Environment Variables
The new sender variables—SENDER_ADRESSES_DEVNET,SENDER_ADDRESS_TESTNET, andSENDER_ADDRESS_MAINNET—are well-defined and align with the updated configuration objectives. Please verify that all code references and documentation have been updated to use these new variables.
13-13: Update Chain IDs Configuration
ReplacingVEAOUTBOX_CHAIN_IDwithVEAOUTBOX_CHAINSto support multiple chain IDs is correctly implemented. Ensure that any scripts or docs depending on this variable correctly parse the comma-separated values.
16-17: Corrected Subgraph Variable Name
The subgraph endpoint variable is now correctly namedRELAYER_SUBGRAPH, fixing the previous typo. Please double-check that every reference across the project has been updated accordingly.relayer-cli/src/utils/proof.ts (9)
3-13: Good addition of typed interfaces!Adding the
MessageSentDataandMessageSentsDataResponseinterfaces improves type safety and code clarity. This makes the code more maintainable and helps catch potential issues at compile time.
40-40: Good type safety improvement!Adding explicit type casting for the GraphQL response ensures type safety and better code quality.
42-42: Protect against potential undefined dataYou do
result["messageSents"][0]without validating the array length. This might trigger a runtime error if no messages are returned.- return [result[`messageSents`][0].to.id, result[`messageSents`][0].data]; + if (!result.messageSents?.length) { + return undefined; + } + const [ firstMessage ] = result.messageSents; + return [firstMessage.to.id, firstMessage.data];
49-55: Good addition of response interfaces!Adding the
LayerResponseandProofAtCountResponseinterfaces improves type safety and code clarity, making the GraphQL response handling more robust.
67-67: Good parameter addition for filtering by inboxAdding the
inboxAddressparameter enables more precise filtering of nodes, which aligns with the PR's goal of supporting multiple networks.
72-72: Improved early return logicGood addition of an early return when there are no proof indices, which prevents unnecessary processing.
73-83: Better query construction with inbox address filteringThe updated query construction to include inbox address filtering allows for more precise data retrieval and is well documented with the comment.
93-93: Good type annotation for clarityAdding the explicit type annotation for
proofasstring[]improves code readability and type safety.
111-111: Good type annotation for proof arrayExplicitly typing
proofasstring[]improves type safety and clarity.relayer-cli/src/utils/relay.ts (10)
3-3: Well-organized imports for enhanced modularity.The addition of EventEmitter, new utility functions, and custom error classes improves the organization and error handling capabilities of the module.
Also applies to: 6-9
11-13: Good practice: Type definition for GraphQL response.Adding the SnapshotResponse interface improves type safety when working with the GraphQL API response.
20-36: LGTM on the updated getCount function.The function now correctly retrieves the subgraph from an environment variable and uses proper typing with the SnapshotResponse interface.
45-66: Improved error handling and network abstraction in relay function.The modifications to the relay function are well-implemented, including:
- Addition of the network parameter for multi-network support
- Explicit error handling for missing bridgeConfig and privateKey
- Use of Promise.all for parallel execution
- Proper error handling for missing message data
68-80: Good interface extension for multi-network support.The RelayBatchDeps interface has been appropriately extended to include the network parameter and emitter for event handling.
91-152: Enhanced batch relaying with improved error handling.The relayBatch function has been well-refactored to support multiple networks and includes:
- Proper batching of transactions using arrays
- Gas estimation for optimized transactions
- Event emission for better monitoring
- Pre-checking with staticCall to validate transactions before including them in the batch
161-166: Improved relayAllFrom function signature for multiple senders.The function signature has been updated to handle multiple message senders and includes event emission capabilities.
216-222: Good practice: Type definitions for GraphQL responses.Adding the MessageSent and MessageSentsResponse interfaces improves type safety when working with the GraphQL API.
231-252: LGTM on the updated getNonceFrom function.The function now correctly retrieves the subgraph from an environment variable and uses proper typing with the MessageSentsResponse interface.
198-198: Correct use of nonce variable in sendMessage function call.You're now using the correct variable
xfor the nonce in the sendMessage function call, addressing a previous issue that was flagged in a past review.relayer-cli/src/consts/bridgeRoutes.ts (3)
28-31: Good use of enums for improved type safety.Using an enum for network types enhances code readability and ensures type safety when working with network values throughout the application.
75-80: Improved error handling ingetBridgeConfig.The updated function now throws an explicit error for unsupported chain IDs, which is a good improvement for error handling and debugging.
33-53: Well-structured contract organization.Organizing contracts by network type in dedicated constants makes the code more maintainable and aligns well with the multi-network relayer approach described in the PR objectives.
relayer-cli/src/relayer.ts (5)
18-22: Interface structure is clear.
TheRelayerConfiginterface is straightforward and well-defined. Keeping it explicit is good for maintainability and future extensibility.
50-56: Guard usage ofsenders[0].
Ifsendersis empty, referencingsenders[0]will be undefined. Confirm thatgetNetworkConfig()always populates at least one sender. If not, add a guard to handle empty sender arrays.
74-82: Revisit epoch calculation logic.
Line 79 usesMath.floor(currentTS / 1000)in the modulus operation, which might be unintended ifcurrentTSis already in seconds. Verify the math, as reducing the timestamp further by 1,000 could offset the actual epoch timing.
30-40: 🛠️ Refactor suggestionAdd error handling within the main loop.
Wrapping the core of thewhileloop in a try/catch would prevent unhandled errors from terminating the entire relayer unexpectedly. This ensures graceful error logging and potential recovery or retry logic.⛔ Skipped due to learnings
Learnt from: fcanela PR: kleros/vea#344 File: relayer-cli/src/devnetRelayExample.ts:9-14 Timestamp: 2025-03-20T18:46:11.455Z Learning: In `relayer-cli/src/devnetRelayExample.ts`, error handling is managed by the upper layer via `ShutdownManager`'s error signal handler, so adding a `try...catch` block in the main loop is unnecessary.
58-69: 🛠️ Refactor suggestionAdd error handling for relaying operations.
Currently,relayBatchorrelayAllFromerrors are not caught here. Consider wrapping them in a try/catch or promise rejection handler to allow for better logging and recovery mechanisms when relaying fails.⛔ Skipped due to learnings
Learnt from: fcanela PR: kleros/vea#344 File: relayer-cli/src/utils/relay.ts:28-40 Timestamp: 2025-03-20T18:46:01.922Z Learning: In `relayer-cli/src/utils/relay.ts`, avoid suggesting adding try/catch blocks that only log errors and rethrow in the `relay` function.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
relayer-cli/.env.dist (3)
6-6: New DEVNET Sender Addresses Configuration.
The introduction ofSENDER_ADDRESSES_DEVNETto accept multiple comma-separated addresses is a solid enhancement for supporting multi-network relaying. Please verify that all addresses are in the correct hexadecimal checksum format to prevent configuration errors.
7-7: Testnet Sender Address Using Zero Address.
Using the zero address inSENDER_ADDRESSES_TESTNETto allow all senders is an effective approach. Consider adding a brief inline explanation to reinforce why the zero address is used here.
13-13: Multi-Chain Outbox Configuration.
The change fromVEAOUTBOX_CHAIN_IDtoVEAOUTBOX_CHAINSnow supports multiple chain IDs (e.g.,11155111,10200). Ensure that downstream consumers of this variable are designed to parse comma-separated values correctly.relayer-cli/src/utils/relayerHelpers.ts (1)
165-191: Comprehensive network configuration setup.The
getNetworkConfigfunction effectively parses environment variables to create network configurations. It appropriately handles the cases where different network senders are configured and throws a meaningful error when no valid configuration is found.However, consider adding validation for the chain IDs to ensure they are valid numbers.
for (const chainId of chainIds) { + const numericChainId = Number(chainId); + if (isNaN(numericChainId)) { + emitter.emit(BotEvents.WARNING, `Invalid chain ID: ${chainId}, skipping`); + continue; + } if (toRelayDevnet) { relayerNetworkConfig.push({ - chainId: Number(chainId), + chainId: numericChainId, network: Network.DEVNET, senders: devnetSenders, }); } if (toRelayTestnet) { relayerNetworkConfig.push({ - chainId: Number(chainId), + chainId: numericChainId, network: Network.TESTNET, senders: testnetSenders, }); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
relayer-cli/.env.dist(1 hunks)relayer-cli/package.json(1 hunks)relayer-cli/src/utils/relayerHelpers.test.ts(2 hunks)relayer-cli/src/utils/relayerHelpers.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- relayer-cli/package.json
🧰 Additional context used
🧠 Learnings (2)
relayer-cli/src/utils/relayerHelpers.test.ts (1)
Learnt from: mani99brar
PR: kleros/vea#395
File: relayer-cli/src/utils/relayerHelpers.ts:95-97
Timestamp: 2025-03-31T10:11:18.544Z
Learning: The shutdown manager improvements in relayer-cli/src/utils/relayerHelpers.ts, including proper exit code handling, will be addressed in a separate PR to keep the current changes focused.
relayer-cli/src/utils/relayerHelpers.ts (1)
Learnt from: mani99brar
PR: kleros/vea#395
File: relayer-cli/src/utils/relayerHelpers.ts:95-97
Timestamp: 2025-03-31T10:11:18.544Z
Learning: The shutdown manager improvements in relayer-cli/src/utils/relayerHelpers.ts, including proper exit code handling, will be addressed in a separate PR to keep the current changes focused.
🧬 Code Definitions (2)
relayer-cli/src/utils/relayerHelpers.test.ts (1)
relayer-cli/src/utils/relayerHelpers.ts (3)
cleanupLockFile(201-201)ShutdownManager(204-204)setupExitHandlers(202-202)
relayer-cli/src/utils/relayerHelpers.ts (3)
relayer-cli/src/utils/shutdownManager.ts (1)
ShutdownManager(1-15)relayer-cli/src/consts/bridgeRoutes.ts (1)
Network(88-88)relayer-cli/src/utils/errors.ts (1)
NetworkConfigNotSet(1-7)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Scorecard analysis
- GitHub Check: dependency-review
- GitHub Check: Analyze (javascript)
🔇 Additional comments (16)
relayer-cli/.env.dist (4)
3-5: Clear Documentation for Sender Addresses.
The updated comments now clearly explain how sender addresses work, with a zero address allowing relay for all senders and an empty value disabling the network. This clarification improves configuration clarity.
8-8: Empty Mainnet Sender Address.
LeavingSENDER_ADDRESS_MAINNETempty appears intentional to disable relaying on mainnet by default. Please confirm that this behavior aligns with your deployment strategy and update any related documentation if needed.
16-17: Updated Subgraph Endpoint Variable.
The renaming toRELAYER_SUBGRAPHalong with the updated endpoint example enhances clarity and consistency across the codebase. Make sure that all references in the TypeScript/JavaScript modules have been updated to reflect this change.
19-20: New Transaction Batcher Contract Addresses.
DefiningTRANSACTION_BATCHER_CONTRACT_SEPOLIAandTRANSACTION_BATCHER_CONTRACT_CHIADOfor the new batch transaction processing is a necessary update for migrating from the deprecated web3-batcher. It’s advisable to cross-check these addresses with the deployed contracts to avoid misconfiguration issues.relayer-cli/src/utils/relayerHelpers.test.ts (5)
2-2: Updated imports to include new functionality.The import statement has been updated to include the newly added functions (
cleanupLockFile,setupExitHandlers) and theShutdownManagerclass from the relayerHelpers module. This properly reflects the expanded functionality being tested in this file.
69-83: Well-structured tests for cleanupLockFile.The tests for
cleanupLockFileare well-implemented with clear test cases for both when the file exists and when it doesn't. The test correctly mocks the file system operations and verifies the expected behavior.
85-111: Comprehensive setup for exit handlers tests.Good implementation of the setup and teardown for testing exit handlers. The use of
beforeEachto initialize theShutdownManagerand mock theprocess.exitfunction ensures a clean test environment for each test case.I particularly like the explicit cleanup in
afterEachto remove all event listeners, which prevents potential side effects between tests.
113-117: Verification of signal handler registration.This test appropriately verifies that the signal handlers for all three signals (SIGINT, SIGTERM, SIGQUIT) are correctly registered by checking the listener count.
119-138: Complete signal handling test coverage.The tests thoroughly verify the shutdown behavior for all three signals (SIGINT, SIGTERM, SIGQUIT). Each test ensures that:
- The shutdown manager is triggered
- The process exits with the correct code
This provides excellent coverage for this critical functionality.
relayer-cli/src/utils/relayerHelpers.ts (7)
2-2: Added necessary imports for new functionality.The additional imports for
path,NetworkConfigNotSet, andNetworkappropriately support the new functionality in this file.Also applies to: 7-8
28-28: Updated return type to handle edge cases.Changing the return type from
Promise<number>toPromise<number | null>allows for more flexible handling of initialization states.
33-35: Improved cross-platform file path handling.Using
path.joinfor constructing the state file path improves cross-platform compatibility.
88-112: Well-implemented cleanupLockFile function.The
cleanupLockFilefunction is well-implemented with proper error handling and event emission. It correctly usespath.joinfor file path construction and checks for file existence before attempting deletion.
155-159: Clear type definition for network configuration.The
RelayerNetworkConfigtype clearly defines the structure expected for network configurations, which improves code readability and type safety.
197-206: Updated exports to include new functionality.The export statement has been updated to include all the new functions and types, making them available for use in other modules.
127-131: 🛠️ Refactor suggestionPotential race condition in handleExit.
The
handleExitfunction is marked as async but doesn't await thecleanupLockFilecall, which could lead to the process exiting before the cleanup is complete.const handleExit = async (exitCode: number = 0) => { shutdownManager.triggerShutdown(); emitter.emit(BotEvents.EXIT); - await cleanupLockFile(chainId, network, emitter); + try { + await cleanupLockFile(chainId, network, emitter); + } catch (error) { + emitter.emit(BotEvents.EXCEPTION, new Error(`Failed to cleanup during exit: ${error}`)); + } process.exit(0); };Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
relayer-cli/.env.dist (1)
3-8: Enhanced Multi-Sender Addresses ConfigurationThe new variables
SENDER_ADDRESSES_DEVNET,SENDER_ADDRESSES_TESTNET, andSENDER_ADDRESS_MAINNETnow support multiple sender addresses. This change aligns well with the PR’s objective of managing configurable sender addresses per network. Please ensure that the relayer logic properly parses these comma-separated values and that an emptySENDER_ADDRESS_MAINNETis handled as intended.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
relayer-cli/.env.dist(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test
- GitHub Check: Scorecard analysis
- GitHub Check: Analyze (javascript)
- GitHub Check: dependency-review
🔇 Additional comments (4)
relayer-cli/.env.dist (4)
13-14: Multi-Chain Outbox SupportThe update from a singular
VEAOUTBOX_CHAIN_IDtoVEAOUTBOX_CHAINSnow allows the configuration of multiple chain IDs (e.g., "11155111,10200"). Verify that the downstream code correctly parses and validates these comma-separated IDs to enable proper chain-specific routing.
16-17: Subgraph Endpoint Variable UpdateThe subgraph endpoint variable has been correctly renamed to
RELAYER_SUBGRAPHwith an updated example endpoint. This change resolves the previous typo and improves clarity. No further action is needed.
19-22: Transaction Batcher Contract Address UpdatesThe revised variables
TRANSACTION_BATCHER_CONTRACT_SEPOLIAandTRANSACTION_BATCHER_CONTRACT_CHIADOclearly designate the contract addresses for each network. Ensure that these addresses are consistently integrated within the transaction batcher logic.
23-24: Enforced Trailing Slash for STATE_DIRThe update to
STATE_DIRnow enforces a trailing slash, which will help prevent file path concatenation issues. Please confirm that any path operations using this variable correctly utilize the trailing slash.
fcanela
left a comment
fcanela
left a comment
There was a problem hiding this comment.
Great job! Code looks nice. I've added a few notes here and there, but they are just suggestions and mostly opinions on code practices.
I'm not approving it as I'm not familiar enough with Vea to provide such approval, but the refactor looks good to me!
|
|
The veascan build issues are addressed in #418 |
Agnostic relayer-cli:
Merged testnetRelayer and devnetRelayer into relayer which runs for both networks and multiple chains, with relayer config for devnetSenders/testnetSenders env var:- no value passed represents respective network will not run
- first address passed = zeroAddress represents relay for all addresses on the respective network
- address/addresses represents relay for the passed addresses only
Added TransactionBatcher.sol from kleros-v2:
Replaced deprecated web3-batcher with TransactionBatcher.sol
Added validation and support for pm2
Added type and env checks to ensure strict typescript along with more custom errors
Summary by CodeRabbit
New Features
Refactor
Chores
Tests
PR-Codex overview
This PR focuses on refactoring and enhancing the
relayer-clicodebase, including changes to configuration, error handling, and message relaying functionality. It introduces new features and improves existing ones while updating dependencies.Detailed summary
testnetRelayer.tsanddevnetRelayExample.ts.jest.config.tsto includemoduleNameMapper.baseUrlintsconfig.jsonto.and addedoutDir.devnet-relayertovea-relayerinecosystem.config.js.BotEventsfor message relay inbotEvents.ts.logger.tsfor message relay events.errors.ts.TransactionBatchercontract inTransactionBatcher.sol..env.distto reflect new sender address format and removed old entries.proof.tsto include new interfaces for message data.relayer.tsfor clearer structure and added new relaying logic.relayerHelpers.tswith new functions for cleanup and configuration management.relay.tsto support batching and relaying messages with improved error handling.relay.test.tsto cover new functionalities and edge cases.bridgeRoutes.tsto use a new structure for contract addresses and added error handling.ethers.tsto support new contract factories and improved address handling.