Skip to content

Conversation

@ovitrif
Copy link
Collaborator

@ovitrif ovitrif commented Aug 5, 2025

Related: #49
Related: #222

Design: Figma v57 Send - Speed

This PR adds UI and logic to enable selecting a preconfigured transaction speed (ie. fee rate) when sending onchain payments.

Setting a custom fee value in the Send flow is for the next PR. Use settings > general > transaction speed > custom to test for now.

Description

  • Fee rate selection UI in send flow
  • Show fee amount and rate details on send and review screen
  • Fix send fee estimation to use selected UTXOs and fee rates
  • Use the transaction speed selected in user settings as default for send
  • Preload fee rates when starting a send flow
  • Preselect UTXOs for deterministic fee estimation
  • Fix to avoid validating amount for LN during onchain send
  • Use VERBOSE log level for logs that are too frequent
  • Log error type together with error message

Preview

speedSelect.mp4

QA Notes

Test - Send Onchain

  • Test with default (NORMAL) Transaction Speed in settings
  • Test with CUSTOM Transaction Speed in settings
  • Test with FAST Transaction Speed in settings
  • Test selecting different transaction speed in send flow
  • Test selecting different fee rates & checking send confirm screen updates

@ovitrif ovitrif requested a review from jvsena42 August 6, 2025 15:24
@ovitrif ovitrif marked this pull request as ready for review August 6, 2025 15:24
@ovitrif ovitrif marked this pull request as draft August 6, 2025 21:39
@ovitrif
Copy link
Collaborator Author

ovitrif commented Aug 7, 2025

The issue where we are seeing different fees is coming from ldk-node.

Android Studio 2025-08-06 000079

We get different return value each time we call node.onchainPayment().calculateTotalFee() with utxosToSpend=null, even if all the other params are the same.

The underlying issue seems to be related to which UTXOs are selected. If we pass the same list of UTXOs we do get the same fee returned each time.

If we don't pass UTXOs, it's probably almost always estimating for a different number of UTXOs.

I noticed the same issue when calling node.onchainPayment().selectUtxosWithAlgorithm() with the BRANCH_AND_BOUND algorithm (default one, didn't check with the others).
Very often it returns a different number of UTXOs.

This makes implementing a reliable fee estimation system quite difficult in the native app, bloating the codebase with hacks to manage state, all for not having to re-estimate fees unless absolutely necessarily.
I already had to re-refactor many times as I was discovering more clearly the root cause of the bugs I saw on UI.

Even after all the patch fixing, I still encounter an issue which I can't properly solve:

🐞 When user selects another fee rate, I must re-run auto UTXOs selection using the current algorithm, because the old list of UTXOs could be invalid.
Currently this is what I am doing, but because of it, when selecting another fee rate, the fee estimate on the review screen gets changed, so it's not displaying the value selected on the previous screen.

cc @coreyphillips, would appreciate your input 🙏🏻

@ovitrif ovitrif marked this pull request as ready for review August 7, 2025 10:41
@ovitrif ovitrif requested review from jvsena42 and removed request for jvsena42 August 7, 2025 11:38
Copy link
Member

@jvsena42 jvsena42 left a comment

Choose a reason for hiding this comment

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

Tested in a Mi A2

@jvsena42 jvsena42 merged commit bc9f14c into master Aug 7, 2025
5 checks passed
@jvsena42 jvsena42 deleted the feat/send-fee branch August 7, 2025 12:33
@ovitrif ovitrif mentioned this pull request Jul 18, 2025
4 tasks
@coreyphillips
Copy link

Hey @ovitrif! I may need to consider adding a few comments or updating the documents around fee estimation.

The randomness around fees when passing null to utxosToSpend is due to how BDK selects UTXO's. Effectively, When calculate_total_fee() is called without specifying UTXOs, it delegates to BDK's build_tx() which uses random selection, resulting in different UTXOs and thus different fees each time. So if we want to remove this randomness we'll need to pass it specific UTXO's.

Otherwise, any time we adjust the desired fee-rate, value and/or UTXO set we'll need to re-run the fee-estimation method since adjusting these values in any way may result in a change to the resulting fee that the user will need to pay.

@ovitrif
Copy link
Collaborator Author

ovitrif commented Aug 11, 2025

Hey @ovitrif! I may need to consider adding a few comments or updating the documents around fee estimation.

The randomness around fees when passing null to utxosToSpend is due to how BDK selects UTXO's. Effectively, When calculate_total_fee() is called without specifying UTXOs, it delegates to BDK's build_tx() which uses random selection, resulting in different UTXOs and thus different fees each time. So if we want to remove this randomness we'll need to pass it specific UTXO's.

Otherwise, any time we adjust the desired fee-rate, value and/or UTXO set we'll need to re-run the fee-estimation method since adjusting these values in any way may result in a change to the resulting fee that the user will need to pay.

Thank you @coreyphillips 🙏🏻

Makes sense to me, would've helped me to know this in advance so I could've avoided the trial-and-error approach I had to take to optimise this for a bug-free UI, by ensuring a more-deterministic fee calculation.

Updating the documentation + comments in code if needed (especially if we want to open a PR with upstream) is a good solution IMHO 🙏🏻

cc. @pwltr - knowing this in advance will help get it right from 1st try on iOS side 🙏🏻 :
TL;DR:

calculateTotalFee needs utxos passed to it, to return a deterministic value. Otherwise the randomness coming from BDK will make the implementation look buggy on the UI by always returning a different fee estimation after each new call to the method.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants