wallet: use headers chain for anti fee sniping #10040

pull ghost wants to merge 1 commits into bitcoin:master from changing 2 files +14 −2
  1. ghost commented at 9:25 pm on March 20, 2017: none
  2. gmaxwell commented at 9:31 pm on March 20, 2017: contributor
    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
  3. fanquake added the label Privacy on Mar 20, 2017
  4. fanquake added the label Wallet on Mar 20, 2017
  5. jonasschnelli commented at 7:43 am on March 22, 2017: contributor
    I think taking the headers height as @gmaxwell describes would make more sense. #9483 has some simple pre-work that would be reusable (https://github.com/bitcoin/bitcoin/pull/9483/commits/1687a0e5bc958bd204644f2d5ad3647ab461d65d).
  6. unknown renamed this:
    wallet: don't leak height of local chain during inital sync
    wallet: use headers chain for fee sniping
    on Mar 22, 2017
  7. ghost commented at 2:39 pm on March 24, 2017: none
  8. MarcoFalke commented at 2:55 pm on March 24, 2017: member

    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.​

  9. sipa commented at 8:37 pm on March 24, 2017: member
    @MarcoFalke I guess we can do both. When offline (= tip header too old), don’t use anti fee sniping; otherwise, use tip header information.
  10. unknown renamed this:
    wallet: use headers chain for fee sniping
    wallet: use headers chain for anti fee sniping
    on Mar 25, 2017
  11. keystrike commented at 9:25 am on March 28, 2017: contributor

    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.

  12. laanwj commented at 11:47 am on June 26, 2017: member
    Needs rebase; and comments addressed.
  13. keystrike commented at 12:12 pm on June 27, 2017: contributor
    I’ve made a comment related to this here.
  14. ghost commented at 3:33 pm on June 27, 2017: none
    Rebased and awaiting review.
  15. petertodd commented at 3:52 pm on June 27, 2017: contributor
    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.
  16. ghost commented at 5:28 pm on June 27, 2017: none
    I’d prefer to remove the randomization thing or leave it as is.
  17. jonasschnelli commented at 7:46 pm on August 15, 2017: contributor
    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.
  18. wallet: don't leak height of local chain if not synced
    Fixes #10020
    984e7ae2fc
  19. ghost commented at 8:46 am on August 20, 2017: none
    Removed that headers CChain.
  20. petertodd commented at 9:40 am on August 20, 2017: contributor

    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.

  21. ryanofsky commented at 9:18 pm on January 9, 2018: member

    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:

    https://github.com/bitcoin/bitcoin/blob/22e301a3d56dc9e6878380ee92c7d19ca43119d2/src/wallet/wallet.cpp#L2668

  22. MarcoFalke added the label Up for grabs on Jan 10, 2018
  23. laanwj commented at 11:00 pm on March 1, 2018: member
    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.
  24. laanwj closed this on Mar 1, 2018

  25. MarcoFalke removed the label Up for grabs on Mar 5, 2019
  26. MarcoFalke commented at 7:36 pm on March 5, 2019: member
    Picked up in " wallet: Avoid leaking nLockTime fingerprint when anti-fee-sniping #15039 "
  27. DrahtBot locked this on Dec 16, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-01 10:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me