feat(cli): add manual set-transceiver-peer and redeem#824
feat(cli): add manual set-transceiver-peer and redeem#824
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a manual NTT transfer mode (CLI flag + manual route), an option to skip destination rate-limit checks in route quoting, new CLI peer command, multiple XRPL NTT SDK packages and implementations, and several dependency/workspace updates and test/config files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as TokenTransfer CLI
participant RouteSel as Route Selector
participant Manual as NTT Manual Route
participant Executor as NTT Executor Route
participant SDK as NTT SDK
User->>CLI: run token-transfer (--manual?)
CLI->>RouteSel: select route (manual ?)
alt manual = true
RouteSel->>Manual: instantiate nttManualRoute
Manual->>SDK: validate/quote(params { skipDstRateLimitCheck: true })
SDK-->>Manual: return quote (no dst rate-limit checks)
Manual-->>CLI: return ManualTransferReceipt
else manual = false
RouteSel->>Executor: instantiate nttExecutorRoute
Executor->>SDK: validate/quote(params { skipDstRateLimitCheck: false })
SDK-->>Executor: return quote (includes dst rate-limit checks)
Executor-->>CLI: return ExecutorTransferReceipt
end
CLI-->>User: display result / instructions
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cli/src/index.ts`:
- Around line 2933-2950: The code treats argv["vaa"] as hex always and fails
when a file path is provided; modify the logic around vaaHex/vaaBytes so that
before calling encoding.hex.decode you check if the value is a path to an
existing readable file (use fs.existsSync or fs.stat) and if so read the file
contents (fs.readFileSync) into vaaHex; otherwise keep the original string,
strip a leading 0x/0X as already done, then call encoding.hex.decode(vaaHex)
inside the try/catch (referencing argv["vaa"], vaaHex, encoding.hex.decode, and
vaaBytes) so file inputs are supported and errors still trigger the existing
error handling.
- Around line 2812-2816: The CLI currently always calls toUniversal(peerChain,
peerAddress) when building peerChainAddress, but peerAddress may already be a
32-byte universal hex string per docs; update the logic in the peer address
construction (peerChainAddress, toUniversal, peerAddress, peerChain) to detect a
32-byte hex universal input (e.g., optional 0x prefix + 64 hex characters) and
if detected use/normalize that value directly as the universal address,
otherwise call toUniversal(peerChain, peerAddress) to convert from chain-native
format; ensure normalization (lowercase, ensure 0x prefix or whatever internal
format is expected) and add a small validation/error if the supplied universal
hex is the wrong length.
In `@cli/src/tokenTransfer.ts`:
- Around line 146-150: The final success message still unconditionally says the
executor will relay even when the CLI option "manual" (defined with
.option("manual")) is set; update the final log block that prints the transfer
result (the console.log/processLogger.info call after the transfer completes) to
branch on the "manual" flag (argv.manual or options.manual) and print a
different message when manual is true (e.g., "Transfer initiated via Manual
route — not auto‑relayed; relay must be executed separately") and keep the
existing executor-relay message when manual is false. Ensure you reference the
existing "manual" option and the same logging call so only messaging changes.
evan-gray
force-pushed
the
xrpl-testing
branch
from
a9325c6 to
8bb9e01
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cli/src/tokenTransfer.ts`:
- Around line 370-372: The code currently skips ensureChainSupported when
argv.manual is true, but later unconditionally calls
wh.getChain(destinationChainInput) and routes.RouteTransferRequest.create which
can throw unclear errors if the destination chain lacks RPC config; update the
logic so that when argv.manual is true you either (a) wrap the
routes.RouteTransferRequest.create (and the preceding wh.getChain call) in a
try/catch that catches configuration errors and rethrows a clear
manual-mode-specific message, or (b) reinstate a minimal validation/comment that
documents why skipping ensureChainSupported is safe; reference
ensureChainSupported, argv.manual, wh.getChain(destinationChainInput), and
routes.RouteTransferRequest.create when applying the fix.
🧹 Nitpick comments (2)
cli/src/tokenTransfer.ts (2)
516-518: Type assertion andanyerode type safety across the transfer flow.The intersection type
NttRoute.ValidatedParams & NttExecutorRoute.ValidatedParamsis incorrect — the params will be one or the other, never both. AndquoteResult: anydisables all downstream type checking on the quote.Consider using a union type and narrowing based on
argv.manual:♻️ Suggested approach
- const validatedParams = validation.params as NttRoute.ValidatedParams & - NttExecutorRoute.ValidatedParams; - let quoteResult: any; + const validatedParams = validation.params as + | NttRoute.ValidatedParams + | NttExecutorRoute.ValidatedParams; + let quoteResult: routes.QuoteResult<NttRoute.Options, typeof validatedParams>;
698-707: Messaging updated for manual mode, but could be more actionable.The manual-mode message correctly tells the user redemption is needed, but doesn't indicate how. Consider appending a hint about the
redeemcommand (per the PR description, a manualredeemCLI command was added).💡 Suggested tweak
argv.manual - ? "Transfer initiated. This will need to be manually redeemed on the destination chain." + ? "Transfer initiated. This will need to be manually redeemed on the destination chain.\nUse the `redeem` command to complete the transfer." : "Transfer submitted. The Wormhole Executor will relay the transfer automatically once finalized."
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
cli/src/commands/token-transfer.ts (1)
370-372:⚠️ Potential issue | 🟠 MajorManual mode still hard-fails on destination chain config.
Skipping
ensureChainSupportedhere is incomplete because Line 385 still unconditionally callswh.getChain(destinationChainInput). In manual mode, that can still fail before transfer initiation, which defeats the intent of supporting chains without full destination support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/commands/token-transfer.ts` around lines 370 - 372, The manual mode bypass currently skips ensureChainSupported but still calls wh.getChain(destinationChainInput) which will hard-fail; update the code so that wh.getChain(destinationChainInput) is only invoked when not in manual mode (argv.manual === false) or make wh.getChain tolerant by returning undefined/null instead of throwing and handle that case later during actual transfer initiation; specifically modify the call sites referencing wh.getChain and the earlier ensureChainSupported usage so manual flow does not call getChain (or checks for undefined) and only enforces/get the destination chain when required for non-manual transfers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/commands/manual.ts`:
- Around line 271-275: Validate the value passed from the --transceiver-index
option before calling ntt.setTransceiverPeer: ensure the parsed value (e.g.,
transceiverIndex) is a Number, not NaN, is an integer (use Number.isInteger) and
>= 0; if the check fails, emit a clear CLI error message and exit/fail fast
(using yargs.fail or process.exit(1)) so negative, fractional or non-numeric
inputs are rejected before invoking ntt.setTransceiverPeer.
- Around line 434-436: The CLI example for "manual redeem" advertises a file
path but the implementation always treats the argument as hex (e.g., the code
around the redeem handler that reads and hex-decodes the raw VAA argument such
as the variable handling the VAA input in the manual redeem command). Update the
redeem command handler to first check whether the provided VAA argument is a
readable file (use fs.existsSync or fs.promises access and
readFile/readFileSync), read and trim the file contents if it exists, then
proceed to decode the resulting string as hex (or accept a leading 0x) into the
VAA bytes; ensure the same logic is applied where the code currently decodes the
raw arg so file paths like "./vaa.txt" work.
In `@cli/src/commands/token-transfer.ts`:
- Around line 493-497: The executor-only msgValue warning block is still
executed when manual routing is requested; wrap the existing executor-specific
branch that emits “Wormhole Executor” guidance (the block that runs when using
executor routing) with a guard so it only runs when argv.manual is false.
Specifically, ensure the logic that selects Route (nttManualRoute vs
nttExecutorRoute) and the subsequent executor-only warning block checks
"!argv.manual" before printing guidance (i.e., only run the executor msgValue
warnings when not argv.manual), while leaving nttManualRoute, nttExecutorRoute,
Route and routeInstance creation unchanged.
---
Duplicate comments:
In `@cli/src/commands/token-transfer.ts`:
- Around line 370-372: The manual mode bypass currently skips
ensureChainSupported but still calls wh.getChain(destinationChainInput) which
will hard-fail; update the code so that wh.getChain(destinationChainInput) is
only invoked when not in manual mode (argv.manual === false) or make wh.getChain
tolerant by returning undefined/null instead of throwing and handle that case
later during actual transfer initiation; specifically modify the call sites
referencing wh.getChain and the earlier ensureChainSupported usage so manual
flow does not call getChain (or checks for undefined) and only enforces/get the
destination chain when required for non-manual transfers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b3f103ec-bc05-4502-a3c5-5e249caeb438
📒 Files selected for processing (4)
cli/src/commands/manual.tscli/src/commands/token-transfer.tssdk/route/src/manual.tssdk/route/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/route/src/manual.ts
- sdk/route/src/types.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
cli/src/commands/token-transfer.ts (1)
493-497:⚠️ Potential issue | 🟡 MinorGate executor msgValue guidance when
--manualis set.Manual routing is selected here, but executor-specific msgValue warnings are still emitted earlier. Please guard the executor msgValue block with
!argv.manualto avoid misleading operator instructions.Suggested fix
- if (destinationPlatform === "Solana") { + if (!argv.manual && destinationPlatform === "Solana") { const msgValueToUse = destinationMsgValueOverride ?? DEFAULT_SOLANA_MSG_VALUE; applyMsgValueOverride(executorConfig, destinationTokenId, msgValueToUse); if (destinationMsgValueOverride === undefined) { console.warn( colors.yellow( `Destination ${destinationChainInput} requires msgValue funding for the Wormhole Executor. Using default ${msgValueToUse.toString()} lamports. Pass --destination-msg-value to override.` ) ); } else { console.log( `Using custom msgValue ${destinationMsgValueOverride.toString()} for ${destinationChainInput}.` ); } - } else if (destinationMsgValueOverride !== undefined) { + } else if (!argv.manual && destinationMsgValueOverride !== undefined) { console.warn( colors.yellow( `--destination-msg-value is only required for SVM destinations. Ignoring override for ${destinationChainInput}.` ) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/commands/token-transfer.ts` around lines 493 - 497, The executor-specific msgValue warning block is still shown even when manual routing is selected; wrap the executor msgValue warning logic with a check for !argv.manual so it only runs for executor routes. Locate the existing msgValue warning logic that references executor config/message guidance and add a guard using argv.manual (i.e., only run when !argv.manual), and ensure routing instantiation still uses Route = argv.manual ? nttManualRoute(executorConfig.ntt) : nttExecutorRoute(executorConfig) and new Route(wh) unchanged.cli/src/commands/manual.ts (1)
271-275:⚠️ Potential issue | 🟡 MinorValidate
--transceiver-indexbefore callingsetTransceiverPeer.
type: "number"does not preventNaN, fractional, or negative values. Add a fail-fast guard before invoking the SDK call.Suggested fix
const transceiverIndex = argv["transceiver-index"]; + if ( + typeof transceiverIndex !== "number" || + Number.isNaN(transceiverIndex) || + !Number.isInteger(transceiverIndex) || + transceiverIndex < 0 + ) { + console.error( + colors.red( + "--transceiver-index must be a non-negative integer" + ) + ); + process.exit(1); + } ... const setTxs = ntt.setTransceiverPeer( transceiverIndex, peerChainAddress, signer.address.address );Also applies to: 365-367
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/commands/manual.ts` around lines 271 - 275, Add explicit validation for the --transceiver-index option before calling setTransceiverPeer: ensure the parsed value is an integer >= 0 (reject NaN, fractional, negative) and fail fast with a clear error message. Locate where the CLI option "transceiver-index" is read and validated in manual.ts (the handler that calls setTransceiverPeer and the similar usage around the second occurrence), coerce or parse the value to an integer, check Number.isInteger(value) && value >= 0, and return/throw a user-facing error if invalid so setTransceiverPeer is only invoked with a valid non-negative integer index.
🧹 Nitpick comments (1)
cli/src/commands/manual.ts (1)
466-467: Inline VAA input comment is now misleading.The comment says file-or-argument input, but this path currently treats
vaaas hex input only. Consider updating the comment to match behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/commands/manual.ts` around lines 466 - 467, Comment mismatch: the code treating argv["vaa"] as hex only makes the "file or argument" comment incorrect; update the comment near the vaaHex variable (vaaHex = argv["vaa"]) to reflect that this path currently expects inline hex VAA input only (or adjust logic to support file input). Specifically, either change the comment to say "Read VAA bytes from inline hex argument" referencing the vaaHex variable, or implement file-reading logic when argv["vaa"] is a filepath before parsing hex in the block that sets vaaHex.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cli/src/commands/manual.ts`:
- Around line 271-275: Add explicit validation for the --transceiver-index
option before calling setTransceiverPeer: ensure the parsed value is an integer
>= 0 (reject NaN, fractional, negative) and fail fast with a clear error
message. Locate where the CLI option "transceiver-index" is read and validated
in manual.ts (the handler that calls setTransceiverPeer and the similar usage
around the second occurrence), coerce or parse the value to an integer, check
Number.isInteger(value) && value >= 0, and return/throw a user-facing error if
invalid so setTransceiverPeer is only invoked with a valid non-negative integer
index.
In `@cli/src/commands/token-transfer.ts`:
- Around line 493-497: The executor-specific msgValue warning block is still
shown even when manual routing is selected; wrap the executor msgValue warning
logic with a check for !argv.manual so it only runs for executor routes. Locate
the existing msgValue warning logic that references executor config/message
guidance and add a guard using argv.manual (i.e., only run when !argv.manual),
and ensure routing instantiation still uses Route = argv.manual ?
nttManualRoute(executorConfig.ntt) : nttExecutorRoute(executorConfig) and new
Route(wh) unchanged.
---
Nitpick comments:
In `@cli/src/commands/manual.ts`:
- Around line 466-467: Comment mismatch: the code treating argv["vaa"] as hex
only makes the "file or argument" comment incorrect; update the comment near the
vaaHex variable (vaaHex = argv["vaa"]) to reflect that this path currently
expects inline hex VAA input only (or adjust logic to support file input).
Specifically, either change the comment to say "Read VAA bytes from inline hex
argument" referencing the vaaHex variable, or implement file-reading logic when
argv["vaa"] is a filepath before parsing hex in the block that sets vaaHex.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c6b7935-a0d7-4e48-84be-6fd4fd5698bb
📒 Files selected for processing (4)
cli/src/commands/manual.tscli/src/commands/token-transfer.tssdk/route/src/manual.tssdk/route/src/types.ts
evan-gray
force-pushed
the
xrpl-testing
branch
2 times, most recently
from
b4dd7c7 to
affddd5
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bunfig.toml`:
- Around line 23-25: The dependency list contains a duplicate entry
"@wormhole-foundation/sdk-base"; remove the repeated
"@wormhole-foundation/sdk-base" (the later occurrence) so the array only
includes unique packages (keep "@wormhole-foundation/sdk-btc" and a single
"@wormhole-foundation/sdk-base").
In `@xrpl/ts/src/ntt.ts`:
- Around line 176-199: The code currently prefers
destination.address.toUint8Array() which can yield non-32-byte native forms;
instead always normalize to the 32-byte universal form: if
destination.address.toUniversalAddress exists, call it and use its
toUint8Array(); only fall back to toUint8Array when toUniversalAddress is not
available, and in all cases validate that destinationAddressBytes.length === 32
(throw a clear error if not). Update the block around destination.address /
destinationAddressBytes (and ensure consistency with recipient_address in
layouts.ts) to prefer toUniversalAddress, then validate length === 32 before
continuing.
- Around line 220-236: The transfer() function is an async generator (async *)
but currently returns the constructed SubmittableTransaction named payment,
which sets the generator's completion value instead of emitting it; change the
function to yield payment (use yield payment) so the UnsignedTransaction<N, C>
is emitted to consumers iterating with for await...of, ensuring the manual
transfer path actually receives the transaction from transfer().
- Around line 89-94: Add a Biome rule suppression comment to each async
generator stub that throws instead of yielding (e.g., async *setThreshold, and
the other async * methods at the ranges you flagged) so the linter won't flag
correctness/useYield; specifically insert the comment // biome-ignore
correctness/useYield: intentional stub immediately above each method declaration
(for example above async *setThreshold(...)) to document the intentional stub
and suppress the rule.
In `@xrpl/ts/src/nttWithExecutor.ts`:
- Around line 221-237: The async generator transfer (async *transfer) yields the
first payment but currently uses return for executorPayment which ends the
generator and prevents consumers from receiving the second leg; change the final
return of executorPayment to yield executorPayment so both transactions are
emitted (ensure the symbol executorPayment in nttWithExecutor.ts is yielded from
async *transfer rather than returned).
- Around line 156-185: The code that uses provider.request("account_tx") to grab
the latest transaction may pick up a different transaction than the submitted
NTT payment, so update the logic in nttWithExecutor.ts to reliably identify the
NTT payment before computing ledgerIndex/txIndex: either return and use the
submitted transaction hash from the caller (match account_tx results by hash) or
validate the retrieved lastTx against the expected nttPayment properties
(compare tx hash or ensure lastTx.tx.TransactionType === "Payment",
lastTx.tx.Destination === expectedNttManager, and any expected Memo values)
before extracting ledger_index and TransactionIndex to compute messageId; reject
and retry/fetch by hash if no match. Ensure you reference nttPayment, sender,
provider.request("account_tx"), lastTx, ledgerIndex, txIndex and messageId when
implementing the match/validation.
- Around line 101-124: The destination address conversion
(destinationAddressBytes) can produce non-32-byte native encodings; update the
logic in nttWithExecutor.ts to normalize destination.address the same way as
recipientManagerAddress by calling destination.address.toUniversalAddress(),
checking it is non-null, then using its toUint8Array() result (ensuring exactly
32 bytes), rather than preferring native toUint8Array(); keep the existing
try/catch but replace the multi-branch detection with a single
toUniversalAddress() path and validate the resulting byte length.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5df00dd-57f7-43b5-af3c-a21ffb117edd
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
bunfig.tomlcli/package.jsonevm/ts/package.jsonpackage.jsonsdk/definitions/package.jsonsdk/definitions/src/ntt.tssdk/examples/package.jsonsdk/route/package.jsonsdk/route/src/types.tssolana/package.jsonsui/ts/package.jsonxrpl/.gitignorexrpl/ts/jest.config.tsxrpl/ts/package.jsonxrpl/ts/src/__tests__/layouts.test.tsxrpl/ts/src/index.tsxrpl/ts/src/layouts.tsxrpl/ts/src/ntt.tsxrpl/ts/src/nttWithExecutor.tsxrpl/ts/tsconfig.cjs.jsonxrpl/ts/tsconfig.esm.json
✅ Files skipped from review due to trivial changes (2)
- xrpl/.gitignore
- xrpl/ts/tsconfig.cjs.json
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/route/src/types.ts
| "@wormhole-foundation/sdk-base", | ||
| "@wormhole-foundation/sdk-btc", | ||
| "@wormhole-foundation/sdk-base", |
There was a problem hiding this comment.
Remove duplicate @wormhole-foundation/sdk-base entry.
Line 25 duplicates the entry already present on line 23. This appears to be an accidental copy when inserting sdk-btc.
🔧 Proposed fix
"@wormhole-foundation/sdk-base",
"@wormhole-foundation/sdk-btc",
- "@wormhole-foundation/sdk-base",
"@wormhole-foundation/sdk-connect",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@wormhole-foundation/sdk-base", | |
| "@wormhole-foundation/sdk-btc", | |
| "@wormhole-foundation/sdk-base", | |
| "@wormhole-foundation/sdk-base", | |
| "@wormhole-foundation/sdk-btc", | |
| "@wormhole-foundation/sdk-connect", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bunfig.toml` around lines 23 - 25, The dependency list contains a duplicate
entry "@wormhole-foundation/sdk-base"; remove the repeated
"@wormhole-foundation/sdk-base" (the later occurrence) so the array only
includes unique packages (keep "@wormhole-foundation/sdk-btc" and a single
"@wormhole-foundation/sdk-base").
| async *setThreshold( | ||
| threshold: number, | ||
| payer?: AccountAddress<C> | ||
| ): AsyncGenerator<UnsignedTransaction<N, C>> { | ||
| throw new Error("unsupported"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "(ntt\.ts|biome|lint)" | head -20Repository: wormhole-foundation/native-token-transfers
Length of output: 261
🏁 Script executed:
find . -type f -name "*.ts" -path "*/ts/src/*" | head -10Repository: wormhole-foundation/native-token-transfers
Length of output: 621
🏁 Script executed:
find . -maxdepth 2 -type f -name "biome*" -o -name ".biomerc*" -o -name "*.config.*"Repository: wormhole-foundation/native-token-transfers
Length of output: 139
🏁 Script executed:
wc -l xrpl/ts/src/ntt.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 108
🏁 Script executed:
cat -n xrpl/ts/src/ntt.ts | sed -n '85,160p'Repository: wormhole-foundation/native-token-transfers
Length of output: 2673
🏁 Script executed:
cat -n xrpl/ts/src/ntt.ts | sed -n '235,290p'Repository: wormhole-foundation/native-token-transfers
Length of output: 1713
🏁 Script executed:
cat -n xrpl/ts/src/ntt.ts | sed -n '315,330p'Repository: wormhole-foundation/native-token-transfers
Length of output: 585
🏁 Script executed:
cat biome.json | head -100Repository: wormhole-foundation/native-token-transfers
Length of output: 1860
🏁 Script executed:
grep -n "biome-ignore\|eslint-disable\|@ts-ignore\|TODO\|FIXME" xrpl/ts/src/ntt.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 619
🏁 Script executed:
ls -la xrpl/ts/Repository: wormhole-foundation/native-token-transfers
Length of output: 510
🏁 Script executed:
cat -n xrpl/package.json | head -50Repository: wormhole-foundation/native-token-transfers
Length of output: 135
🏁 Script executed:
grep -A 3 "async \*" evm/ts/src/ntt.ts | head -40Repository: wormhole-foundation/native-token-transfers
Length of output: 1540
🏁 Script executed:
find xrpl -name "package.json" -type fRepository: wormhole-foundation/native-token-transfers
Length of output: 106
🏁 Script executed:
find . -maxdepth 2 -name "*.yml" -o -name "*.yaml" | head -10Repository: wormhole-foundation/native-token-transfers
Length of output: 171
🏁 Script executed:
cat -n xrpl/ts/package.jsonRepository: wormhole-foundation/native-token-transfers
Length of output: 2238
🏁 Script executed:
cat sdk/ci.yaml | head -100Repository: wormhole-foundation/native-token-transfers
Length of output: 3021
🏁 Script executed:
head -80 xrpl/ts/src/ntt.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 2150
🏁 Script executed:
grep -n "class.*Ntt\|interface.*Ntt\|extends\|implements" xrpl/ts/src/ntt.ts | head -20Repository: wormhole-foundation/native-token-transfers
Length of output: 656
🏁 Script executed:
grep -A 10 "class NttManager" evm/ts/src/ntt.ts | head -15Repository: wormhole-foundation/native-token-transfers
Length of output: 68
🏁 Script executed:
find . -type f -name "*.ts" -path "*/definitions*" | grep -i nttRepository: wormhole-foundation/native-token-transfers
Length of output: 245
🏁 Script executed:
grep -r "interface Ntt\|type Ntt" --include="*.ts" | grep -v node_modules | head -10Repository: wormhole-foundation/native-token-transfers
Length of output: 902
🏁 Script executed:
cat -n sdk/definitions/src/ntt.ts | head -150Repository: wormhole-foundation/native-token-transfers
Length of output: 5549
🏁 Script executed:
grep -n "interface\|abstract class" sdk/definitions/src/ntt.ts | head -20Repository: wormhole-foundation/native-token-transfers
Length of output: 567
🏁 Script executed:
cat -n sdk/definitions/src/ntt.ts | tail -200Repository: wormhole-foundation/native-token-transfers
Length of output: 7826
🏁 Script executed:
grep -n "async \*" sdk/definitions/src/ntt.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 68
🏁 Script executed:
web_search
Biome useYield rule correctness linting async generatorRepository: wormhole-foundation/native-token-transfers
Length of output: 180
🏁 Script executed:
grep -n "async \*" sdk/definitions/src/ntt.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 68
🌐 Web query:
Biome useYield rule recommended correctness
💡 Result:
In Biome, useYield is the linter rule lint/correctness/useYield.
- What it checks: it reports a diagnostic when you declare a generator function (
function*) that never usesyield(i.e., it’s a generator in syntax only). It’s equivalent in intent to ESLint’srequire-yield. [1] - Why it matters (correctness): a
function*returns an iterator/generator object, not a normal return value—so declaring it as a generator withoutyieldis usually a mistake or misleading API design. [1] - Recommended? Yes—Biome marks it as recommended, so it’s enabled by default when you use Biome’s recommended rules. [1]
- Default severity: error. [1]
Configure (explicitly enable / set severity):
{
"linter": {
"rules": {
"correctness": {
"useYield": "error"
}
}
}
}[1]
If you want to turn it off for a project, set "useYield": "off" under correctness (same location as above). [1]
Add Biome rule suppression for unsupported async generator stubs.
The async generator methods in this file (lines 89–94, 108–129, 132–153, 238–243, 265–270, 282–288, 317–323) trigger Biome's useYield error because they never yield—they only throw. Since XRPL intentionally does not support these operations yet, suppress the rule with // biome-ignore correctness/useYield: intentional stub on each method instead of leaving the linter in an error state.
🧰 Tools
🪛 Biome (2.4.4)
[error] 89-94: This generator function doesn't contain yield.
(lint/correctness/useYield)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/ts/src/ntt.ts` around lines 89 - 94, Add a Biome rule suppression
comment to each async generator stub that throws instead of yielding (e.g.,
async *setThreshold, and the other async * methods at the ranges you flagged) so
the linter won't flag correctness/useYield; specifically insert the comment //
biome-ignore correctness/useYield: intentional stub immediately above each
method declaration (for example above async *setThreshold(...)) to document the
intentional stub and suppress the rule.
| // Convert destination address to bytes | ||
| // TODO: do this address handling stuff properly, copied from Sui | ||
| let destinationAddressBytes: Uint8Array; | ||
| try { | ||
| if (typeof destination.address.toUint8Array === "function") { | ||
| destinationAddressBytes = destination.address.toUint8Array(); | ||
| } else if (typeof destination.address.toUniversalAddress === "function") { | ||
| const universalAddr = destination.address.toUniversalAddress(); | ||
| if (!universalAddr) { | ||
| throw new Error("toUniversalAddress() returned null or undefined"); | ||
| } | ||
| destinationAddressBytes = universalAddr.toUint8Array(); | ||
| } else { | ||
| throw new Error( | ||
| `destination.address does not have expected methods. Type: ${typeof destination.address}` | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| throw new Error( | ||
| `Failed to convert destination address to bytes: ${ | ||
| error instanceof Error ? error.message : String(error) | ||
| }` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the layouts.ts file to confirm recipient_address is fixed at 32 bytes
fd -e "ts" -e "js" "layouts" | head -20Repository: wormhole-foundation/native-token-transfers
Length of output: 146
🏁 Script executed:
# Let's find the layouts file
find . -name "layouts.ts" -o -name "layouts.js" | head -10Repository: wormhole-foundation/native-token-transfers
Length of output: 110
🏁 Script executed:
# Search for recipient_address definition
rg "recipient_address" -A 2 -B 2Repository: wormhole-foundation/native-token-transfers
Length of output: 13406
🏁 Script executed:
# Look for toUniversal function
rg "toUniversal" -B 2 -A 5Repository: wormhole-foundation/native-token-transfers
Length of output: 34302
🏁 Script executed:
# Check the ntt.ts file context around the issue
cat -n xrpl/ts/src/ntt.ts | sed -n '160,210p'Repository: wormhole-foundation/native-token-transfers
Length of output: 2296
Always normalize the destination address to a 32-byte universal format before serialization.
recipient_address is fixed at 32 bytes in xrpl/ts/src/layouts.ts, but this code prefers the native toUint8Array() form whenever it exists. This is inconsistent with EVM and Solana implementations which normalize via toUniversalAddress(). Any destination with a non-32-byte native address representation will be misencoded or rejected during serialization.
Suggested fix
- let destinationAddressBytes: Uint8Array;
- try {
- if (typeof destination.address.toUint8Array === "function") {
- destinationAddressBytes = destination.address.toUint8Array();
- } else if (typeof destination.address.toUniversalAddress === "function") {
- const universalAddr = destination.address.toUniversalAddress();
- if (!universalAddr) {
- throw new Error("toUniversalAddress() returned null or undefined");
- }
- destinationAddressBytes = universalAddr.toUint8Array();
- } else {
- throw new Error(
- `destination.address does not have expected methods. Type: ${typeof destination.address}`
- );
- }
- } catch (error) {
+ let destinationAddressBytes: Uint8Array;
+ try {
+ destinationAddressBytes = toUniversal(
+ destination.chain,
+ destination.address.toString()
+ ).toUint8Array();
+ } catch (error) {
throw new Error(
`Failed to convert destination address to bytes: ${
error instanceof Error ? error.message : String(error)
}`
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/ts/src/ntt.ts` around lines 176 - 199, The code currently prefers
destination.address.toUint8Array() which can yield non-32-byte native forms;
instead always normalize to the 32-byte universal form: if
destination.address.toUniversalAddress exists, call it and use its
toUint8Array(); only fall back to toUint8Array when toUniversalAddress is not
available, and in all cases validate that destinationAddressBytes.length === 32
(throw a clear error if not). Update the block around destination.address /
destinationAddressBytes (and ensure consistency with recipient_address in
layouts.ts) to prefer toUniversalAddress, then validate length === 32 before
continuing.
| const payment: SubmittableTransaction = { | ||
| TransactionType: "Payment", | ||
| Account: sender.toString(), | ||
| Destination, | ||
| Amount: amount.toString(), // XRP in drops | ||
| Memos: [ | ||
| { | ||
| Memo: { | ||
| MemoFormat: encoding.hex.encode("application/x-ntt-transfer"), | ||
| MemoData: memoData, | ||
| }, | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| return payment; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the broader context of the transfer function around lines 220-236
head -n 250 xrpl/ts/src/ntt.ts | tail -n 60Repository: wormhole-foundation/native-token-transfers
Length of output: 1670
🏁 Script executed:
# Also check the function signature to confirm it's async *
sed -n '190,240p' xrpl/ts/src/ntt.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 1524
🌐 Web query:
JavaScript async generator return vs yield - does return value appear in for await...of loop
💡 Result:
No. In an async generator, return someValue becomes the final iterator result { value: someValue, done: true } (the completion record), but for await...of stops when it sees done: true and does not run the loop body for that final result, so the returned value is not yielded to the loop. [1], [2], [3]
How to get the return value (if you need it): iterate manually and read the final {value, done} yourself:
const it = myAsyncGen();
while (true) {
const r = await it.next();
if (r.done) {
console.log("return value:", r.value);
break;
}
console.log("yielded:", r.value);
}Or, if you want the “final value” to be seen by for await...of, yield it instead of return (and then return/fall through with undefined). [3]
Related: calling asyncGen.return(x) also finishes the generator with { value: x, done: true }, which likewise won’t show up as a loop iteration in for await...of. [2]
Sources:
[1] MDN — for await...of exits when next() yields { done: true }
[2] MDN — AsyncGenerator.prototype.return() returns { value, done: true }
[3] Stack Overflow — for-of / for-await-of ignore the final done: true result value
🏁 Script executed:
# Find the transfer method signature - search from around line 150-220
sed -n '150,225p' xrpl/ts/src/ntt.ts | head -n 100Repository: wormhole-foundation/native-token-transfers
Length of output: 2721
🏁 Script executed:
# Also search more broadly for the transfer method definition
grep -n "transfer\s*(" xrpl/ts/src/ntt.ts | head -n 5Repository: wormhole-foundation/native-token-transfers
Length of output: 108
🏁 Script executed:
# Get the full method signature with return type
sed -n '190,225p' xrpl/ts/src/ntt.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 1239
Yield the XRPL payment instead of returning it.
The transfer() method is declared as async * with return type AsyncGenerator<UnsignedTransaction<N, C>>, but return payment only sets the generator's completion value (the final { value: payment, done: true } result). When consumers iterate with for await...of, they stop upon done: true and never see the returned value, so no transactions are emitted to this method and the manual transfer path breaks.
Suggested fix
- return payment;
+ yield payment as unknown as UnsignedTransaction<N, C>;🧰 Tools
🪛 Biome (2.4.4)
[error] 156-236: This generator function doesn't contain yield.
(lint/correctness/useYield)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/ts/src/ntt.ts` around lines 220 - 236, The transfer() function is an
async generator (async *) but currently returns the constructed
SubmittableTransaction named payment, which sets the generator's completion
value instead of emitting it; change the function to yield payment (use yield
payment) so the UnsignedTransaction<N, C> is emitted to consumers iterating with
for await...of, ensuring the manual transfer path actually receives the
transaction from transfer().
| // Convert destination address to bytes | ||
| // TODO: do this address handling stuff properly, copied from Sui | ||
| let destinationAddressBytes: Uint8Array; | ||
| try { | ||
| if (typeof destination.address.toUint8Array === "function") { | ||
| destinationAddressBytes = destination.address.toUint8Array(); | ||
| } else if (typeof destination.address.toUniversalAddress === "function") { | ||
| const universalAddr = destination.address.toUniversalAddress(); | ||
| if (!universalAddr) { | ||
| throw new Error("toUniversalAddress() returned null or undefined"); | ||
| } | ||
| destinationAddressBytes = universalAddr.toUint8Array(); | ||
| } else { | ||
| throw new Error( | ||
| `destination.address does not have expected methods. Type: ${typeof destination.address}` | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| throw new Error( | ||
| `Failed to convert destination address to bytes: ${ | ||
| error instanceof Error ? error.message : String(error) | ||
| }` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /repo && find . -type f -name "nttWithExecutor.ts" | head -5Repository: wormhole-foundation/native-token-transfers
Length of output: 141
🏁 Script executed:
cat -n xrpl/ts/src/nttWithExecutor.ts | head -150Repository: wormhole-foundation/native-token-transfers
Length of output: 5602
🏁 Script executed:
# Search for similar address conversion logic in non-executor paths
rg "toUint8Array|toUniversalAddress" xrpl/ts/src/ -A 5 -B 2Repository: wormhole-foundation/native-token-transfers
Length of output: 3435
🏁 Script executed:
# Look for any documentation or tests related to recipient_address field requirements
rg "recipient_address|32.*byte" xrpl/ts/src/ | head -20Repository: wormhole-foundation/native-token-transfers
Length of output: 408
🏁 Script executed:
rg "serializeLayout" xrpl/ts/src/ntt.ts -B 5 -A 5 | head -40Repository: wormhole-foundation/native-token-transfers
Length of output: 749
🏁 Script executed:
cat -n xrpl/ts/src/ntt.ts | sed -n '88,130p'Repository: wormhole-foundation/native-token-transfers
Length of output: 1517
🏁 Script executed:
# Search for related review comments or issues about address encoding
rg "32.*byte|recipient.*address|toUniversalAddress" xrpl/ts/ --type ts -C 3 | grep -E "(TODO|FIXME|BUG|issue)" -i -A 2 -B 2Repository: wormhole-foundation/native-token-transfers
Length of output: 68
🏁 Script executed:
# Check the layouts to understand field validation
cat -n xrpl/ts/src/layouts.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 1678
🏁 Script executed:
# Look at how recipientManagerAddress is handled for comparison
rg "toUniversal" xrpl/ts/src/ -B 2 -A 2Repository: wormhole-foundation/native-token-transfers
Length of output: 2667
🏁 Script executed:
# Search for ChainAddress type definition and address type constraints
rg "ChainAddress|toUint8Array|toUniversalAddress" xrpl/ts/src/__tests__/ -C 3 | head -50Repository: wormhole-foundation/native-token-transfers
Length of output: 68
🏁 Script executed:
# Check if there are any address type implementations that might have variable byte widths
fd "\.ts$" xrpl/ts/src/ -exec grep -l "address.*toUint8Array\|toUniversalAddress" {} \;Repository: wormhole-foundation/native-token-transfers
Length of output: 321
🏁 Script executed:
# Look for test cases that show expected address encoding behavior
cat -n xrpl/ts/src/__tests__/layouts.test.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 1019
🏁 Script executed:
# Verify by looking at the non-executor path in ntt.ts to confirm same pattern
cat -n xrpl/ts/src/ntt.ts | sed -n '55,90p'Repository: wormhole-foundation/native-token-transfers
Length of output: 1156
🏁 Script executed:
# Check if there's a way to normalize to universal form like recipientManagerAddress does
rg "toUniversal.*destination\|toUniversalAddress" xrpl/ts/src/ -B 3 -A 3Repository: wormhole-foundation/native-token-transfers
Length of output: 68
🏁 Script executed:
# Look for any other XRPL address encoding patterns
rg "recipient_address|destinationAddressBytes" xrpl/ts/src/ --type ts -B 2 -A 2Repository: wormhole-foundation/native-token-transfers
Length of output: 3656
🏁 Script executed:
# Check what happens when serializeLayout receives mismatched size
cat -n xrpl/ts/src/ntt.ts | sed -n '140,170p'Repository: wormhole-foundation/native-token-transfers
Length of output: 1244
🏁 Script executed:
# Search for how toUniversal is used vs direct toUint8Array to understand the intent
rg "toUniversal\(" xrpl/ts/src/ -B 2 -A 2 | head -60Repository: wormhole-foundation/native-token-transfers
Length of output: 831
🏁 Script executed:
# Check if there are any address type definitions or documentation about byte widths
fd "\.ts$" -type f | xargs grep -l "toUint8Array\|toUniversalAddress" | head -5Repository: wormhole-foundation/native-token-transfers
Length of output: 256
🏁 Script executed:
# Check the transfer method signature and how destination is passed
cat -n xrpl/ts/src/nttWithExecutor.ts | sed -n '76,105p'Repository: wormhole-foundation/native-token-transfers
Length of output: 1290
🏁 Script executed:
# Look for how destination parameter is used and if there's a universal conversion method
rg "destination.address|toUniversal.*address" xrpl/ts/src/ -B 1 -A 1Repository: wormhole-foundation/native-token-transfers
Length of output: 2316
🏁 Script executed:
# Check if the SDK has documentation about address byte widths
rg "class ChainAddress|interface ChainAddress" --type ts -A 10Repository: wormhole-foundation/native-token-transfers
Length of output: 68
🏁 Script executed:
# Let me check if there's any indication of what address types might return non-32-byte values
rg "toUint8Array|toUniversalAddress" --type ts -B 5 -A 5 | grep -E "(size|byte|width|length)" -i -B 2 -A 2 | head -40Repository: wormhole-foundation/native-token-transfers
Length of output: 2399
🏁 Script executed:
# Check the test to see what address format is expected
cat -n xrpl/ts/src/__tests__/layouts.test.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 1019
🏁 Script executed:
# Verify the exact pattern: recipientManagerAddress uses toUniversal but destination does not
cat -n xrpl/ts/src/nttWithExecutor.ts | sed -n '96,130p'Repository: wormhole-foundation/native-token-transfers
Length of output: 1676
🏁 Script executed:
# Verify the critical difference: recipientManagerAddress uses toUniversal but destination does not
cat -n xrpl/ts/src/ntt.ts | sed -n '180,210p'Repository: wormhole-foundation/native-token-transfers
Length of output: 1469
🏁 Script executed:
# Check if toUniversal is meant to normalize addresses to 32 bytes
rg "toUniversal" . -B 3 -A 3 | head -50Repository: wormhole-foundation/native-token-transfers
Length of output: 2754
🏁 Script executed:
# Final verification: Check if serializeLayout validates field sizes
rg "serializeLayout|binary.*bytes.*size" . --type ts -B 2 -A 2 | head -40Repository: wormhole-foundation/native-token-transfers
Length of output: 2392
🏁 Script executed:
# Confirm the Sui reference mentioned in the TODO comment
cat -n ./sui/ts/src/nttWithExecutor.ts | sed -n '100,125p'Repository: wormhole-foundation/native-token-transfers
Length of output: 909
Normalize destination.address to universal 32-byte form, consistent with how recipientManagerAddress is handled.
The recipient_address field requires exactly 32 bytes. While recipientManagerAddress uses toUniversal() to normalize, the destination address conversion prefers native toUint8Array() bytes when available. Addresses with native widths other than 32 bytes will be misencoded. Use the same pattern: destination.address.toUniversalAddress().toUint8Array() to ensure consistent 32-byte encoding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/ts/src/nttWithExecutor.ts` around lines 101 - 124, The destination
address conversion (destinationAddressBytes) can produce non-32-byte native
encodings; update the logic in nttWithExecutor.ts to normalize
destination.address the same way as recipientManagerAddress by calling
destination.address.toUniversalAddress(), checking it is non-null, then using
its toUint8Array() result (ensuring exactly 32 bytes), rather than preferring
native toUint8Array(); keep the existing try/catch but replace the multi-branch
detection with a single toUniversalAddress() path and validate the resulting
byte length.
| // After the NTT payment is submitted and confirmed, look up the result | ||
| // to compute the messageId = (ledgerIndex << 32) | txIndex | ||
| const senderAddr = sender.toString(); | ||
| const accountTx = await this.provider.request({ | ||
| command: "account_tx", | ||
| account: senderAddr, | ||
| limit: 1, | ||
| forward: false, | ||
| }); | ||
|
|
||
| const lastTx = accountTx.result.transactions[0]; | ||
| if (!lastTx || !lastTx.meta || typeof lastTx.meta === "string") { | ||
| throw new Error("Could not retrieve NTT transfer transaction result"); | ||
| } | ||
|
|
||
| const ledgerIndex = lastTx.tx_blob | ||
| ? lastTx.validated | ||
| ? (lastTx as any).ledger_index | ||
| : undefined | ||
| : (lastTx.tx as any)?.ledger_index; | ||
| const txIndex = (lastTx.meta as any).TransactionIndex; | ||
|
|
||
| if (ledgerIndex === undefined || txIndex === undefined) { | ||
| throw new Error( | ||
| "Could not determine ledgerIndex or txIndex from NTT transfer result" | ||
| ); | ||
| } | ||
|
|
||
| // messageId = (ledgerIndex << 32) | txIndex | ||
| const messageId = (BigInt(ledgerIndex) << 32n) | BigInt(txIndex); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the file structure and size
wc -l xrpl/ts/src/nttWithExecutor.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 120
🏁 Script executed:
# Get the broader context around lines 156-185
sed -n '140,200p' xrpl/ts/src/nttWithExecutor.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 1912
🏁 Script executed:
# Find the function that contains these lines
sed -n '1,160p' xrpl/ts/src/nttWithExecutor.ts | tail -40Repository: wormhole-foundation/native-token-transfers
Length of output: 1394
🏁 Script executed:
# Search for where the NTT payment is submitted
rg -A 5 -B 5 "submitTx|account_tx" xrpl/ts/src/nttWithExecutor.ts | head -100Repository: wormhole-foundation/native-token-transfers
Length of output: 423
🏁 Script executed:
# Get the full function context - start from line 1 to see function declaration
sed -n '100,245p' xrpl/ts/src/nttWithExecutor.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 4798
🏁 Script executed:
# Let's also look at the beginning of the file to understand the class structure
head -100 xrpl/ts/src/nttWithExecutor.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 2680
🏁 Script executed:
# Check if there's any transaction type validation or memo matching
sed -n '156,185p' xrpl/ts/src/nttWithExecutor.ts | grep -i "Payment\|memo\|destination\|type"Repository: wormhole-foundation/native-token-transfers
Length of output: 231
🏁 Script executed:
# Search for any hash-based lookup mechanisms elsewhere in the codebase
rg -i "txhash|tx_hash|tx_id" xrpl/ts/src/ | head -20Repository: wormhole-foundation/native-token-transfers
Length of output: 68
🏁 Script executed:
# Check if there's any comment or documentation about this pattern
rg -B 10 -A 2 "account_tx" xrpl/ts/src/nttWithExecutor.ts | head -30Repository: wormhole-foundation/native-token-transfers
Length of output: 482
🏁 Script executed:
# Let's verify if there's any validation of transaction type, destination, or memos in the lookup
sed -n '165,190p' xrpl/ts/src/nttWithExecutor.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 943
🏁 Script executed:
# Check the xrpl library types to understand what fields are available on transaction response
rg "interface.*Transaction|type.*Transaction" xrpl/ts/src/ | head -20Repository: wormhole-foundation/native-token-transfers
Length of output: 68
🏁 Script executed:
# Search for any similar patterns elsewhere in the codebase that might handle this differently
rg -B 5 "account_tx" xrpl/ts/src/ -A 5Repository: wormhole-foundation/native-token-transfers
Length of output: 764
Validate the NTT payment transaction before extracting ledgerIndex and txIndex.
The code yields the nttPayment transaction for the caller to submit, then uses account_tx with limit: 1, forward: false to retrieve what it assumes is that payment. However, there is no transaction hash tracking or validation. If the account has other concurrent activity or if the caller prefetches transactions, the returned latest transaction may not be the NTT payment, causing messageId to be computed from the wrong transaction and breaking the executor request.
To fix this, either:
- Have the caller return the submitted transaction hash so it can be matched, or
- Validate the retrieved transaction matches the
nttPaymentcharacteristics (e.g., destination = NTT manager, contains expected memo, transaction type = Payment).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/ts/src/nttWithExecutor.ts` around lines 156 - 185, The code that uses
provider.request("account_tx") to grab the latest transaction may pick up a
different transaction than the submitted NTT payment, so update the logic in
nttWithExecutor.ts to reliably identify the NTT payment before computing
ledgerIndex/txIndex: either return and use the submitted transaction hash from
the caller (match account_tx results by hash) or validate the retrieved lastTx
against the expected nttPayment properties (compare tx hash or ensure
lastTx.tx.TransactionType === "Payment", lastTx.tx.Destination ===
expectedNttManager, and any expected Memo values) before extracting ledger_index
and TransactionIndex to compute messageId; reject and retry/fetch by hash if no
match. Ensure you reference nttPayment, sender, provider.request("account_tx"),
lastTx, ledgerIndex, txIndex and messageId when implementing the
match/validation.
| // Transaction 2: Executor request payment | ||
| const executorPayment: SubmittableTransaction = { | ||
| TransactionType: "Payment", | ||
| Account: senderAddr, | ||
| Destination: payeeAddress, | ||
| Amount: quote.estimatedCost.toString(), // relay fee in drops | ||
| Memos: [ | ||
| { | ||
| Memo: { | ||
| MemoFormat: encoding.hex.encode("application/x-executor-request"), | ||
| MemoData: executorMemoData, | ||
| }, | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| return executorPayment; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "nttWithExecutor.ts" -type fRepository: wormhole-foundation/native-token-transfers
Length of output: 258
🏁 Script executed:
wc -l xrpl/ts/src/nttWithExecutor.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 120
🏁 Script executed:
cat -n xrpl/ts/src/nttWithExecutor.tsRepository: wormhole-foundation/native-token-transfers
Length of output: 9107
Yield the executor payment; return drops the second leg.
The function is an async generator (async *transfer), and only the first payment is yielded on line 154. The second payment is returned on line 237, which terminates the generator without yielding. Async-iterator consumers will never see the second transaction.
Suggested fix
- return executorPayment;
+ yield executorPayment as unknown as UnsignedTransaction<N, C>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Transaction 2: Executor request payment | |
| const executorPayment: SubmittableTransaction = { | |
| TransactionType: "Payment", | |
| Account: senderAddr, | |
| Destination: payeeAddress, | |
| Amount: quote.estimatedCost.toString(), // relay fee in drops | |
| Memos: [ | |
| { | |
| Memo: { | |
| MemoFormat: encoding.hex.encode("application/x-executor-request"), | |
| MemoData: executorMemoData, | |
| }, | |
| }, | |
| ], | |
| }; | |
| return executorPayment; | |
| // Transaction 2: Executor request payment | |
| const executorPayment: SubmittableTransaction = { | |
| TransactionType: "Payment", | |
| Account: senderAddr, | |
| Destination: payeeAddress, | |
| Amount: quote.estimatedCost.toString(), // relay fee in drops | |
| Memos: [ | |
| { | |
| Memo: { | |
| MemoFormat: encoding.hex.encode("application/x-executor-request"), | |
| MemoData: executorMemoData, | |
| }, | |
| }, | |
| ], | |
| }; | |
| yield executorPayment as unknown as UnsignedTransaction<N, C>; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xrpl/ts/src/nttWithExecutor.ts` around lines 221 - 237, The async generator
transfer (async *transfer) yields the first payment but currently uses return
for executorPayment which ends the generator and prevents consumers from
receiving the second leg; change the final return of executorPayment to yield
executorPayment so both transactions are emitted (ensure the symbol
executorPayment in nttWithExecutor.ts is yielded from async *transfer rather
than returned).
1 participant
These are the changes I used to configure and test a deployment for XRP to Solana.
hack in the XRPL chain into the sdk package in the node_modules, setting it as Sui so I could pass the 32 byte addresses (not included, modified the filesupdated to the SDK with XRPL supportnode_modules/@wormhole-foundation/sdk-base/dist/esm/constants/chains.jsandnode_modules/@wormhole-foundation/sdk-base/dist/esm/constants/platforms.js)--manualflag to token-transfer to support chains that don't yet have full SDK or Executor support.The XRP payment used was
The emitter and manager address are derived as
The example transfer command I tested
Summary by CodeRabbit
New Features
Tests
Chores