sdk: add build transactions functions to route#822
Conversation
📝 WalkthroughWalkthroughIntroduces sender-based transaction builders and public helpers that produce arrays of unsigned transactions (buildInitiateTransactions, buildCompleteTransactions, buildFinalizeTransactions), refactors internal _build...Xfer signatures to accept sender addresses, and adds a collectTransactions utility and workflow helpers for completion/finalization. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Executor
participant Builder
participant TxCollector
participant Signer
Client->>Executor: buildInitiateTransactions(request, sender, recipient, quote)
Executor->>Builder: _buildInitiateXfer(request, sender, to, quote)
Builder->>Builder: construct unsigned transfer(s)
Builder-->>Executor: AsyncGenerator<UnsignedTransaction>
Executor->>TxCollector: collectTransactions(asyncGen)
TxCollector->>TxCollector: accumulate UnsignedTransaction[]
TxCollector-->>Executor: UnsignedTransaction[]
Executor-->>Client: UnsignedTransaction[]
Client->>Signer: sign & send selected transactions
Signer-->>Client: signed txids / receipts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 |
b8d5272 to
fe7e197
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@sdk/route/src/types.ts`:
- Around line 605-620: buildFinalizeXfer directly destructures
receipt.attestation.attestation and assumes the receipt is a queued transfer;
add a guard that validates the receipt is in the expected queued state (e.g.,
reuse the same check used by finalize such as isDestinationQueued or at minimum
verify receipt.attestation and receipt.attestation.attestation exist and contain
the expected fields) and throw a clear, descriptive error if the precondition is
not met; ensure the error mentions buildFinalizeXfer and the missing/invalid
attestation so callers get actionable feedback (or alternatively document the
required precondition in the public API if you prefer not to throw).
- Around line 592-603: The complete function in types.ts calls buildCompleteXfer
and signSendWait but returns the original receipt, losing state and txid
updates; capture the result from signSendWait, update the receipt state to
DestinationInitiated and set receipt.destinationTxs (or the appropriate field)
from the transaction id(s) returned by signSendWait (mirroring executor.ts
complete behavior), then return the mutated receipt; if the omission was
intentional, add a brief comment in complete explaining that state/tx updates
are handled elsewhere (e.g., by the track generator).
🧹 Nitpick comments (2)
sdk/route/src/executor/multiToken.ts (1)
617-628:senderparameter is unused inbuildCompleteTransactions.The
senderparameter is accepted but never forwarded toMultiTokenNttRoute.buildCompleteXfer, which only takes(chain, receipt). Same applies tobuildFinalizeTransactions(line 650). This is presumably for API consistency withNttExecutorRoute.buildCompleteTransactions(which does usesender), but it may confuse callers into thinking the sender influences the built transactions.Consider either documenting why
senderis accepted but unused, or using an underscore prefix (_sender) to signal intent.sdk/route/src/executor/executor.ts (1)
699-747: RedundantisDestinationQueuedcheck infinalize.
_buildFinalizeXfer(line 700) already validatesisDestinationQueuedand throws.finalize(line 732) repeats the same check. Not a bug—just a minor redundancy. You could remove the outer check infinalizeor keep it for fail-fast clarity before constructingsender.
67f5381 to
7e75fd0
Compare
7e75fd0 to
608a260
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sdk/route/src/executor/multiToken.ts (1)
617-628: Remove unusedsenderparameter or document why MultiTokenNtt doesn't require it.The
senderparameter in bothbuildCompleteTransactions(line 617) andbuildFinalizeTransactions(line 650) is never used. Unlike the baseNttExecutorRoutewhich forwardssenderto_buildCompleteXferand_buildFinalizeXfer, theMultiTokenNttRoutehelper methods (buildCompleteXferandbuildFinalizeXfer) don't accept a sender parameter. If this is intentional due to MultiTokenNtt protocol differences, add a comment explaining why. Otherwise, remove the unused parameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/route/src/executor/multiToken.ts` around lines 617 - 628, The sender parameter is unused in MultiTokenNttRoute.buildCompleteTransactions and buildFinalizeTransactions; either remove the unused sender parameter from those method signatures to match that MultiTokenNttRoute.buildCompleteXfer/buildFinalizeXfer do not require it, or if omission is intentional, add a short clarifying comment in each method (referencing buildCompleteTransactions, buildFinalizeTransactions and the helper methods MultiTokenNttRoute.buildCompleteXfer and MultiTokenNttRoute.buildFinalizeXfer) explaining why MultiTokenNtt protocol does not need the sender while the base NttExecutorRoute does.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sdk/route/src/executor/multiToken.ts`:
- Around line 617-628: The sender parameter is unused in
MultiTokenNttRoute.buildCompleteTransactions and buildFinalizeTransactions;
either remove the unused sender parameter from those method signatures to match
that MultiTokenNttRoute.buildCompleteXfer/buildFinalizeXfer do not require it,
or if omission is intentional, add a short clarifying comment in each method
(referencing buildCompleteTransactions, buildFinalizeTransactions and the helper
methods MultiTokenNttRoute.buildCompleteXfer and
MultiTokenNttRoute.buildFinalizeXfer) explaining why MultiTokenNtt protocol does
not need the sender while the base NttExecutorRoute does.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c1c097b-39a4-40cf-958f-645f85f399fe
📒 Files selected for processing (4)
sdk/route/src/executor/executor.tssdk/route/src/executor/multiToken.tssdk/route/src/executor/utils.tssdk/route/src/types.ts
Expose building transactions directly instead of having to capture them with a signer
Summary by CodeRabbit
New Features
Refactor