docs: Document that 'fee' is unavailable when there are non-wallet inputs

shesek commented at 1:56 am on May 18, 2020: contributor

docs: Document that 'fee' is unavailable when there are non-wallet inputs 917f852496

fanquake added the label Docs on May 18, 2020

luke-jr commented at 2:29 am on May 18, 2020: member

It shouldn’t be on non-“send” transactions at all, no matter what we know of their inputs…?

shesek commented at 3:05 am on May 18, 2020: contributor

It would be great if bitcoind reported the fee of unconfirmed non-send transactions (possible because all the spent outputs are still available). Even better if it kept the fee in the wallet transaction db and made it permanently available.

But this PR only meant to correct the documentation according to the current behavior. Perhaps a new issue should be opened for improving it? (since I won’t be able to contribute much to making it happen, I didn’t feel comfortable opening one)

luke-jr commented at 3:56 am on May 18, 2020: member

The current “fee” field exists to indicate an effect on your wallet. The fee of incoming transactions has no such effect.

Furthermore, the fee on incoming transactions isn’t by itself relevant to the recipient at all: what you care about is the best feerate, including descendants. But that seems like a feature request for a new field.

shesek commented at 4:53 am on May 18, 2020: contributor

I misunderstood your previous comment, I thought you were suggesting that it should include the fee if we know the enough about the inputs to determine it. I now understand that you meant the exact opposite :)

Just to clarify: this PR is about “send” transactions with inputs that are external to the wallet (i.e. coinjoined), not about supporting non-“send” transactions.

I do agree with your point, looking at just the direct fee on its own without taking the feerate of unconfirmed parents and descendants into account isn’t very useful. I found myself in the position of needing to report the fee for compatibility with an existing protocol, but now I’m re-considering if that’s the right thing to do - perhaps its better to show no fee than a misleading one.

(This could even potentially be abused by making a transaction paying just the relay fee, then a descendant transaction with a high feerate paying to a victim to make him mistakenly believe that its likely to get confirmed soon. But thankfully no one is accepting unconfirmed payments, right? :stuck_out_tongue:)

sipa commented at 6:23 am on May 18, 2020: member

I agree that if we report fee for not purely from-us transactions, it should be a different field. Right the invariant holds that the sum of amount and fee fields is the effect of the transaction on the wallet’s balance.

Though if it’s just for unconfirmed transactions, getmempoolentry may be what you want?

MarcoFalke commented at 11:33 am on May 18, 2020: member

The current “fee” field exists to indicate an effect on your wallet.

This is not documented nor tested in that way and users generally assume this is a bug instead of a feature:

shesek commented at 1:06 pm on May 18, 2020: contributor

@MarcoFalke Hmm, so its not “unavailable” like I thought and not just a matter of fixing the documentation, its actually reported wrong. An initial fix could be to do what’s described here, but I guess its best to close this PR for now until this gets implemented? The documentation can be updated alongside the fix.

luke-jr commented at 2:42 pm on May 18, 2020: member

Bitcoin Core does not support CoinJoins, and if CoinJoins are generated externally, it is impossible for Core to track them correctly (it’s as blind to the metadata as third parties are).

To fix that would require some kind of metadata telling Core which outputs are actually ours, and the CoinJoin software to report that information to Core.

MarcoFalke commented at 2:53 pm on May 18, 2020: member

The wallet will happily add coinjoins to the database. If this is not supported, at the very least it should be documented as such.

DrahtBot commented at 3:15 am on November 24, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#22798 (doc: Fix RPC result documentation by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot commented at 8:32 am on September 23, 2021: member

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

DrahtBot added the label Needs rebase on Sep 23, 2021

DrahtBot commented at 12:29 pm on December 22, 2021: member

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

MarcoFalke commented at 10:24 am on February 22, 2022: member

Closing for now per previous comment, can be reopened or a new pull can be created any time.

MarcoFalke closed this on Feb 22, 2022

DrahtBot locked this on Feb 22, 2023

docs: Document that ‘fee’ is unavailable when there are non-wallet inputs #19002

Conflicts