fix(evm-sdk): use dynamic transceiver indices instead of hardcoded 0#831
Open
fix(evm-sdk): use dynamic transceiver indices instead of hardcoded 0#831
Conversation
The SDK hardcoded transceiver index 0 in encodeOptions(), quoteDeliveryPrice(), and verifyAddresses(). This breaks when the active transceiver has a non-zero registered index (e.g. after removing the original transceiver and registering a new one, since on-chain indices are monotonically increasing and never reused). Additionally, the manager's quoteDeliveryPrice() has a bug where it sizes the instructions array using enabledTransceivers.length instead of numRegisteredTransceivers, causing ARRAY_RANGE_ERROR(50) when the active transceiver's index >= enabled count. The SDK now bypasses the manager's quoteDeliveryPrice and calls each transceiver directly as a workaround. Changes: - Add registeredIndex property to EvmNttWormholeTranceiver - Add initTransceiverIndices() that fetches real indices via getTransceiverInfo() - Fix encodeOptions() to use cached registered indices for all transceivers - Fix quoteDeliveryPrice() to call transceivers directly (bypasses contract bug) - Fix verifyAddresses() to match transceiver by address instead of array position Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
14 tests covering: - initTransceiverIndices: on-chain index fetching, case-insensitive address matching, graceful fallback for old ABIs, idempotency, multiple transceivers - encodeOptions: uses registered index, sorted output, skipRelay flag encoding - quoteDeliveryPrice: calls transceivers directly (not manager), correct instruction-to-transceiver pairing, price summation - End-to-end: transceiver at index 1 after remove+re-add produces correct encoded bytes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughFetches on-chain transceiver indices, maps them to SDK transceivers by address, caches per-transceiver Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant EvmNtt
participant OnChain
participant Transceiver
Client->>EvmNtt: fromRpc() / use instance
EvmNtt->>EvmNtt: initTransceiverIndices()
EvmNtt->>OnChain: getTransceivers()
OnChain-->>EvmNtt: enabled addresses
loop per address
EvmNtt->>OnChain: getTransceiverInfo(address)
OnChain-->>EvmNtt: info (includes index)
EvmNtt->>Transceiver: set registeredIndex
end
EvmNtt-->>Client: ready
Client->>EvmNtt: encodeOptions()
loop per transceiver
EvmNtt->>Transceiver: read registeredIndex / build instruction
end
EvmNtt->>EvmNtt: sort instructions by index
EvmNtt-->>Client: encoded options
Client->>EvmNtt: quoteDeliveryPrice()
EvmNtt->>EvmNtt: initTransceiverIndices() (idempotent)
loop per transceiver
EvmNtt->>Transceiver: quoteDeliveryPrice(instruction with registeredIndex)
Transceiver-->>EvmNtt: price
end
EvmNtt->>EvmNtt: sum quotes
EvmNtt-->>Client: total price
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
evm/ts/src/ntt.ts (1)
1-1:⚠️ Potential issue | 🟡 MinorFix Prettier formatting issues.
The CI pipeline failed due to code style issues detected by Prettier. Run
bun run prettier --writeto fix formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts/src/ntt.ts` at line 1, Run the Prettier formatter on the repository and fix formatting in evm/ts/src/ntt.ts by executing the suggested command (bun run prettier --write) or your project's Prettier invocation, then stage and commit the updated file; ensure the file now passes CI formatting checks and that any remaining style issues are resolved before pushing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@evm/ts/src/ntt.ts`:
- Line 1: Run the Prettier formatter on the repository and fix formatting in
evm/ts/src/ntt.ts by executing the suggested command (bun run prettier --write)
or your project's Prettier invocation, then stage and commit the updated file;
ensure the file now passes CI formatting checks and that any remaining style
issues are resolved before pushing.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
evm/ts/__tests__/transceiverIndex.test.ts (1)
33-51: Add a constructor regression test for unsupported transceiver keys.Please add a case where
contracts.ntt.transceivercontainswormholeplus an extra unknown type, and assert constructor throws. This will catch the filtering regression around Line 262 inevm/ts/src/ntt.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts/__tests__/transceiverIndex.test.ts` around lines 33 - 51, Add a regression test to ensure the EvmNtt constructor rejects unsupported transceiver keys by creating a test where contracts.ntt.transceiver includes "wormhole" and an extra unknown key and asserting the constructor throws; locate the test suite in transceiverIndex.test.ts and add a new it() case similar to the existing constructor test that constructs EvmNtt (the EvmNtt constructor) with contracts containing transceiver: { wormhole: {...}, unknown: {...} } and expect the construction to throw to catch the filtering bug referenced around the transceiver handling in evm/ts/src/ntt.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@evm/ts/src/ntt.ts`:
- Around line 262-264: The filter callback on
Object.keys(configuredTransceivers) doesn't return a boolean, so change the
callback in the expression that currently uses .filter((transceiverType) => {
transceiverType !== "wormhole"; }) to return the predicate (e.g.,
(transceiverType) => transceiverType !== "wormhole") so non-wormhole keys are
preserved and the subsequent unsupported-transceiver guard (the check around
configuredTransceivers usage) can run; locate the usage of
configuredTransceivers and the filter call in ntt.ts and replace the
block-bodied arrow with a returned expression or an explicit return statement.
---
Nitpick comments:
In `@evm/ts/__tests__/transceiverIndex.test.ts`:
- Around line 33-51: Add a regression test to ensure the EvmNtt constructor
rejects unsupported transceiver keys by creating a test where
contracts.ntt.transceiver includes "wormhole" and an extra unknown key and
asserting the constructor throws; locate the test suite in
transceiverIndex.test.ts and add a new it() case similar to the existing
constructor test that constructs EvmNtt (the EvmNtt constructor) with contracts
containing transceiver: { wormhole: {...}, unknown: {...} } and expect the
construction to throw to catch the filtering bug referenced around the
transceiver handling in evm/ts/src/ntt.ts.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
evm/ts/src/ntt.ts (1)
314-317: Consider logging the caught error for observability.The empty catch block silently swallows all errors. While the retry-on-failure approach is reasonable for transient RPC issues, persistent errors (e.g., ABI mismatch, contract misconfiguration) will silently fall back to index 0, potentially causing the exact bug this PR aims to fix.
Proposed improvement
- } catch { + } catch (e) { + // Log for debugging; do not mark initialized on failure so + // transient RPC errors can be retried on next call. + console.warn("Failed to fetch transceiver indices, will retry:", e); // Do not mark initialized on failure: retry on next call so transient // RPC/provider errors don't permanently pin indices to default 0. }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts/src/ntt.ts` around lines 314 - 317, The empty catch in evm/ts/src/ntt.ts is swallowing errors and should log them for observability; update the catch to capture the thrown error (e.g., catch (err)) and call the project's logger (or console.error) with a clear contextual message that includes identifiers like the variable initialized, the contract/address or index being resolved, and the original error so transient errors still retry but persistent issues are visible for debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@evm/ts/src/ntt.ts`:
- Around line 314-317: The empty catch in evm/ts/src/ntt.ts is swallowing errors
and should log them for observability; update the catch to capture the thrown
error (e.g., catch (err)) and call the project's logger (or console.error) with
a clear contextual message that includes identifiers like the variable
initialized, the contract/address or index being resolved, and the original
error so transient errors still retry but persistent issues are visible for
debugging.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
evm/ts/__tests__/transceiverIndex.test.tsevm/ts/src/ntt.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- evm/ts/tests/transceiverIndex.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
evm/ts/src/ntt.ts (2)
537-548: Defensive fallback should ideally never trigger.The
instructionsarray is created fromencodeOptions()which iterates overthis.xcvrs, so there should always be a matching instruction for each transceiver. The fallback at lines 539-542 is good defensive coding, but if it ever executes, it likely indicates a logic error.Consider adding a debug log or assertion to catch unexpected cases during development.
♻️ Optional: Add visibility when fallback is used
const ix = instructions.find((i) => i.index === xcvr.registeredIndex); + if (!ix) { + console.debug( + `quoteDeliveryPrice: no instruction found for index ${xcvr.registeredIndex}, using empty payload` + ); + } const instruction = ix ?? { index: xcvr.registeredIndex, payload: new Uint8Array(), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts/src/ntt.ts` around lines 537 - 548, The fallback creating a default instruction when no match is found should never occur because instructions is built from encodeOptions() iterating this.xcvrs; update the loop in the method that calls xcvr.transceiver.quoteDeliveryPrice to surface unexpected cases by adding a debug log and/or assertion when ix is undefined (i.e., when using the payload: new Uint8Array() fallback). Specifically, reference this.xcvrs, instructions, xcvr.registeredIndex, encodeOptions, and quoteDeliveryPrice: log the xcvr.registeredIndex and any relevant context or throw/assert to fail fast in development so you can detect and fix the upstream logic error if the fallback is ever used.
314-317: Consider logging or capturing the error for observability.The empty catch block silently swallows errors, making it difficult to diagnose transient RPC failures or unexpected issues during debugging. While the retry behavior is intentional, consider adding minimal logging at debug level.
♻️ Suggested improvement
- } catch { + } catch (e) { // Do not mark initialized on failure: retry on next call so transient // RPC/provider errors don't permanently pin indices to default 0. + // Uncomment for debugging: console.debug("initTransceiverIndices failed, will retry:", e); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts/src/ntt.ts` around lines 314 - 317, The empty catch block in evm/ts/src/ntt.ts is swallowing errors and should record them for observability: update the catch after the initialization logic (the block that currently prevents marking initialized on failure) to log the caught error at debug/trace level while preserving the current behavior of not setting initialized so retries still occur; reference the same symbol(s) used there (e.g., the initialized flag and the surrounding initialization function in ntt.ts) and use the project’s logger if available (falling back to console.debug/console.error) to emit a concise message like "ntt initialization transient error" with the error object. Ensure you do not change the retry logic or rethrow the error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@evm/ts/src/ntt.ts`:
- Around line 537-548: The fallback creating a default instruction when no match
is found should never occur because instructions is built from encodeOptions()
iterating this.xcvrs; update the loop in the method that calls
xcvr.transceiver.quoteDeliveryPrice to surface unexpected cases by adding a
debug log and/or assertion when ix is undefined (i.e., when using the payload:
new Uint8Array() fallback). Specifically, reference this.xcvrs, instructions,
xcvr.registeredIndex, encodeOptions, and quoteDeliveryPrice: log the
xcvr.registeredIndex and any relevant context or throw/assert to fail fast in
development so you can detect and fix the upstream logic error if the fallback
is ever used.
- Around line 314-317: The empty catch block in evm/ts/src/ntt.ts is swallowing
errors and should record them for observability: update the catch after the
initialization logic (the block that currently prevents marking initialized on
failure) to log the caught error at debug/trace level while preserving the
current behavior of not setting initialized so retries still occur; reference
the same symbol(s) used there (e.g., the initialized flag and the surrounding
initialization function in ntt.ts) and use the project’s logger if available
(falling back to console.debug/console.error) to emit a concise message like
"ntt initialization transient error" with the error object. Ensure you do not
change the retry logic or rethrow the error.
Contributor
|
Is this change backwards compatible with existing deployments? |
Contributor
Author
Yes, registeredIndex defaults to 0 ( native-token-transfers/evm/ts/src/ntt.ts Line 56 in 91fc31f Existing deployments with a single transceiver, that was never removed and have index 0 hit the default path and behave identically to before. |
kev1n-peters
approved these changes
Mar 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
2 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.
Summary
Fixes a bug where the EVM TypeScript SDK hardcodes transceiver index
0inencodeOptions(),quoteDeliveryPrice(), andverifyAddresses(). This breaks when the active transceiver has a non-zero on-chain registered index — which happens after removing the original transceiver and registering a new one, sinceTransceiverRegistryindices are monotonically increasing and never reused.Root cause
There are actually two bugs that stack:
SDK (fixed here):
encodeOptions()hardcodesindex: 0instead of querying each transceiver's actual registered index viagetTransceiverInfo().Contract (separate follow-up):
ManagerBase.quoteDeliveryPrice()sizes the instructions array usingenabledTransceivers.lengthinstead ofnumRegisteredTransceivers(which_prepareForTransferalready does correctly). This causes anARRAY_RANGE_ERROR(50)when the active transceiver's registered index >= number of enabled transceivers.This PR fixes both issues from the SDK side:
quoteDeliveryPriceby calling each transceiver'squoteDeliveryPricedirectlyReproduction scenario
removeTransceiver()setTransceiver()(gets index 1)ARRAY_RANGE_ERROR(50)/Panic(0x32)Changes
EvmNttWormholeTranceiver: addregisteredIndexproperty (defaults to 0 for backwards compat)EvmNtt.initTransceiverIndices(): new method that fetches on-chain indices viagetTransceiverInfo()+getTransceivers()and caches them. Falls back gracefully for ABI versions that don't supportgetTransceiverInfoEvmNtt.encodeOptions(): iterates all transceivers using their cachedregisteredIndex, sorts by index (contract requires strictly increasing order)EvmNtt.quoteDeliveryPrice(): calls each transceiver'squoteDeliveryPricedirectly instead of the manager's buggy implementationEvmNtt.verifyAddresses(): matches the wormhole transceiver by address instead of assuming array position[0]Follow-up (contract fix)
ManagerBase.quoteDeliveryPrice()should use_getRegisteredTransceiversStorage().lengthinstead ofenabledTransceivers.lengthto match_prepareForTransfer. One-line fix, but requires a contract upgrade.Test plan
initTransceiverIndices: index fetching, case-insensitive address matching, fallback for old ABIs, idempotency, multiple transceiversencodeOptions: dynamic index usage, sorted output, flag encodingquoteDeliveryPrice: direct transceiver calls, correct instruction-to-transceiver pairing, price summation🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests