Skip to content

fix(shielded): fee and transfer calculation correctness issues #795

@lklimek

Description

@lklimek

Summary

Several shielded transaction calculations have correctness issues that can cause failed transactions, insufficient funds errors, or premature success reporting.

Found during CodeRabbit review of PR #793 (zk branch).

⚠️ Wallet-touching files

Files marked with 🔀 touch wallet code and may conflict with the wallet refactor (PR #791 / #794). Prioritize these for early resolution.

Issues

1. Note selection ignores transaction fees (src/backend_task/shielded/bundle.rs:683)

select_notes_for_amount() selects notes to cover only the send amount, without reserving fee headroom. Transitions can fail because selected inputs don't cover amount + fees.

Fix: Add a fee_headroom: u64 parameter (or compute estimated fee internally). Select notes for amount + fee_headroom. Update all 3 callers (shielded_transfer, unshield_credits, shielded_withdrawal) to pass an estimated fee. Surface ShieldedInsufficientBalance with required = amount + fee_headroom.

2. Asset-lock fee truncated before safety buffer (src/backend_task/shielded/bundle.rs:401)

platform_fee_credits / CREDITS_PER_DUFF truncates before the 20% buffer is applied. For small amounts the truncation error is proportionally large.

Fix: Use ceiling division before applying the buffer:

let platform_fee_duffs = ((platform_fee_credits + CREDITS_PER_DUFF - 1) / CREDITS_PER_DUFF)
    .saturating_mul(120) / 100;

3. Fee retry loop uses relative scale delta (src/backend_task/core/mod.rs:678)

The loop that reduces total_amount using scale_factor stops when scale_factor change is < 0.0001. For large payments, this exits too early — the absolute duff reduction may still be significant.

Fix: Change stop condition to absolute duff delta: check whether the actual integer reduction in duffs is < MIN_DUFF_STEP (e.g., 100 duffs) or reaches zero.

4. override_fee silently ignored in SPV send path (src/backend_task/core/mod.rs:727)

The SPV send path always uses FeeRate::normal(). WalletPaymentRequest::override_fee is silently discarded.

Fix: Check request.override_fee — either convert to FeeRate and apply, or return an explicit error that override_fee is unsupported in SPV mode.

5. 🔀 Shield balance check ignores platform fees (src/ui/wallets/shield_credits_screen.rs:631)

Balance check compares amount * repeat against read_address_balance() without accounting for platform/transition fees. Users can attempt shields that will fail due to insufficient funds after fees.

Fix: Query the fee estimator, check amount * repeat + estimated_fees * repeat <= balance. Cap max amount or show error if fees make transfers unaffordable.

6. Shield reports success before nonce is usable (src/backend_task/shielded/bundle.rs:214)

shield_credits() returns Ok immediately after broadcast(). The nonce isn't updated until the next sync, so a rapid follow-up shield can reuse the old nonce and fail.

Fix: After broadcast(), call wait_for_response() (or equivalent confirmation) and bump the cached platform nonce before returning Ok(()). Mirror the batch broadcaster's follow-up logic.

Acceptance Criteria

  • select_notes_for_amount accounts for estimated fees
  • Asset-lock fee uses ceiling division
  • Fee retry loop uses absolute duff delta
  • override_fee is either applied or explicitly rejected in SPV path
  • Shield balance check includes fee estimation
  • Shield waits for confirmation before reporting success

🤖 Co-authored by Claudius the Magnificent AI Agent

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions