fix: address path ordering with mixed inputs#1063
Conversation
Use a Map to collect address paths during the two-pass validation (tokens first, HTR second), then rebuild in this.inputs order. Fixes invalid signatures when users provide HTR inputs before token inputs. Closes #1057 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduce a Map-based collection of address paths during UTXO validation so utxosAddressPath is rebuilt in the original input order; update validateUtxos signature to accept an optional addressPathMap and throw when an input's address path is missing. Tests added to cover HTR/token ordering and rejection cases. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1063 +/- ##
=======================================
Coverage 87.93% 87.94%
=======================================
Files 114 114
Lines 8910 8917 +7
Branches 2030 2032 +2
=======================================
+ Hits 7835 7842 +7
Misses 1047 1047
Partials 28 28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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)
src/wallet/sendTransactionWalletService.ts (1)
445-464:⚠️ Potential issue | 🟠 MajorHandle manual HTR inputs even when the required HTR amount is
0n.Line 447 still skips the native pass when
htrAmountis0n. If the caller manually includes HTR UTXOs for a custom-token-only send, those inputs never create HTR change and never populateaddressPathMap, so Line 461 now fails withAddress path not found....SendTransactionWalletService.prepareTxData()already validates manual HTR inputs against0n, so these two preparation paths still diverge here. A smallhasUserHtrInput/cached-UTXO flag from the first scan would let this branch runvalidateUtxoswhenhtrAmount > 0n || hasUserHtrInput.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wallet/sendTransactionWalletService.ts` around lines 445 - 464, The code skips the native-UTXO validation when htrAmount === 0n which breaks later lookup for manually provided HTR inputs; update the condition around the validateUtxos call in SendTransactionWalletService (the htrAmount / NATIVE_TOKEN_UID block) to run when htrAmount > 0n OR when a flag indicating the caller provided manual HTR UTXOs is true (e.g., hasUserHtrInput discovered during the initial UTXO scan in prepareTxData). Ensure that hasUserHtrInput is set during the first scan of inputs/utxos, then change the if from (htrAmount > 0n) to (htrAmount > 0n || hasUserHtrInput) so validateUtxos(htrTokenAmount, { onlyNative: true, addressPathMap }) is invoked and addressPathMap gets populated before rebuilding utxosAddressPath from this.inputs.
🤖 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 `@src/wallet/sendTransactionWalletService.ts`:
- Around line 445-464: The code skips the native-UTXO validation when htrAmount
=== 0n which breaks later lookup for manually provided HTR inputs; update the
condition around the validateUtxos call in SendTransactionWalletService (the
htrAmount / NATIVE_TOKEN_UID block) to run when htrAmount > 0n OR when a flag
indicating the caller provided manual HTR UTXOs is true (e.g., hasUserHtrInput
discovered during the initial UTXO scan in prepareTxData). Ensure that
hasUserHtrInput is set during the first scan of inputs/utxos, then change the if
from (htrAmount > 0n) to (htrAmount > 0n || hasUserHtrInput) so
validateUtxos(htrTokenAmount, { onlyNative: true, addressPathMap }) is invoked
and addressPathMap gets populated before rebuilding utxosAddressPath from
this.inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dbae6d47-6258-4b8b-b394-dafa5c7b29ca
📒 Files selected for processing (1)
src/wallet/sendTransactionWalletService.ts
Add tests for the utxosAddressPath ordering fix: - Verify paths match input order when HTR comes before token inputs (regression test for #1057) - Verify rejection of unnecessary HTR inputs when no HTR is needed in the transaction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a user provides an HTR input but the transaction requires no HTR (no HTR outputs, no fees), the error now explains the root cause instead of showing a generic "address path not found" message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
Fixes the
utxosAddressPathordering bug inSendTransactionWalletService.prepareTx()when user-provided inputs mix HTR and custom token UTXOs.Closes #1057
Problem
When users provide manual inputs,
prepareTx()validates them in two passes:ignoreNative: true)onlyNative: true)The resulting
utxosAddressPathis concatenated as[token_paths..., htr_paths...], regardless of the original input order. During signing, inputiis signed withutxosAddressPath[i], causing mismatched signatures when HTR inputs appear before token inputs.Why two passes are necessary
The two-pass structure is load-bearing and cannot be collapsed into a single pass. Fee-token change outputs are discovered during the first pass (custom token validation), and each one increases
_feeAmountbyFEE_PER_OUTPUT. The correct HTR amount can only be computed after all fee-token changes are accounted for. Processing everything in one pass would validate HTR against an incomplete fee total.Fix
Adds an optional
addressPathMap: Map<string, string>parameter tovalidateUtxos. Both passes populate this map keyed by${txId}:${index}, thenprepareTxrebuildsutxosAddressPathinthis.inputsorder. This preserves the two-pass fee calculation while decoupling address path collection from array concatenation order.Acceptance criteria
🤖 Generated with Claude Code
Summary by CodeRabbit