feat/send all api #131
feat/send all api #131
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a new sendAll() API to the TxBuilder that enables draining all wallet assets to a single recipient address. The implementation integrates into the existing transaction builder phases with special handling in the Selection and ChangeCreation phases.
Changes:
- Added
sendAll()builder method that collects all wallet UTxOs and sends them to a single address - Implemented validation to ensure
sendAll()is mutually exclusive with other transaction operations - Updated documentation files with adjusted nav_order values to accommodate the new module
Reviewed changes
Copilot reviewed 141 out of 141 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/evolution/src/sdk/builders/phases/Selection.ts | Added SendAll mode detection and validation logic that collects all available UTxOs |
| packages/evolution/src/sdk/builders/phases/ChangeCreation.ts | Added SendAll output creation with minUTxO validation |
| packages/evolution/src/sdk/builders/operations/SendAll.ts | New operation file defining the SendAll program and types |
| packages/evolution/src/sdk/builders/operations/Operations.ts | Added SendAllParams interface to operation definitions |
| packages/evolution/src/sdk/builders/TransactionBuilder.ts | Added sendAll method to builder interface and sendAllTo field to state |
| packages/evolution/src/sdk/builders/operations/index.ts | Exported SendAll module |
| packages/evolution/docs//*.md, docs/content/docs//*.mdx | Updated nav_order values and added SendAll documentation |
| .changeset/nine-frogs-argue.md | Added changeset describing the new feature |
Comments suppressed due to low confidence (1)
packages/evolution/src/sdk/builders/phases/Selection.ts:39
- The formatAssetsForLog helper function is duplicated across multiple phase files (Selection.ts, ChangeCreation.ts, and Balance.ts). This violates DRY principles and makes maintenance harder. Consider extracting this helper to a shared utils file or the phases/utils.ts module to avoid duplication.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Check if this is a script transaction (has redeemers or deferred redeemers) | ||
| // If so, route to Collateral BEFORE ChangeCreation | ||
| if (stateAfterSelection.redeemers.size > 0 || stateAfterSelection.deferredRedeemers.size > 0) { | ||
| yield* Effect.logDebug("[Selection] Script transaction detected - routing to Collateral phase") | ||
| return { next: "collateral" as const } | ||
| } |
There was a problem hiding this comment.
The SendAll implementation routes script transactions to the Collateral phase before ChangeCreation, but script transactions are explicitly forbidden by the validation checks at lines 250-258. This code path is unreachable because sendAll() validates that there are no certificates (staking operations), which would catch script-based staking, but the redeemer check at line 307 suggests allowing scripts while the validation suggests prohibiting them. Either remove this unreachable code path or clarify the intention if specific script scenarios should be allowed.
| // Create the sendAll output using the txOutputToTransactionOutput helper | ||
| const sendAllOutput = yield* txOutputToTransactionOutput({ | ||
| address: state.sendAllTo, | ||
| assets: tentativeLeftover | ||
| }) |
There was a problem hiding this comment.
The comment on line 235-236 states the sendAll output is stored as a change output 'to ensure the phase loop doesn't double-count it in outputAssets'. However, in sendAll mode, state.outputs is empty (validated in Selection phase), so there's no risk of double-counting. Consider clarifying the comment to explain that storing it as a changeOutput (rather than in state.outputs) is the correct pattern for outputs created during the phase loop, or remove the misleading justification.
| /** | ||
| * SendAll operation - sends all wallet assets to a recipient address. | ||
| * | ||
| * This operation marks the transaction as a "send all" transaction, | ||
| * which triggers special handling in the build phases: | ||
| * 1. All wallet UTxOs are collected as inputs | ||
| * 2. A single output is created with all assets minus fee | ||
| * 3. No change output is created (everything goes to recipient) |
There was a problem hiding this comment.
The documentation states 'No change output is created (everything goes to recipient)' but technically the sendAll output IS created as a change output (stored in buildCtx.changeOutputs). The documentation should clarify that while it's stored internally as a changeOutput, semantically it represents the full transfer to the recipient, not traditional 'change' that returns to the sender.
| if (state.outputs.length > 0) { | ||
| return yield* Effect.fail( | ||
| new TransactionBuilderError({ | ||
| message: | ||
| "sendAll() cannot be used with payToAddress(). " + | ||
| "sendAll automatically creates the output with all wallet assets." | ||
| }) | ||
| ) | ||
| } |
There was a problem hiding this comment.
The sendAll feature lacks test coverage. Given the comprehensive test suite for TxBuilder (TxBuilder.EdgeCases.test.ts, TxBuilder.FeeCalculation.test.ts, etc.), this new feature should have corresponding tests covering: basic sendAll operation, validation of mutual exclusivity with other operations, insufficient funds scenarios, multi-asset wallets, and minUTxO validation.
solidsnakedev
force-pushed
the
feat/send-all-api
branch
2 times, most recently
from
1ae9438 to
20a1a4f
Compare
solidsnakedev
force-pushed
the
feat/send-all-api
branch
from
20a1a4f to
684d8a3
Compare
No description provided.