Conversation
…ss dedup SpokePoolClient.spokePoolAddress is an Address object, not a string. The noImplicitAny cleanup in #3157 replaced working code that called .toNative() before passing to toAddressType() with a version that passed the Address object directly, causing `address.startsWith is not a function` at runtime. Use the Address objects directly (skipping redundant toAddressType) and deduplicate by string representation via Map since Set only deduplicates objects by reference. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mrice32
requested review from
bmzig,
dohaki,
nicholaspai and
pxrl
as code owners
Simplify the Map-based dedup to use dedupArray on native strings, then convert back via toAddressType. Clearer intent, same result. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex re-review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1d6a56212
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/monitor/MonitorClientHelper.ts
Outdated
| Object.values(spokePoolClients).map(({ chainId, spokePoolAddress: address }) => toAddressType(address, chainId)) | ||
| ); | ||
| Object.values(spokePoolClients).map(({ spokePoolAddress }) => spokePoolAddress.toNative()) | ||
| ).map((address) => toAddressType(address, hubPoolClient.chainId)); |
There was a problem hiding this comment.
After deduplicating with native strings, this maps every address back with hubPoolClient.chainId, which is the L1 EVM chain. constructSpokePoolClientsWithLookback can include non-EVM spoke pools (see getSpokePoolClientsForContract creating SVMSpokePoolClient for non-EVM chains), so a Solana/base58 spokePoolAddress will be reconstructed as if it were an EVM address and can fail parsing or produce the wrong Address type at runtime. The previous logic preserved each spoke pool’s own chain ID, so this introduces a regression specifically when SVM chains are enabled.
Useful? React with 👍 / 👎.
The previous commit used hubPoolClient.chainId (L1 EVM) when reconstructing all addresses via toAddressType, which would misclassify SVM spoke pool addresses as EvmAddress. Instead, use the existing Address objects directly — they are already correctly typed as EvmAddress/SvmAddress — and deduplicate by native string representation via Map. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex re-review. |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Same logic, fewer lines. The comment now explains why a Map is used (value-based dedup preserving Address subtypes). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex re-review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Summary
TypeError: address.startsWith is not a functioncrash in the monitor bot introduced by improve: Fix noImplicitAny type crimes (1/n) #3157SpokePoolClient.spokePoolAddressis anAddressobject, not a string. The noImplicitAny cleanup replaced working code that called.toNative()before passing totoAddressType()with a version that passes theAddressobject directly, which crashes whentoAddressTypecalls.startsWith("0x")on it.dedupArray, then converts back toAddressviatoAddressType— matching the original intent from refactor: Simplify Monitor balance reporting and delete TokenTransferClient #3111 more readably.Why TypeScript didn't catch this
The relayer's
tsconfig.jsonhas bothskipLibCheck: trueandnoImplicitAny: false.skipLibCheckprevents full type-checking of SDK.d.tsfiles, and when TypeScript can't resolve a type it silently infersanyrather than erroring. SospokePoolAddresswas typed asany, makingtoAddressType(any, number)valid — even though the actual runtime value is anAddressobject, not astring. EnablingnoImplicitAnywould have caught this at compile time.Test plan
TypeError: address.startsWith is not a function🤖 Generated with Claude Code