Fix/relayer error handling#447
Merged
Merged
Conversation
✅ Deploy Preview for veascan ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for veashi-scan ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
WalkthroughThis PR adds multi-endpoint RPC fallback, structured error and event context, and provider-aware relay and watcher wiring across relayer-cli and validator-cli. It also updates lifecycle cleanup, logging, configuration parsing, documentation, and Docker base images. ChangesCore Relayer Refactor: Multi-RPC, Error Handling, and Events
Validator CLI Multi-RPC and watcher updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler as relayer:start()
participant NetworkProc as processNetworkConfig
participant Relay as relay/relayBatch/relayAllFrom
participant Fallback as FallbackRpcProvider
participant Subgraph as Graph queries / proof helpers
participant Outbox as VeaOutbox
Scheduler->>NetworkProc: config, emitter
NetworkProc->>Relay: execute with network context
Relay->>Fallback: build from rpcOutbox
Relay->>Subgraph: fetch count / proof / message data
Subgraph-->>Relay: data or DataError
Relay->>Outbox: send or execute message
Outbox-->>Relay: success or error
Relay-->>NetworkProc: result or ExecutionError
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
relayer-cli/src/utils/relay.ts (1)
8-8: ⚡ Quick winRemove unused
DataErrorimport.
DataErroris not used in this file and is already reported by static analysis.Proposed fix
-import { MissingEnvironmentVariable, InvalidChainId, DataError, ExecutionError } from "./errors"; +import { MissingEnvironmentVariable, InvalidChainId, ExecutionError } from "./errors";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relayer-cli/src/utils/relay.ts` at line 8, Remove the unused import DataError from the import list in this module: update the import statement that currently reads import { MissingEnvironmentVariable, InvalidChainId, DataError, ExecutionError } from "./errors" to only include the used symbols (MissingEnvironmentVariable, InvalidChainId, ExecutionError) so the file no longer imports DataError.relayer-cli/src/utils/hashi.ts (1)
65-68: ⚡ Quick winMake this missing-config failure actionable and drop the dead return.
After the throw, Line 68 is unreachable, and the current exception text still does not tell the operator which Hashi fields are missing.
Suggested cleanup
if (!yaruAddress || !yahoAddress || !hashiAddress) { emitter.emit(BotEvents.HASHI_NOT_CONFIGURED, targetChainId); - throw new MissingEnvironmentVariable(`Hashi bridge ${sourceChainId} -> ${targetChainId}`); - return 0; + const missing = [ + !yaruAddress && "yaruAddress", + !yahoAddress && "yahoAddress", + !hashiAddress && "hashiAddress", + ] + .filter(Boolean) + .join(", "); + + throw new MissingEnvironmentVariable( + `Missing Hashi bridge config for ${sourceChainId} -> ${targetChainId}: ${missing}` + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relayer-cli/src/utils/hashi.ts` around lines 65 - 68, The current block throws MissingEnvironmentVariable then returns dead code and doesn't say which env vars are missing; remove the unreachable "return 0", compute which of yaruAddress, yahoAddress, hashiAddress are falsy (e.g. build an array missing = ['YARU...'] filtered by their values), include that missing list in the MissingEnvironmentVariable message and in the emitter payload (replace emitter.emit(BotEvents.HASHI_NOT_CONFIGURED, targetChainId) with emitter.emit(BotEvents.HASHI_NOT_CONFIGURED, { targetChainId, missing })) so operators see exactly which fields are absent.relayer-cli/src/utils/hashi.test.ts (1)
156-162: ⚡ Quick winAdd an array-
sourceRPCcase to this expectation.This PR widens the contract to
string | string[], but this suite only locks the single-endpoint path right now. A regression in the new failover wiring would still pass.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relayer-cli/src/utils/hashi.test.ts` around lines 156 - 162, The test only asserts the single-string path for fetchAllMessageLogs but the API now accepts string|string[]; add an additional assertion that validates the array form is handled: call or expect fetchAllMessageLogs with sourceRPC as an array (e.g., ["http://test.rpc"]) and the same other args (sourceChainId, ["http://test.rpc"], "0xYAHO", startBlockNumber, mockEmitter) or add a second expect toHaveBeenCalledWith using that array form so the failover/array wiring is covered; locate and update the test around the existing expect(fetchAllMessageLogs).toHaveBeenCalledWith(...) assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relayer-cli/README.md`:
- Around line 55-61: Update the README section that claims to "Generate contract
types(from root)" to use the actual build command rather than tests: replace the
current steps that change into the contracts folder and run `yarn test` with the
workspace build command `yarn workspace `@kleros/vea-contracts` build` so the
documented flow matches the relayer type-generation step; keep the initial `yarn
install` step.
In `@relayer-cli/src/consts/bridgeRoutes.ts`:
- Around line 67-72: The current rpcInbox and rpcOutbox initialization can
produce arrays like [undefined] when RPC_ARBITRUM_SEPOLIA or RPC_SEPOLIA are
unset; update the logic in bridgeRoutes where rpcInbox and rpcOutbox are built
to validate and normalize the env var: either throw a clear error if the env var
is missing (so the app fails early) or always run (process.env.VAR ?? "")
.split(",").map(s=>s.trim()).filter(Boolean) to produce a non-empty array of
endpoints; ensure the change references rpcInbox and rpcOutbox and results in a
validated array before being passed to FallbackRpcProvider.
In `@relayer-cli/src/relayer.ts`:
- Around line 36-42: The sendHeartbeat(...) failures are currently allowed to
throw and abort the relayer; wrap each await sendHeartbeat call (e.g., the
startup call sendHeartbeat("started", HEARTBEAT_URL!), the in-loop
sendHeartbeat("running", HEARTBEAT_URL!), and the other occurrence) in a
try/catch so heartbeat transport errors are caught, logged (include context and
the error), and not rethrown; keep the relayer loop and start() execution
continuing on heartbeat failures. Use the existing logging mechanism and
reference the sendHeartbeat function and HEARTBEAT_URL constant when making the
change.
- Around line 92-95: The epoch delay calculation uses seconds stored in
currentTS but then divides it by 1000 again; update the computation in the block
using getEpochPeriod(chainId) so the modulus uses currentTS directly (i.e.,
replace Math.floor(currentTS / 1000) % epochPeriod with currentTS % epochPeriod)
and keep timeLeft expressed in milliseconds by multiplying the remaining seconds
by 1000 (still add the 100*1000 jitter), ensuring variables currentTS,
epochPeriod, timeLeft and the return Date.now() + timeLeft are adjusted
accordingly.
In `@relayer-cli/src/utils/fallbackProvider.ts`:
- Around line 26-27: The label() helper currently falls back to returning the
raw endpoint URL (this.endpoints[i].url) which can leak credentials/tokens;
change label() to never emit the full URL — return endpoint.label when present,
otherwise return a sanitized identifier (e.g., hostname[:port] or a masked URL
that strips credentials and query string) by parsing this.endpoints[i].url.
Implement a small sanitizer utility used by label() and update the other places
that call label() (the spots around the other uses of this.endpoints[...] where
raw URLs were being emitted) to use the sanitized result so no credentials or
query tokens appear in logs/events.
- Around line 13-16: The constructor currently dereferences normalized[0].url
which throws for an empty endpoints array; add an explicit guard at the start of
the constructor to validate endpoints/normalized is non-empty (using the
constructor signature and normalized variable) and bail out with a clear,
structured error or emitter notification instead of letting a raw crash occur.
Specifically: check if normalized.length === 0 (or endpoints.length === 0) and
throw a descriptive Error (or emit an error via the provided EventEmitter)
before calling super, so you never access normalized[0].url when there are no
RPC endpoints.
In `@relayer-cli/src/utils/hashi.ts`:
- Around line 300-303: The code dereferences latestFinalized without checking
for null after calling provider.getBlock("finalized"); update the block lookup
in the FallbackRpcProvider usage so you check that latestFinalized is non-null
before accessing latestFinalized.number (e.g., call
provider.getBlock("finalized"), if result is null log an error/ warning and
either retry/fallback to provider.getBlock("latest") or throw a clear error),
ensuring the logic around toBlock uses the guarded value; refer to
provider.getBlock("finalized"), latestFinalized and toBlock when making the
change.
In `@relayer-cli/src/utils/hashiHelpers/bridgeRoutes.ts`:
- Around line 16-21: The current sourceRPC/targetRPC assignments (e.g.,
sourceRPC, targetRPC using process.env.RPC_ARBITRUM_SEPOLIA and
process.env.RPC_SEPOLIA) can yield undefined via the non-null assertion; change
them to always produce a string[] by reading the env var into a string (or empty
string if undefined), split on ',' and trim/filter out empty entries so the
value is always string[]; apply this pattern to all similar configs in
bridgeRoutes.ts (all places referencing RPC_ARBITRUM_SEPOLIA, RPC_SEPOLIA, etc.)
so downstream code no longer receives undefined.
In `@relayer-cli/src/utils/relay.ts`:
- Around line 83-91: The batch-path functions relayBatch and relayAllFrom
currently destructure bridgeConfig and use process.env.PRIVATE_KEY without
validation; update them to mirror the guards used in relay: call
fetchBridgeConfig(chainId) and if it returns falsy throw InvalidChainId (or the
same error type used in relay), validate process.env.PRIVATE_KEY and throw
MissingEnvironmentVariable (or the same env error) when absent, and only then
extract batcherAddress, veaContracts, rpcOutbox and construct
veaInbox/veaOutbox, rpcOutboxUrls, FallbackRpcProvider, batcher (fetchBatcher)
and veaOutbox (fetchVeaOutbox); ensure you reference the same helper functions
(fetchBridgeConfig, fetchBatcher, fetchVeaOutbox) and env var PRIVATE_KEY so the
error surfaces consistently instead of causing downstream failures.
- Line 198: The return currently does `lastNonce + 1` which yields 1 when
lastNonce is null; change the final return in the function that returns
Promise<number | null> to explicitly return null when lastNonce is null (e.g.
`return lastNonce === null ? null : lastNonce + 1`) so the function returns null
to indicate no relay occurred and only returns a number when a valid lastNonce
exists; update any nearby comments to match this behavior (the variable to
change is lastNonce and the function's declared return type Promise<number |
null> referenced by the early exit at line 162).
In `@relayer-cli/src/utils/relayerHelpers.ts`:
- Around line 119-124: The handleExit routine currently always calls
process.exit() and can be re-entered from the Node "exit" listener; change
handleExit(signature) to accept a boolean flag (e.g., shouldExit: boolean =
true) and an internal guarded "isExiting" flag to avoid double runs; when
invoked set isExiting and perform shutdownManager.triggerShutdown(),
emitter.emit(BotEvents.EXIT), await cleanupAllLockFiles(emitter) and only call
process.exit(exitCode) if shouldExit is true. Update the process signal handlers
to call handleExit() normally, but refactor the "exit" event listener to perform
only synchronous cleanup (e.g., shutdownManager.triggerShutdown() and
emitter.emit(BotEvents.EXIT)) without awaiting async work or calling
handleExit/process.exit to prevent re-entrancy.
- Around line 126-129: The addListenerOnce helper currently skips registering if
any listener exists globally (process.listenerCount(event) === 0), which can
silently prevent your handlers from being added; change addListenerOnce to
maintain an internal Set (e.g., registeredEvents) tracking events this module
has registered, and only skip registration when that Set already contains the
event; on first registration call process.on(event, handler) and add the event
to registeredEvents so subsequent calls from this module are no-ops while not
being affected by external listeners.
---
Nitpick comments:
In `@relayer-cli/src/utils/hashi.test.ts`:
- Around line 156-162: The test only asserts the single-string path for
fetchAllMessageLogs but the API now accepts string|string[]; add an additional
assertion that validates the array form is handled: call or expect
fetchAllMessageLogs with sourceRPC as an array (e.g., ["http://test.rpc"]) and
the same other args (sourceChainId, ["http://test.rpc"], "0xYAHO",
startBlockNumber, mockEmitter) or add a second expect toHaveBeenCalledWith using
that array form so the failover/array wiring is covered; locate and update the
test around the existing expect(fetchAllMessageLogs).toHaveBeenCalledWith(...)
assertion.
In `@relayer-cli/src/utils/hashi.ts`:
- Around line 65-68: The current block throws MissingEnvironmentVariable then
returns dead code and doesn't say which env vars are missing; remove the
unreachable "return 0", compute which of yaruAddress, yahoAddress, hashiAddress
are falsy (e.g. build an array missing = ['YARU...'] filtered by their values),
include that missing list in the MissingEnvironmentVariable message and in the
emitter payload (replace emitter.emit(BotEvents.HASHI_NOT_CONFIGURED,
targetChainId) with emitter.emit(BotEvents.HASHI_NOT_CONFIGURED, {
targetChainId, missing })) so operators see exactly which fields are absent.
In `@relayer-cli/src/utils/relay.ts`:
- Line 8: Remove the unused import DataError from the import list in this
module: update the import statement that currently reads import {
MissingEnvironmentVariable, InvalidChainId, DataError, ExecutionError } from
"./errors" to only include the used symbols (MissingEnvironmentVariable,
InvalidChainId, ExecutionError) so the file no longer imports DataError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e1c1aff6-cc88-4a91-81ff-90e270bf0c0b
📒 Files selected for processing (17)
relayer-cli/Dockerfilerelayer-cli/README.mdrelayer-cli/src/consts/bridgeRoutes.tsrelayer-cli/src/relayer.tsrelayer-cli/src/utils/botEvents.tsrelayer-cli/src/utils/errors.tsrelayer-cli/src/utils/ethers.tsrelayer-cli/src/utils/fallbackProvider.tsrelayer-cli/src/utils/graphQueries.tsrelayer-cli/src/utils/hashi.test.tsrelayer-cli/src/utils/hashi.tsrelayer-cli/src/utils/hashiHelpers/bridgeRoutes.tsrelayer-cli/src/utils/logger.tsrelayer-cli/src/utils/proof.tsrelayer-cli/src/utils/relay.tsrelayer-cli/src/utils/relayerHelpers.test.tsrelayer-cli/src/utils/relayerHelpers.ts
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
relayer-cli/src/utils/relay.ts (1)
83-87:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit
bridgeConfigvalidation before destructuring.Line 83 and Line 153 can return an undefined config, and destructuring at Line 86 / Line 154 then throws a generic runtime error instead of surfacing a clear chain-config failure. Please mirror the explicit chain guard used elsewhere.
Proposed fix
const bridgeConfig = fetchBridgeConfig(chainId); + if (!bridgeConfig) throw new InvalidChainId(chainId); const privateKey = process.env.PRIVATE_KEY; if (!privateKey) throw new MissingEnvironmentVariable("PRIVATE_KEY"); const { batcherAddress, veaContracts, rpcOutbox } = bridgeConfig;const bridgeConfig = getBridgeConfig(chainId); + if (!bridgeConfig) throw new InvalidChainId(chainId); const { veaContracts, batcherAddress, rpcOutbox } = bridgeConfig;Also applies to: 153-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relayer-cli/src/utils/relay.ts` around lines 83 - 87, The code destructures bridgeConfig immediately after calling fetchBridgeConfig which can return undefined; add an explicit guard after calling fetchBridgeConfig (in both places where bridgeConfig is used) that checks if bridgeConfig is falsy and throws a clear error (e.g., throw new Error(`Missing bridge config for chainId ${chainId}`)) before destructuring batcherAddress, veaContracts, rpcOutbox and before accessing veaContracts[network].veaInbox.address so the failure surfaces as a clear chain-config error instead of a generic runtime exception.relayer-cli/src/relayer.ts (1)
36-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t let heartbeat transport failures stop the relayer loop.
Line 36, Line 42, and Line 54 await heartbeat calls directly; a transient heartbeat failure will throw and abort relay execution. Wrap each call in
try/catchand log/emit exception context without rethrowing.Also applies to: 42-42, 54-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relayer-cli/src/relayer.ts` at line 36, The relayer currently awaits sendHeartbeat(…, HEARTBEAT_URL!) directly (e.g., the three sendHeartbeat calls in relayer.ts), so a transient network error will throw and abort the relayer loop; wrap each await sendHeartbeat(...) in its own try/catch, catch the error and log or emit the exception context (including the status string and HEARTBEAT_URL) using the existing logger/emit mechanism, and do not rethrow so the relayer continues running; ensure you update the three call sites that reference sendHeartbeat and HEARTBEAT_URL to follow this pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@relayer-cli/src/relayer.ts`:
- Line 36: The relayer currently awaits sendHeartbeat(…, HEARTBEAT_URL!)
directly (e.g., the three sendHeartbeat calls in relayer.ts), so a transient
network error will throw and abort the relayer loop; wrap each await
sendHeartbeat(...) in its own try/catch, catch the error and log or emit the
exception context (including the status string and HEARTBEAT_URL) using the
existing logger/emit mechanism, and do not rethrow so the relayer continues
running; ensure you update the three call sites that reference sendHeartbeat and
HEARTBEAT_URL to follow this pattern.
In `@relayer-cli/src/utils/relay.ts`:
- Around line 83-87: The code destructures bridgeConfig immediately after
calling fetchBridgeConfig which can return undefined; add an explicit guard
after calling fetchBridgeConfig (in both places where bridgeConfig is used) that
checks if bridgeConfig is falsy and throws a clear error (e.g., throw new
Error(`Missing bridge config for chainId ${chainId}`)) before destructuring
batcherAddress, veaContracts, rpcOutbox and before accessing
veaContracts[network].veaInbox.address so the failure surfaces as a clear
chain-config error instead of a generic runtime exception.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c35f882-2870-45d1-8dac-e6ba526fb9d4
📒 Files selected for processing (5)
relayer-cli/README.mdrelayer-cli/src/consts/bridgeRoutes.tsrelayer-cli/src/relayer.tsrelayer-cli/src/utils/relay.tsrelayer-cli/src/utils/relayerHelpers.ts
✅ Files skipped from review due to trivial changes (1)
- relayer-cli/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- relayer-cli/src/consts/bridgeRoutes.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
validator-cli/Dockerfile (1)
6-6: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin both Node base images by digest.
validator-cli/Dockerfile:6,56still use the mutable tagnode:24.14-alpine3.23; pin bothFROMlines to the same digest to keep builds reproducible and reduce supply-chain risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@validator-cli/Dockerfile` at line 6, Both Node base images are still using the mutable node:24.14-alpine3.23 tag, so update the two FROM statements in validator-cli/Dockerfile to reference the same pinned digest instead. Keep the validator-build stage and the final runtime stage aligned on that exact digest to make builds reproducible and avoid tag drift.Source: Linters/SAST tools
validator-cli/src/utils/fallbackProvider.ts (1)
11-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMark never-reassigned members
readonly.Static analysis flags
endpoints,emitter, andstaticChainIdas never reassigned.- private endpoints: RPCEndpoint[]; + private readonly endpoints: RPCEndpoint[]; private activeIndex = 0; - private emitter: EventEmitter; - private staticChainId?: number; + private readonly emitter: EventEmitter; + private readonly staticChainId?: number;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@validator-cli/src/utils/fallbackProvider.ts` around lines 11 - 14, The fields in FallbackProvider that are never reassigned should be marked readonly to satisfy the static analysis warning. Update the class member declarations for endpoints, emitter, and staticChainId in FallbackProvider so they are immutable after initialization, while leaving activeIndex mutable because it is intentionally changed later.Source: Linters/SAST tools
validator-cli/src/utils/fallbackProviderV5.ts (1)
12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMark never-reassigned members
readonly.Static analysis flags
endpointsandemitteras never reassigned.- private endpoints: RPCEndpoint[]; + private readonly endpoints: RPCEndpoint[]; private activeIndex = 0; - private emitter: EventEmitter; + private readonly emitter: EventEmitter;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@validator-cli/src/utils/fallbackProviderV5.ts` around lines 12 - 14, Mark the never-reassigned members in FallbackProviderV5 as readonly so they match their actual usage. Update the class fields endpoints and emitter in fallbackProviderV5 and keep activeIndex mutable since it changes, making sure the constructor still initializes the readonly members correctly.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@validator-cli/src/consts/bridgeRoutes.ts`:
- Around line 30-34: splitRpcUrls currently turns missing or empty values into
an empty array, which is fine for optional routerRPC but not for required
inboxRPC/outboxRPC. Update splitRpcUrls or its callers in bridgeRoutes to
validate required env vars early and throw a contextual error when inboxRPC or
outboxRPC is absent/blank, while preserving empty arrays only for routerRPC. Use
the existing splitRpcUrls helper and the inboxRPC/outboxRPC/routerRPC config
wiring to place the check where the values are parsed.
In `@validator-cli/src/utils/fallbackProvider.ts`:
- Around line 32-34: The fallback provider label method is exposing sensitive
RPC URLs when no explicit label is set, which can leak secrets through RPC
failure/recovery logs. Update FallbackProvider.label to avoid returning the raw
endpoint url and instead use a safe non-sensitive identifier for each endpoint,
matching the safer behavior expected for FallbackRpcProvider and its
RPC_FAILURE/RPC_RECOVERED logging paths.
In `@validator-cli/src/utils/fallbackProviderV5.ts`:
- Around line 26-28: The fallback in label() on FallbackProviderV5 currently
returns the raw endpoint url, which can expose embedded API keys in RPC URLs and
then propagate into RPC_FAILURE/RPC_RECOVERED logs. Update label(i) to return a
non-sensitive identifier instead of url when label is missing, and keep
logger.ts payloads using that sanitized label so rpc_failure and rpc_recovered
never emit secrets. Use the label() method in fallbackProviderV5 and the log
paths in logger.ts to verify the unsafe fallback is removed.
- Around line 1-24: Add `@ethersproject/providers` to the explicit dependencies
for `validator-cli`, since `FallbackProviderV5` imports `JsonRpcProvider`
directly and should not rely on hoisting or transitive resolution. Update the
package manifest for `validator-cli` so the module used by `FallbackProviderV5`
is declared, keeping the import in `fallbackProviderV5` unchanged.
In `@validator-cli/src/utils/transactionHandlers/arbToGnosisHandler.ts`:
- Around line 31-34: `approveWeth()` is creating a new `FallbackRpcProvider`
instead of using the handler’s persistent outbox provider, which resets fallback
state on every call. Update `arbToGnosisHandler` so `approveWeth()` reuses the
`BaseTransactionHandler` outbox provider already available on the handler
instance, and keep the existing `getWallet`/`getWETH` flow wired to that shared
provider rather than constructing a fresh one.
---
Nitpick comments:
In `@validator-cli/Dockerfile`:
- Line 6: Both Node base images are still using the mutable
node:24.14-alpine3.23 tag, so update the two FROM statements in
validator-cli/Dockerfile to reference the same pinned digest instead. Keep the
validator-build stage and the final runtime stage aligned on that exact digest
to make builds reproducible and avoid tag drift.
In `@validator-cli/src/utils/fallbackProvider.ts`:
- Around line 11-14: The fields in FallbackProvider that are never reassigned
should be marked readonly to satisfy the static analysis warning. Update the
class member declarations for endpoints, emitter, and staticChainId in
FallbackProvider so they are immutable after initialization, while leaving
activeIndex mutable because it is intentionally changed later.
In `@validator-cli/src/utils/fallbackProviderV5.ts`:
- Around line 12-14: Mark the never-reassigned members in FallbackProviderV5 as
readonly so they match their actual usage. Update the class fields endpoints and
emitter in fallbackProviderV5 and keep activeIndex mutable since it changes,
making sure the constructor still initializes the readonly members correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6b9807f-b11c-435e-9890-49562af586b9
📒 Files selected for processing (12)
validator-cli/.env.distvalidator-cli/Dockerfilevalidator-cli/README.mdvalidator-cli/src/consts/bridgeRoutes.tsvalidator-cli/src/utils/botEvents.tsvalidator-cli/src/utils/ethers.tsvalidator-cli/src/utils/fallbackProvider.tsvalidator-cli/src/utils/fallbackProviderV5.tsvalidator-cli/src/utils/logger.tsvalidator-cli/src/utils/transactionHandlers/arbToGnosisHandler.tsvalidator-cli/src/watcher.tsvalidator-cli/tsconfig.json
✅ Files skipped from review due to trivial changes (2)
- validator-cli/README.md
- validator-cli/.env.dist
|
jaybuidl
approved these changes
Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR-Codex overview
This PR focuses on enhancing the
validator-cliandrelayer-cliprojects by introducing RPC fallback mechanisms, updating dependencies, and refining error handling. It also improves the structure of the code for better maintainability and adds testing instructions in the documentation.Detailed summary
targetintsconfig.jsontoes2021.RPC_FAILUREandRPC_RECOVEREDevents inbotEvents.ts.@ethersproject/providersdependency.DataErrorandExecutionErrorclasses.FallbackProviderfor RPC calls infallbackProvider.ts.getVeaInboxandgetVeaOutboxto useJsonRpcProvider.README.md.Summary by CodeRabbit