Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix custom withdrawal fee bug #5602

Merged
merged 1 commit into from Jul 6, 2021
Merged

Fix custom withdrawal fee bug #5602

merged 1 commit into from Jul 6, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jul 3, 2021

The wallet code was by default using the value on Setting/Preferences instead of the value chosen on the Withdrawal screen.

Fixes #5575

  • Pass the chosen fee rate to the wallet when doing a withdraw.
  • Calculate sats/vb in the summary popup instead of showing the value of the input text field.

Steps to reproduce the issue (regtest)

  • Prerequisite: Use custom withdrawal fee is unchecked in Settings/Preferences.
  • Go to Funds/Receive funds select a receiving address and copy to clipboard.
  • Go to Funds/Send funds, paste your address into the withdraw to address field.
  • Take note of the default suggested transaction fee (in this case 50 sats/vb).
  • Click use custom value for fee, enter 2 sats/vbyte or the lowest value permissible.
  • Click Withdraw Selected. Note the mining fee and the sats/vb reported alongside it.
  • Click yes to withdraw.
  • Now go to the log where it details the transaction, note the weight e.g. 110 virtual bytes, and note the fee paid e.g. 0.00055 BTC ( = 55000 sats).
  • Divide the fee paid by virtual bytes to get the actual fee per vb paid, in this case 55000 / 110 = 50 sats/vb.
  • The fee used is the value originally shown in the fee text box, instead of the one the user entered.

[update] Refactored the fix to pass the chosen fee rate to the wallet instead of setting the custom withdrawal flag in user preferences. I think this approach is cleaner and more in line with @BtcContributor original idea.

Also: fixes #5605

@ripcurlx ripcurlx added this to the v1.7.1 milestone Jul 6, 2021
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

@ripcurlx ripcurlx merged commit f4ca766 into bisq-network:master Jul 6, 2021
@MatthewCroughan
Copy link

#5601

@ghost ghost mentioned this pull request Jul 18, 2021
@ghost ghost deleted the fix_withdrawal_fee_bug branch May 29, 2022 22:50
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.

Confusing Withdrawal Pop-up Transaction was made with 20 sat/vb instead of 2 sat/vb
2 participants