Skip to content

feat(key-wallet): payload-finalization seam for input-committing special transactions - #991

Open
QuantumExplorer wants to merge 1 commit into
devfrom
feat/special-payload-finalizer-seam
Open

feat(key-wallet): payload-finalization seam for input-committing special transactions#991
QuantumExplorer wants to merge 1 commit into
devfrom
feat/special-payload-finalizer-seam

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Aug 27, 2026

Copy link
Copy Markdown
Member

What

Adds TransactionBuilder::build_signed_reserved_with_payload_finalizer — a two-phase build path for special transactions whose payload commits to the chosen inputs and is itself signed. The motivating case is the ProUpServTx (unban / update-service): its payload's inputs_hash is only knowable after input selection, its operator-BLS payload_sig covers that hash, and each input's ECDSA sighash covers the finished payload — a strict select → hash → BLS-sign → input-sign order that the existing build_signed_reserved (which fuses selection and input signing) cannot express.

How

  • After assemble_unsigned selects, BIP-69-sorts, and reserves the inputs, a finalize_payload closure receives the unsigned transaction (placeholder payload still attached) and returns the finalized payload; the builder installs it and only then computes input sighashes (the legacy sighash consensus-encodes the whole tx, payload included).
  • A placeholder payload (same variant, selection-dependent fields zeroed) is required so coin selection prices the payload bytes into the fee. Guards reject a finalized payload that changes variant or estimates larger than the placeholder — the fee is fixed at selection time, so growth would underpay the configured rate.
  • Every failure path (finalizer error, guards, signing) releases the reservation owner-guarded, identical to the failed-sign discipline in build_signed_reserved (feat(kotlin-sdk): split build/broadcast with reservation release for BIP70-style deferred submission platform#4185).
  • The fee-sizing payload match moves out of calculate_base_size into estimated_payload_size, so the size guard and the fee estimate use one source of truth. No behavior change to existing paths.

Tests

Six new tests: happy path (finalizer sees selected unsigned inputs, payload installed before input signing, inputs_hash matches, inputs stay reserved), finalizer-error release, variant-change guard, size-growth guard, missing-placeholder refusal (finalizer never runs, nothing reserved), and size-guard/fee-estimate agreement. Full key-wallet suite: 678 passed. cargo fmt, clippy --all-features --all-targets, and cargo check --workspace --all-features clean.

Context

First of a three-PR sequence for unbanning PoSe-banned masternodes/evonodes from the mobile wallets: this seam (rust-dashcore) → ProUpServTx orchestrator + FFI (dashpay/platform) → wallet UI. The seam is generic and also unlocks future ProUpRegTx / ProUpRevTx builders. Note for the platform bump: the current platform pin (3d13d983, on chore/sync-fixes-without-swept) has not touched this file, so this commit cherry-picks cleanly onto that lineage if the pin cannot move to dev yet.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added support for finalizing special transaction payloads after inputs are selected but before signing.
    • Added validation to reject missing placeholder payloads, transaction variant changes, or payloads exceeding the reserved size.
  • Bug Fixes

    • Reservations are now safely released when payload finalization fails.

…ial transactions

A ProUpServTx's payload commits to the chosen inputs (inputs_hash) and is
itself BLS-signed by the operator key, while each input's ECDSA sighash
covers the finished payload. build_signed_reserved fuses input selection
and input signing with no seam between them, so such a payload could
never be finalized at the right moment.

Add TransactionBuilder::build_signed_reserved_with_payload_finalizer:
after selection reserves the chosen inputs, a finalizer closure receives
the unsigned transaction (inputs chosen and BIP-69 sorted, placeholder
payload attached) and returns the finalized payload; the builder
installs it and only then signs the inputs. The placeholder is required
so selection prices the payload bytes into the fee, and the finalized
payload must keep the placeholder's variant and estimated size — the fee
is fixed at selection time, so growth would underpay the configured
rate. Every failure path (finalizer error, guards, signing) releases the
reservation owner-guarded, exactly like a failed sign in
build_signed_reserved.

The fee-sizing payload match moves out of calculate_base_size into
estimated_payload_size so the size guard and the fee estimate can never
disagree.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d2dcee19-fce4-48a7-8aeb-d8a190610108

📥 Commits

Reviewing files that changed from the base of the PR and between 237f79a and fee16d9.

📒 Files selected for processing (1)
  • key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

TransactionBuilder now supports finalizing special payloads after input selection and before signing. It validates payload variants and estimated sizes, releases reservations on failure, and adds coverage for successful and rejected finalization paths.

Changes

Payload Finalization Flow

Layer / File(s) Summary
Payload size estimation extraction
key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
The payload-size match moves into estimated_payload_size. Existing payload size calculations remain unchanged.
Reserved transaction payload finalizer
key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
build_signed_reserved_with_payload_finalizer finalizes payloads after input selection, rejects missing placeholders, variant changes, and payload growth, and releases reservations on errors. Tests cover signing, finalizer errors, validation failures, and size estimation.
Estimated code review effort: 4 (Complex) ~45 minutes

Merge Risk: 🟡 Moderate · up to fee16

The new signing path reserves inputs while a payload is finalized and signed, but interruption or expiration during that window can leave inputs unavailable or allow a stale build to return after ownership changes, while payload correctness depends on callers enforcing semantic commitments. Merge should wait for explicit owner acceptance or hardening of the reservation lifecycle and caller contract.

Sequence Diagram(s)

sequenceDiagram
  participant TransactionBuilder
  participant finalize_payload
  participant TransactionSigner
  TransactionBuilder->>TransactionBuilder: Assemble unsigned transaction
  TransactionBuilder->>finalize_payload: Finalize payload using selected inputs
  finalize_payload-->>TransactionBuilder: Return finalized payload
  TransactionBuilder->>TransactionBuilder: Validate variant and estimated size
  TransactionBuilder->>TransactionSigner: Sign finalized transaction
  TransactionSigner-->>TransactionBuilder: Return signed transaction
Loading

Suggested reviewers: zocolini, xdustinface

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a payload-finalization seam in key-wallet for special transactions whose payload commits to selected inputs.
Docstring Coverage ✅ Passed Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/special-payload-finalizer-seam

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.

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.76271% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.22%. Comparing base (237f79a) to head (fee16d9).

Files with missing lines Patch % Lines
.../wallet/managed_wallet_info/transaction_builder.rs 85.76% 42 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #991      +/-   ##
==========================================
+ Coverage   77.14%   77.22%   +0.08%     
==========================================
  Files         329      329              
  Lines       82998    83244     +246     
==========================================
+ Hits        64026    64284     +258     
+ Misses      18972    18960      -12     
Flag Coverage Δ
core 78.25% <ø> (ø)
ffi 52.41% <ø> (+<0.01%) ⬆️
rpc 20.00% <ø> (ø)
spv 92.07% <ø> (+0.07%) ⬆️
wallet 79.40% <85.76%> (+0.18%) ⬆️
Files with missing lines Coverage Δ
.../wallet/managed_wallet_info/transaction_builder.rs 91.36% <85.76%> (+1.29%) ⬆️

... and 7 files with indirect coverage changes

@github-actions github-actions Bot added the ready-for-review CodeRabbit has approved this PR label Aug 27, 2026

@QuantumExplorer QuantumExplorer left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review CodeRabbit has approved this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant