wallet: use headers chain for anti fee sniping

[ghost]

No description provided.

[gmaxwell]

This still leaks that you were in IBD, but I think there isn't much that could be done about that. One possibility would be to use the best headers height which you'll have very early in sync... but it would require reasoning about some corner cases. @petertodd

[jonasschnelli]

I think taking the headers height as @gmaxwell describes would make more sense. #9483 has some simple pre-work that would be reusable (1687a0e).

[ghost]

@jonasschnelli indeed

[maflcko]

W ​asn't the original issue talking about a client that is offline in a sense of air gap (but was connected once to the network) and less about a client that is currently syncing? In which case the current solution does not help either, because the headers' height can be assumed to be identifier just as unique as the chain height. I think the best we can do for offline clients is to not set it at all.​

[sipa]

@MarcoFalke I guess we can do both. When offline (= tip header too old), don't use anti fee sniping; otherwise, use tip header information.

[keystrike]

Yes, in the initial issue I described this because of the ability to fingerprint clients which are not synchronized for some time and will not synchronize in the future. The goal is to get rid of this unique metadata. I agree with @sipa's suggestion.

Just for interest I will check the blockchain for nlocktimes which are set to past times to look at the history of this metadata leak.

[laanwj]

Needs rebase; and comments addressed.

[keystrike]

I've made a comment related to this here.

[ghost]

Rebased and awaiting review.

[petertodd]

In addition to the IsInitialChainDownload() check, it may be good to change the randomization thing to pick a random block in a much wider range, rather than just a few blocks back. Then make the IsInitialChainDownload() check also activate that randomization for all txs.

[ghost]

I'd prefer to remove the randomization thing or leave it as is.

[jonasschnelli]

The extra headers CChain object is not really required (at least not for this use case).
I would go back to your original simple implementation of anti-fee sniping and just use pindexBestHeader as the header you get your height from.

[ghost]

Removed that headers CChain.

[petertodd]

Concept NACK

W/ the various SHA256 alts that have been launched, and will be launched, it's quite possible that you have a headers chain with more blocks in it than the actual Bitcoin chain. This is even possible if that chain isn't the actual most-work chain, as you might not know about that chain.

I think re: privacy, changing the anti-fee sniping mechanism to occasionally pick nLockTime's from a much larger range is probably the better way to help the privacy problem.

[ryanofsky]

Perhaps this issue could be tagged up for grabs if it's not still active. If I understand the suggestion, it's basically just to replace 100 with a much larger value here:

bitcoin/src/wallet/wallet.cpp

Line 2668 in 22e301a

txNew.nLockTime = std::max(0, (int)txNew.nLockTime - GetRandInt(100));

[laanwj]

Closing this, seems inactive.
I also agree with @petertodd, it's risky to lock in with just the headers - especially given all kinds of forks and alts starting from the bitcoin chain.

[maflcko]

Picked up in " wallet: Avoid leaking nLockTime fingerprint when anti-fee-sniping #15039 "