wallet: feature: transfer amount with fee included #8861

jeffro256 · 2023-05-16T18:44:46Z

To transfer ~5 XMR to an address such that your balance drops by exactly 5 XMR, provide a subtractfeefrom flag to the transfer command. For example:

transfer 76bDHojqFYiFCCYYtzTveJ8oFtmpNp3X1TgV2oKP7rHmZyFK1RvyE4r8vsJzf7SyNohMnbKT9wbcD3XUTgsZLX8LU5JBCfm 5 subtractfeefrom=all

If my walet balance was exactly 30 XMR before this transaction, it will be exactly 25 XMR afterwards and the destination address will receive slightly less than 5 XMR. You can manually select which destinations fund the transaction fee and which ones do not by providing the destination index. For example:

transfer 75sr8AAr... 3 74M7W4eg... 4 7AbWqDZ6... 5 subtractfeefrom=0,2

This will drop your balance by exactly 12 XMR including fees and will spread the fee cost equally over destinations with addresses 75sr8AAr... and 7AbWqDZ6....

This feature was requested by @LocalMonero.

LocalMonero · 2023-05-16T18:46:43Z

This closes #7980

jeffro256 · 2023-06-12T22:27:35Z

Depends on #8888

woodser · 2023-06-20T15:57:14Z

This is a needed feature for application developers which I will be using too.

Is anything else needed before merging? @luigi1111

jeffro256 · 2023-06-20T16:00:55Z

@woodser if you would like to see this happen, would you mind reviewing #8888 as well if you’d like?

selsta · 2023-06-20T19:09:48Z

@woodser it needs an approval for this and #8888

woodser · 2023-07-08T23:50:03Z

@jeffro256 Please rebase on master.

src/simplewallet/simplewallet.cpp

src/wallet/wallet2.cpp

src/wallet/wallet_rpc_server_commands_defs.h

woodser · 2023-07-08T22:46:44Z

src/wallet/wallet_rpc_server.cpp

@@ -1086,7 +1092,7 @@ namespace tools
    {
      uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0);
      uint32_t priority = m_wallet->adjust_priority(req.priority);
-      std::vector<wallet2::pending_tx> ptx_vector = m_wallet->create_transactions_2(dsts, mixin, req.unlock_time, priority, extra, req.account_index, req.subaddr_indices);
+      std::vector<wallet2::pending_tx> ptx_vector = m_wallet->create_transactions_2(dsts, mixin, req.unlock_time, priority, extra, req.account_index, req.subaddr_indices, req.subtract_fee_from_outputs);


This needs added to on_transfer_split.

For me, it doesn't make as much sense to add this to the split transfer interface since the fees have to be paid out of certain destinations. If those certain destinations are split over multiple transactions, it's unclear which destinations should pay for transactions if a transaction doesn't contain any "subtractable" destinations. Also, this feature can be expanded upon in a future PR for reviewer's sake, and I'd imagine this feature is mainly useful for small transfers (like customers paying for an item with fee included).

simplewallet.cpp already permits applying this feature across split transactions, so either that behavior needs to be well defined and supported or completely disabled.

Then monero-wallet-rpc should be consistent. So if we don't support transfer_split in monero-wallet-rpc now, this feature should be disabled for split transactions in simplewallet too.

As far as I can tell, the expected behavior with split transactions should be clear, where the fee is split across any specified destinations in each transaction, and the entire call fails if any transaction cannot be created with subtractable destinations. So my preference is to support this.

jeffro256 · 2023-07-09T23:59:23Z

@jeffro256 Please rebase on master.

There was an issue but rebasing was fixed

woodser · 2023-07-10T14:14:58Z

@jeffro256 Do you mind to PR this on the release branch in addition to the master branch, so I can cherry-pick only what is necessary for my next release?

Otherwise I would have to base on master which has diverged from the release branch, so it's unofficial and could be risky.

jeffro256 · 2023-07-12T17:16:06Z

In the latest commit, I explicitly disabled using this feature split over multiple transactions to avoid user confusion.

src/simplewallet/simplewallet.cpp

src/wallet/wallet_rpc_server_commands_defs.h

src/wallet/wallet2.cpp

woodser · 2023-11-07T13:42:39Z

@jeffro256 mind updating the release PR too?

woodser · 2024-01-29T14:51:06Z

This pull request provides critical functionality to deduct and split the mining fee among transfer amounts. Haveno relies heavily on this feature in its trade protocol. Happy to help move this forward however possible.

jeffro256 · 2024-02-05T16:59:41Z

Rebased. Will acknowledge @vtnerd's comments soon...

vtnerd

One minor question.

src/simplewallet/simplewallet.cpp

src/wallet/wallet2.cpp

woodser · 2024-02-08T14:39:42Z

Thanks for your updates @jeffro256.

I can test it in the libraries and applications once the release PR is rebased.

woodser · 2024-02-09T17:27:55Z

I tested the latest release branch with #8945 and it's passing my tests.

Should that PR be taken out of draft mode @jeffro256?

jeffro256 · 2024-02-09T17:50:48Z

I'll keep it in draft mode as long as this PR is awaiting approval

woodser · 2024-02-12T15:56:33Z

Understood. @vtnerd approved these changes last week, so I guess we are just waiting on any other approvals.

selsta

Please bump WALLET_RPC_VERSION_MINOR

jeffro256 · 2024-02-20T23:02:26Z

Done

jeffro256 · 2024-02-20T23:07:13Z

Oh shoot you're right. I always get those mixed up

@LocalMonero

To transfer ~5 XMR to an address such that your balance drops by exactly 5 XMR, provide a `subtractfeefrom` flag to the `transfer` command. For example: transfer 76bDHojqFYiFCCYYtzTveJ8oFtmpNp3X1TgV2oKP7rHmZyFK1RvyE4r8vsJzf7SyNohMnbKT9wbcD3XUTgsZLX8LU5JBCfm 5 subtractfeefrom=all If my walet balance was exactly 30 XMR before this transaction, it will be exactly 25 XMR afterwards and the destination address will receive slightly less than 5 XMR. You can manually select which destinations fund the transaction fee and which ones do not by providing the destination index. For example: transfer 75sr8AAr... 3 74M7W4eg... 4 7AbWqDZ6... 5 subtractfeefrom=0,2 This will drop your balance by exactly 12 XMR including fees and will spread the fee cost proportionally (3:5 ratio) over destinations with addresses `75sr8AAr...` and `7AbWqDZ6...`, respectively. Disclaimer: This feature was paid for by @LocalMonero.

Looks like the logic from monero-project#8882 was accidentally removed in monero-project#8861 (regressing to the behavior noted in the monero-project#8882 description). This commit brings that logic back.

plowsof mentioned this pull request May 17, 2023

Monero Community Workgroup Meeting: Saturday 27th May 2023 @ 15:00 UTC monero-project/meta#838

Closed

jeffro256 mentioned this pull request Jun 2, 2023

wallet_rpc_server: dedup transfer RPC responses #8888

Merged

jeffro256 force-pushed the subtract_fee_from_outputs branch from e247a33 to 041a695 Compare June 13, 2023 15:55

jeffro256 mentioned this pull request Jun 16, 2023

wallet_rpc_server: dedup transfer RPC responses [RELEASE] #8892

Merged

woodser suggested changes Jul 8, 2023

View reviewed changes

jeffro256 force-pushed the subtract_fee_from_outputs branch 3 times, most recently from f1fbde1 to 6a28ed4 Compare July 9, 2023 23:58

jeffro256 force-pushed the subtract_fee_from_outputs branch from 6a28ed4 to 3cc4b57 Compare July 12, 2023 17:05

jeffro256 mentioned this pull request Jul 12, 2023

wallet: feature: transfer amount with fee included [RELEASE] #8945

Merged

vtnerd suggested changes Aug 21, 2023

View reviewed changes

jeffro256 force-pushed the subtract_fee_from_outputs branch from 3cc4b57 to b20197e Compare October 18, 2023 16:25

0xFFFC0000 added allarch wallet pending review feature important labels Dec 11, 2023

jeffro256 force-pushed the subtract_fee_from_outputs branch from b20197e to 0dc77f1 Compare February 5, 2024 16:59

vtnerd approved these changes Feb 7, 2024

View reviewed changes

src/simplewallet/simplewallet.cpp Show resolved Hide resolved

src/wallet/wallet2.cpp Outdated Show resolved Hide resolved

jeffro256 force-pushed the subtract_fee_from_outputs branch 2 times, most recently from ec65958 to 800c188 Compare February 8, 2024 19:05

selsta reviewed Feb 20, 2024

View reviewed changes

jeffro256 force-pushed the subtract_fee_from_outputs branch from 800c188 to b4eb3f1 Compare February 20, 2024 23:02

selsta approved these changes Feb 20, 2024

View reviewed changes

jeffro256 force-pushed the subtract_fee_from_outputs branch from b4eb3f1 to b13c5f6 Compare February 20, 2024 23:08

selsta approved these changes Feb 20, 2024

View reviewed changes

luigi1111 merged commit ee104bc into monero-project:master Feb 24, 2024
18 checks passed

jeffro256 deleted the subtract_fee_from_outputs branch February 24, 2024 20:23

jeffro256 mentioned this pull request Feb 24, 2024

Enforce Tx unlock_time is Zero by Relay Rule #9151

Merged

j-berman mentioned this pull request Mar 22, 2024

wallet2: ensure transfers and sweeps use same fee calc logic rnd2 #9259

Merged

0xFFFC0000 removed the pending review label Aug 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wallet: feature: transfer amount with fee included #8861

wallet: feature: transfer amount with fee included #8861

jeffro256 commented May 16, 2023 •

edited

Loading

LocalMonero commented May 16, 2023

jeffro256 commented Jun 12, 2023

woodser commented Jun 20, 2023

jeffro256 commented Jun 20, 2023

selsta commented Jun 20, 2023

woodser commented Jul 8, 2023

woodser Jul 8, 2023

jeffro256 Jul 9, 2023

woodser Jul 10, 2023 •

edited

Loading

jeffro256 commented Jul 9, 2023

woodser commented Jul 10, 2023

jeffro256 commented Jul 12, 2023

woodser commented Nov 7, 2023

woodser commented Jan 29, 2024

jeffro256 commented Feb 5, 2024

vtnerd left a comment

woodser commented Feb 8, 2024

woodser commented Feb 9, 2024

jeffro256 commented Feb 9, 2024

woodser commented Feb 12, 2024

selsta left a comment

jeffro256 commented Feb 20, 2024

jeffro256 commented Feb 20, 2024

wallet: feature: transfer amount with fee included #8861

wallet: feature: transfer amount with fee included #8861

Conversation

jeffro256 commented May 16, 2023 • edited Loading

LocalMonero commented May 16, 2023

jeffro256 commented Jun 12, 2023

woodser commented Jun 20, 2023

jeffro256 commented Jun 20, 2023

selsta commented Jun 20, 2023

woodser commented Jul 8, 2023

woodser Jul 8, 2023

Choose a reason for hiding this comment

jeffro256 Jul 9, 2023

Choose a reason for hiding this comment

woodser Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

jeffro256 commented Jul 9, 2023

woodser commented Jul 10, 2023

jeffro256 commented Jul 12, 2023

woodser commented Nov 7, 2023

woodser commented Jan 29, 2024

jeffro256 commented Feb 5, 2024

vtnerd left a comment

Choose a reason for hiding this comment

woodser commented Feb 8, 2024

woodser commented Feb 9, 2024

jeffro256 commented Feb 9, 2024

woodser commented Feb 12, 2024

selsta left a comment

Choose a reason for hiding this comment

jeffro256 commented Feb 20, 2024

jeffro256 commented Feb 20, 2024

jeffro256 commented May 16, 2023 •

edited

Loading

woodser Jul 10, 2023 •

edited

Loading