backport: fix(wallet): calculate asset lock tx fee dynamically based on input count by lklimek · Pull Request #636 · dashpay/dash-evo-tool

lklimek · 2026-02-24T09:47:33Z

Summary

Replaces hardcoded 3000 duff fee with dynamic calculation based on actual transaction size
Estimates size as: 10B header + (inputs × 148B) + (outputs × 34B) + 60B payload
Properly recalculates change output after real fee is known
Handles fee shortfall gracefully when allow_take_fee_from_amount is set

Test plan

Manual test scenarios: docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md

Single-input asset lock (fee minimum 3000 duffs)
Multiple-input asset lock (fee scales with inputs)
Tight balance with fee deduction from amount
Insufficient funds error with clear message
Change output recalculated after dynamic fee
Transaction accepted by network (meets min relay fee)

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

Summary by CodeRabbit

Bug Fixes
- Improved asset lock transaction fee calculation to use dynamic sizing instead of fixed values, with a minimum fee of 3000 duffs.
- Enhanced handling of transaction change outputs and fee deduction scenarios.
Documentation
- Added comprehensive manual test scenarios for asset lock fee behavior validation.

…ount Replace hardcoded 3000 duff fee with dynamic fee calculation that accounts for actual number of inputs. Estimates tx size using standard component sizes (P2PKH input ~148B, output ~34B, header ~10B, payload ~60B) and uses max(3000, estimated_size) to always meet the min relay fee. Properly handles fee shortfall when allow_take_fee_from_amount is set, and returns clear error messages for insufficient funds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai · 2026-02-24T09:48:06Z

Warning

Rate limit exceeded

@lklimek has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 14 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 2a6c772 and a846657.

📒 Files selected for processing (2)

docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md
src/model/wallet/asset_lock_transaction.rs

📝 Walkthrough

Walkthrough

This pull request introduces dynamic asset lock fee calculation logic that replaces hard-coded fees with a size-based computation model, alongside comprehensive manual test documentation. The implementation includes fee estimation, change output handling, and dust threshold logic within the asset_lock_transaction module.

Changes

Cohort / File(s)	Summary
Documentation `docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md`	New markdown document detailing manual test scenarios for dynamic asset lock fee calculation, covering preconditions, step-by-step actions, expected results, and edge cases across multiple transaction types and funding scenarios.
Core Fee Calculation Logic `src/model/wallet/asset_lock_transaction.rs`	Introduces `AssetLockFeeResult` struct, constants `MIN_ASSET_LOCK_FEE` and `DUST_THRESHOLD`, and new function `calculate_asset_lock_fee` for iterative fee computation with change/dust handling. Replaces hard-coded fee logic in `asset_lock_transaction_from_private_key`. Includes expanded test coverage for single/multi-input cases, change scenarios, insufficient funds, and edge cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 From fixed fees to dynamic flows,
Dust and change now properly grow,
Transactions size-aware and true,
The asset lock gets tested anew! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: dynamic asset lock transaction fee calculation based on input count, replacing hardcoded fees.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch zk-extract/asset-lock-fee-fix

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR backports a fix to make asset-lock transaction fees dynamic (instead of a hardcoded 3000 duffs) by estimating transaction size from the selected input/output count, then recalculating the amount/change based on the resulting fee.

Changes:

Select UTXOs using an initial fee estimate, then compute a dynamic fee from estimated tx size and recompute actual_amount + change.
Add graceful handling for fee shortfall when allow_take_fee_from_amount is enabled.
Add a new manual test plan document covering common and edge-case scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
`src/model/wallet/asset_lock_transaction.rs`	Replaces hardcoded fee with size-based fee estimation and recalculates amount/change accordingly.
`docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md`	Adds manual test scenarios for validating dynamic fee behavior and related edge cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-24T11:24:12Z

@lklimek I've opened a new pull request, #645, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Extract fee/amount/change computation from the inline logic in asset_lock_transaction_from_private_key() into a standalone calculate_asset_lock_fee() function. Uses an iterative approach that fixes a bug where the fee was computed assuming a change output existed (based on the initial 3000-duff estimate). When the real fee eliminated the change, the code overestimated by 34 bytes and could trigger a false insufficient-funds error on edge cases with many inputs. Also removes stale edge case E3 from manual test scenarios (referenced a database refactor not in this PR). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Covers basic fee scenarios (minimum fee, scaling with inputs, exact change, fee-from-amount, insufficient funds) and two regression tests that prove the bug fixed in the previous commit — false insufficient funds when change disappears under the real fee with many inputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Use checked_add for requested_amount + fee to prevent u64 overflow - Add DUST_THRESHOLD (546 duffs) — change below this is absorbed into the fee instead of creating a non-standard dust output - Replace hardcoded 3_000u64 with MIN_ASSET_LOCK_FEE constant in caller Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md (1)

256-272: Edge-case numbering skips E3.

Consider adding E3 or renumbering so the checklist is unambiguous during manual runs.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md` around
lines 256 - 272, The edge-case checklist skips E3 (currently has E1, E2, E4);
update the headings so numbering is sequential by either inserting a new E3 test
case (e.g., a short description for the missing scenario) or renumbering E4→E3
and adjust any related headings/descriptions; ensure any references or links to
these headings (e.g., "E4: Concurrent Asset Lock Creation") are updated to the
new symbol names so the manual test checklist is unambiguous.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/model/wallet/asset_lock_transaction.rs`:
- Around line 247-276: The selection uses MIN_ASSET_LOCK_FEE to pick UTXOs via
take_unspent_utxos_for but then recomputes the fee in calculate_asset_lock_fee
(which uses estimate_tx_size), so when inputs grow the actual fee may exceed the
initial estimate and cause failure; modify the logic to loop: after
take_unspent_utxos_for returns utxos, compute total_input_value, num_inputs and
call calculate_asset_lock_fee, and if it returns an insufficient-funds error
then restore utxos and retry selection with an increased fee estimate (e.g., set
the new estimate to the returned required fee or add a small buffer) by calling
take_unspent_utxos_for again until success or no more UTXOs; ensure the
rollback/restore uses self.utxos.extend(utxos.into_iter()) as currently done and
preserve existing error propagation when retries are exhausted.

---

Nitpick comments:
In `@docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md`:
- Around line 256-272: The edge-case checklist skips E3 (currently has E1, E2,
E4); update the headings so numbering is sequential by either inserting a new E3
test case (e.g., a short description for the missing scenario) or renumbering
E4→E3 and adjust any related headings/descriptions; ensure any references or
links to these headings (e.g., "E4: Concurrent Asset Lock Creation") are updated
to the new symbol names so the manual test checklist is unambiguous.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4ac7ab and 2a6c772.

📒 Files selected for processing (2)

docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md
src/model/wallet/asset_lock_transaction.rs

…rios.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…ror path The rollback code used `extend()` which expected `(Address, HashMap<OutPoint, TxOut>)` but received `(OutPoint, (TxOut, Address))` from `take_unspent_utxos_for()`. Re-group UTXOs by address using entry API to match `self.utxos` structure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace the old asset-lock-fee-fix test scenarios (from PR #636) with comprehensive scenarios covering the full PR #651 scope: Core wallet payment fees, asset lock fees, Platform fee consolidation, UTXO removal consistency, and edge cases (SPV mode, dust threshold, large input count). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

lklimek self-assigned this Feb 24, 2026

docs: add manual test scenarios for asset lock fee fix

87bd1b2

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

lklimek mentioned this pull request Feb 24, 2026

backport: merge v1.0-dev (including extracted non-ZK fixes) back into zk #644

Merged

4 tasks

Merge branch 'v1.0-dev' into zk-extract/asset-lock-fee-fix

55b929e

lklimek marked this pull request as ready for review February 24, 2026 11:15

lklimek requested a review from PastaPastaPasta as a code owner February 24, 2026 11:15

lklimek requested a review from Copilot February 24, 2026 11:15

Copilot started reviewing on behalf of lklimek February 24, 2026 11:16 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 24, 2026

fix(wallet): defer UTXO removal until asset lock tx is fully built and signed #645

Merged

lklimek and others added 3 commits February 24, 2026 12:26

Apply suggestions from code review

963e3ad

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

lklimek requested a review from Copilot February 24, 2026 11:39

Copilot started reviewing on behalf of lklimek February 24, 2026 11:40 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

lklimek and others added 2 commits February 24, 2026 12:55

Apply suggestions from code review

2a6c772

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

lklimek requested a review from Copilot February 24, 2026 12:00

Copilot started reviewing on behalf of lklimek February 24, 2026 12:00 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

Comment thread src/model/wallet/asset_lock_transaction.rs

Comment thread src/model/wallet/asset_lock_transaction.rs

Comment thread docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md Outdated

coderabbitai Bot reviewed Feb 24, 2026

View reviewed changes

Comment thread src/model/wallet/asset_lock_transaction.rs

lklimek and others added 2 commits February 24, 2026 14:09

Update docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scena…

f81c69a

…rios.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

lklimek merged commit 18d4df8 into v1.0-dev Feb 24, 2026
4 checks passed

lklimek deleted the zk-extract/asset-lock-fee-fix branch February 24, 2026 13:50

Conversation

lklimek commented Feb 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lklimek commented Feb 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 24, 2026 •

edited

Loading