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

[Wallet][RPC] Add subtract-fee-from-amount option in Create[Fund]Transaction #2341

Merged
merged 10 commits into from
Jun 13, 2021

Conversation

random-zebra
Copy link

@random-zebra random-zebra commented Apr 27, 2021

Add a feature requested many times (#894, #1079, #2196, ...)

Allow the user to deduct the fee from one or more of the transaction recipient amounts.
The most common use-case is when sending the whole balance, or a selection of UTXOs, without getting any change back.
This is already possible by selecting "after-fee-amount" in coin-control, but such metod is not user-friendly (e.g. in case of shield recipients, to get the correct value, the destinations must already be filled before opening coin-control and copying the "after-fee" value).
Plus, this workaround is only available in the GUI.

The subtract-fee option makes it easier, and enables it via the cli as well.

Here it's exposed only to the RPC, both for transparent and shield recipients:

  • sendtoaddress
  • sendmany
  • shieldsendmany
  • fundrawtransaction

GUI connection will be added in a successive PR (which will close those issues).

Last commit is coming from bitcoin#10294 (in #2351)

Built on top of:

@random-zebra
Copy link
Author

Rebased on master.

@random-zebra random-zebra force-pushed the 202104_subtractfeefromamount branch 4 times, most recently from 40b5547 to 8fc6138 Compare May 17, 2021 22:48
@random-zebra random-zebra force-pushed the 202104_subtractfeefromamount branch 2 times, most recently from 83a015c to 1ae2364 Compare June 2, 2021 10:47
@random-zebra
Copy link
Author

Rebased on master, conflicts fixed.

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Code review till d1cde6f.

src/sapling/sapling_operation.cpp Outdated Show resolved Hide resolved
@random-zebra
Copy link
Author

Rebased on master, fixing conflicts, and added @furszy's CRecipient refactoring.

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

rebase + cherry-pick utACK a1bd590.

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK a1bd590

@furszy furszy merged commit e8d2cf0 into PIVX-Project:master Jun 13, 2021
random-zebra added a commit that referenced this pull request Jul 18, 2021
3e860c5 [GUI] Display actual recipient amount with sffa in confirmation dlg (random-zebra)
3f45e09 GUI: Add styling for subtract fee from amount checkbox for light and dark themes. (furszy)
26ac915 [GUI] Add checkable btn in the contextual menu, for sffa with multirows (random-zebra)
c8e74e0 [GUI] Add checkboxSubtractFeeFromAmount checkbox in sendmultirow (random-zebra)
04e43f9 [CSS] Prepare styling for btn-list-menu::checked (font-size/color) (random-zebra)

Pull request description:

  Follow up to #2341.
  As per title, add controls for this feature in the graphical interface.

  For the moment, it's a simple checkbox, which is visible in case of single recipient.
  The confirmation dialog shows the difference in the "total amount" (paid) when the fee is subtracted, or not, from the recipient amount (in the example, sending `1.00 PIV`).

  <div>
  <img src="https://user-images.githubusercontent.com/18186894/116305097-99fdff80-a7a3-11eb-83a7-35d3d25b8881.png" width="700px"/>

  <img src="https://user-images.githubusercontent.com/18186894/116305490-23adcd00-a7a4-11eb-92dc-4b5a68089653.png" width="700px"/>
  </div>

  ---

  When there are multiple recipients, the checkbox is hidden, and controlled by a checkable button inside the contextual menu.

  <div>
  <img src="https://user-images.githubusercontent.com/18186894/116305579-44762280-a7a4-11eb-84dc-30ac3a7f29ce.png" width="600px" margin="center"/>
  </div>

  Maybe this is not very intuitive, or pretty, but it does the job.
  Later on, possibly with the help of @Neoperol, we can design a better placement for these controls.

  Closes #894
  Closes #2196

  Obviously based on top of
  - [x] #2341

  which must be reviewed before this one.

ACKs for top commit:
  furszy:
    Tested ACK 3e860c5.
  Fuzzbawls:
    ACK 3e860c5

Tree-SHA512: 651045d9fd655b7108682bd5478f2ae2cd5f6c61a725e952d6705a9df0809fab0c1b99152340d829262e7133c3228db0e1fb9abdcc1671dc5d7898f9ff485038
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants