fix: not enough balance to pay a fee in platform to core transfers

[lklimek]

Fee estimation is invalid. Refactored to use dpp min fee instead.

Note this PR only addresses platform-to-core transfers. There is a follow-up PR #514 that addresses platform-to-platform (blocked right now due to dpp bug). Other transfers will follow.

Summary by CodeRabbit

• New Features

  • Dynamic, state-transition-based fee estimation for platform sends and withdrawals, plus a max-fee estimator in the send UI.
  • Iterative address allocation that recalculates fees as inputs stabilize and accounts for destination types.

• Bug Fixes

  • More accurate fee-payer selection and shortfall calculations; improved handling of destination filtering for sends and withdrawals.

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces state-transition-based withdrawal and funding fee estimation and a fee-aware, iterative platform-address allocator; integrates these into platform↔platform and core↔platform send/withdraw flows, updates max-fee and shortfall calculations, and adds imports for new fee/transition types. Private helpers added; external APIs preserved. (47 words)

Changes

Cohort / File(s)

Summary

Send screen — allocation & fee estimation src/ui/wallets/send_screen.rs

Adds estimate_withdrawal_fee_from_transition(...), estimate_address_funding_fee_from_transition(...), allocate_platform_addresses_with_fee(...), and estimate_max_fee_for_platform_send(...). Updates allocate_platform_addresses(...) to delegate to the new helper with a fee callback; replaces prior allocation and fee logic in platform↔platform and core↔platform send & withdraw paths to use iterative, state-transition-based fee estimation; updates max-fee and shortfall calculations and imports for new fee/transition types.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Wallet as WalletSendScreen
  participant Allocator as AddressAllocator
  participant FeeEstimator
  participant Store as AddressStore

  User->>Wallet: initiate send
  Wallet->>Allocator: allocate(inputs, amount, destination, fee_calculator)
  Allocator->>Store: fetch/filter & sort candidate addresses
  Allocator->>FeeEstimator: estimate fee for current inputs (state-transition)
  FeeEstimator-->>Allocator: fee estimate
  alt fee changed
    Allocator->>Store: adjust inputs, recompute
    Allocator->>FeeEstimator: re-estimate fee (loop)
  end
  Allocator-->>Wallet: allocation result (inputs, fee_payer, estimated_fee, shortfall)
  Wallet->>User: present/send confirmation

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• AddressFundsTransferTransition::calculate_min_required_fee is too low platform#3040 — Addresses state-transition fee underestimates; this PR adds state-transition-based fee estimators for withdrawals and funding that map to that objective.

Poem

🐇 I hop through ledgers, counting cents,
I pick the coins with careful sense,
Recalculate, then choose a payer,
Till numbers settle — light and fairer,
I thump my foot: the send's prepared.

🚥 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 directly addresses the main issue fixed: fee estimation for platform-to-core transfers when balance is insufficient to pay 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 docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/not-enough-balance-for-fee

Important

Action Needed: IP Allowlist Update

If your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:

• ✨ 136.113.208.247/32 (new)
• 34.170.211.100/32
• 35.222.179.152/32

Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/ui/wallets/send_screen.rs (1)

77-82: ⚠️ Potential issue | 🟡 Minor

Documentation is inconsistent with implementation.

The comment at line 79 states "Sorts addresses by balance descending (largest first)" but the implementation at line 100 sorts ascending (a.2.cmp(&b.2)). The comment should be updated to reflect the new behavior.

📝 Suggested fix

 /// Algorithm:
 /// 1. Filters out the destination address (can't be both input and output)
-/// 2. Sorts addresses by balance descending (largest first)
-/// 3. The highest-balance address pays the fee
+/// 2. Sorts addresses by balance ascending (smallest first, largest last)
+/// 3. The highest-balance address (last in sorted order) pays the fee
 /// 4. Iteratively allocates until fee estimate converges
 /// 5. Fee payer is always included in inputs (even with 0 contribution) so fee can be deducted

🧹 Nitpick comments (1)

src/ui/wallets/send_screen.rs (1)

864-869: Unnecessary allocation for logging.

The inputs variable creates a redundant copy of allocation.inputs. Since BTreeMap<PlatformAddress, u64> should already implement Debug, you can reference the original directly.

♻️ Suggested simplification

-        let inputs = allocation
-            .inputs
-            .iter()
-            .map(|(addr, amt)| (*addr, *amt))
-            .collect::<BTreeMap<_, _>>();
         tracing::debug!(
-            inputs = ?inputs,
+            inputs = ?allocation.inputs,
             "Platform withdrawal: {} inputs totaling {}, withdraw {}, fee {} (payer idx {})",

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/ui/wallets/send_screen.rs (3)

70-71: ⚠️ Potential issue | 🟡 Minor

Update the sorted_addresses comment to match ascending order.

The struct comment still says “descending” but the list is now ascending.

✏️ Suggested doc fix

-    /// Addresses sorted by balance descending (for UI display)
+    /// Addresses sorted by balance ascending (for UI display)

---

1187-1214: ⚠️ Potential issue | 🟡 Minor

Max amount calculation still assumes largest-first selection.

The “Max” computation sorts descending and sums the largest balances, but the actual allocator now uses smallest-first with the largest reserved for fee payer. This can overstate the max and make the “Max” button fail with a shortfall. Align the max calculation with the allocator’s heuristic.

✅ Suggested alignment with smallest-first + fee-payer

-                // Filter out destination and sort by balance descending
+                // Filter out destination and sort by balance ascending (fee payer is largest)
                 let mut sorted_addresses: Vec<_> = addresses
                     .iter()
                     .filter(|(addr, _, _)| destination.as_ref() != Some(addr))
                     .cloned()
                     .collect();
-                sorted_addresses.sort_by(|a, b| b.2.cmp(&a.2));
+                sorted_addresses.sort_by(|a, b| a.2.cmp(&b.2));
 
                 // Sum balances from top addresses, limited by MAX_PLATFORM_INPUTS.
                 let usable_count = sorted_addresses.len().min(MAX_PLATFORM_INPUTS);
-                let total: u64 = sorted_addresses
-                    .iter()
-                    .take(MAX_PLATFORM_INPUTS)
-                    .map(|(_, _, balance)| *balance)
-                    .sum();
+                let total: u64 = if sorted_addresses.len() <= MAX_PLATFORM_INPUTS {
+                    sorted_addresses.iter().map(|(_, _, b)| *b).sum()
+                } else {
+                    let fee_payer = sorted_addresses.last().map(|(_, _, b)| *b).unwrap_or(0);
+                    let small_sum: u64 = sorted_addresses
+                        .iter()
+                        .take(MAX_PLATFORM_INPUTS - 1)
+                        .map(|(_, _, b)| *b)
+                        .sum();
+                    small_sum + fee_payer
+                };

---

1-33: ⚠️ Potential issue | 🟡 Minor

Move the import on line 33 to the top with other use statements.

The use crate::model::fee_estimation::PlatformFeeEstimator; statement on line 33 should be grouped with the other imports at the top of the file, not placed after the const declaration. All use statements must come before other code items per Rust conventions and will be caught by clippy.

🤖 Fix all issues with AI agents

In `@src/ui/wallets/send_screen.rs`:
- Around line 127-150: The current smallest-first allocation using
sorted_addresses can exhaust input slots with tiny balances and leave remaining
> 0 even when a feasible selection exists; update send allocation to, after the
existing smallest-first loop (using MAX_PLATFORM_INPUTS, sorted_addresses,
inputs, remaining), check if remaining > 0 and inputs.len() < max_non_fee_inputs
and then perform a fallback largest-first pass over the same addresses
(excluding the fee payer at the end) to add larger balances until remaining == 0
or inputs.len() reaches max_non_fee_inputs, ensuring you do not duplicate an
address already in inputs and still reserve the fee payer handling and
estimated_fee logic for the final inclusion.

🧹 Nitpick comments (1)

src/ui/wallets/send_screen.rs (1)

864-872: Avoid cloning the inputs map just for logging.

allocation.inputs is already a BTreeMap; you can log it directly to skip an extra allocation.

🧹 Small logging cleanup

-        let inputs = allocation
-            .inputs
-            .iter()
-            .map(|(addr, amt)| (*addr, *amt))
-            .collect::<BTreeMap<_, _>>();
         tracing::debug!(
-            inputs = ?inputs,
+            inputs = ?allocation.inputs,
             "Platform withdrawal: {} inputs totaling {}, withdraw {}, fee {} (payer idx {})",

[lklimek]

@coderabbitai review all

[coderabbitai]

@lklimek: I'll review all the changes in this PR.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/ui/wallets/send_screen.rs`:
- Around line 87-90: The call to
transition.calculate_min_required_fee(platform_version).unwrap_or(0) silently
falls back to zero; change this to surface the error instead of defaulting to 0
by either propagating the Result from calculate_min_required_fee or logging the
failure before using a fallback. Replace the unwrap_or(0) usage in
send_screen.rs (the transition.calculate_min_required_fee(...) call) with code
that matches the function flow: return or propagate the error to the caller, or
log a warning via the existing logger with the error details and only then use
an explicit safe fallback; ensure the log message mentions
calculate_min_required_fee and the platform_version so the failure context is
clear.
- Around line 76-85: The fee strategy currently hardcodes DeductFromInput(0) in
the AddressCreditWithdrawalTransition construction; change the function
signature that builds this transition to accept a fee_payer_index parameter (use
the same name as other modules like withdraw_from_platform_address.rs and
transfer_platform_credits.rs), and use fee_strategy:
vec![AddressFundsFeeStrategyStep::DeductFromInput(fee_payer_index)] instead of
0. Also replace any unwrap_or(0) calls used to derive the fee payer index with
proper error propagation (use ? to return an error) so failures bubble up
consistently; update references in AddressCreditWithdrawalTransitionV0
construction and any call sites to pass the new fee_payer_index argument.

🧹 Nitpick comments (1)

src/ui/wallets/send_screen.rs (1)

221-317: Consider consolidating duplicate allocation logic.

The allocate_platform_addresses function (lines 221-317) shares significant logic with the new allocate_platform_addresses_with_fee function. The older function could be refactored to delegate to the new one with a default fee calculator based on estimate_platform_fee.

This would reduce code duplication and ensure both paths stay in sync.

♻️ Example refactor

 fn allocate_platform_addresses(
     estimator: &PlatformFeeEstimator,
     addresses: &[(PlatformAddress, Address, u64)],
     amount_credits: u64,
     destination: Option<&PlatformAddress>,
 ) -> AddressAllocationResult {
-    // Filter out the destination address if provided (protocol doesn't allow same address as input and output)
-    let filtered: Vec<_> = addresses
-        .iter()
-        .filter(|(platform_addr, _, _)| destination != Some(platform_addr))
-        .cloned()
-        .collect();
-
-    // ... (rest of the duplicate logic)
+    allocate_platform_addresses_with_fee(addresses, amount_credits, destination, |inputs| {
+        estimate_platform_fee(estimator, inputs.len().max(1))
+    })
 }

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/ui/wallets/send_screen.rs`:
- Around line 95-116: The fee estimation in
estimate_address_funding_fee_from_transition uses AssetLockProof::default(),
which underrates fees because real proofs are larger; replace the default with a
representative AssetLockProof (or construct one with fields sized to match
expected serialized size) when building AddressFundingFromAssetLockTransitionV0
so calculate_min_required_fee() reflects realistic storage costs, or
alternatively add an explicit comment/documentation in
estimate_address_funding_fee_from_transition indicating the estimate is
intentionally approximate and why a default proof is acceptable; update any test
or helper that creates the proof to use the representative/dummy-sized proof so
downstream fee calculations are accurate.

[pauldelucia]

Didn't work first time but worked second time...