AssumeUTXO Mainnet Readiness Tracking #29616

issue fjahr openend this issue on March 10, 2024
  1. fjahr commented at 7:57 pm on March 10, 2024: contributor

    Goals of this issue

    • Track all ongoing work on assumeutxo until mainnet params are included in a release
    • Categorize the work into critical/optional for mainnet readiness
    • Conceptual discussion on assumeutxo topics that may or may not be critical for mainnet readiness

    The initial categorizations here are suggestions and changes should be discussed here in the issue. Obviously let me know if I missed anything.

    Critical Issues/PRs

    • #28553 - The mainnet params themselves
    • #29519 - Stop syncing non-snapshot chain from peers that are already connected
    • #30598 - Resolving #30514
    • #30320 - Deny loading if snapshot is not in best header chain
    • #29996 - Bugfix and additional test coverage
    • #29720 - RPC fixes for missing values, addressing #29328
    • #29612 (which fixes #25675) - Improves size of dump, critical because it is not backwards compatible
    • #29726 - Fix -reindex behavior
    • #29370 - Get rid of faked nTx and nChainTx values (closing issues discussed in #29261)
    • #30267 - Prevent issues from loading dump for invalid block

    Optional Issues/PRs

    (Attempted a rough ordering by importance)

    • #28616 (which fixes #28598) - Show background chain funds as not fully confirmed in GUI
    • #30455 - Additional wallet tests
    • #30214 - First part of #28608
    • #29993 - Improve usability
    • #28608 (draft) - State and locking cleanup
    • #29553 - dumptxoutset with height param
    • #30636
    • #30497
    • #30403 - Additional test coverage and cleanup
    • #30388 - Optimization for testing performance
    • (obsolete) #28670 - Improved error handling for loadtxoutset
    • #29478 - Additional test coverage for loadtxoutset
    • #29617 - Additional test coverage
    • #29973 - Additional test coverage
    • #29428 - Additional test coverage

    Critical conceptual discussion

    • #30288
    • Agreement on a rule of thumb which height we choose once there is consensus for mainnet readiness, i.e. how to choose the height for #28553 once we are close to an upcoming release. I think the earlier we discuss this the better, so that this doesn’t get in the way later.

    Optional conceptual discussion

    These were taken from #28553 and the original assumeutxo PR.

    • Distribution of snapshots - This recently came up here and here. There are multiple levels to this topic but I believe that this is not critical either way. If it were critical then, at the most basic level, we would probably provide an “official” torrent/download link for the dump with the release. It would be great if one or more contributors could do that (@Sjors did this in #28516 for example) and are willing to maintain this for a reasonable amount of time but it is not critical IMO because anyone can use dumptxoutset to recreate the dump and then share it any way they like. So there is no need to provide this service from within the project. I think anyone who plans to distribute the dump to other users (think of downstream projects like BTCPayServer, Raspiblitz, Umbrel, Greenlight etc.) should validate the dump like this at least once anyway. And there may even be options for distribution possible for downstream projects that we can not even think of right now but if we don’t make the feature available they can also not be explored.
    • Usage of MuHash #27669 - While it would be nice to have this I don’t think it is blocking because this can still be introduced later without invalidating existing dumps.
    • Make params configurable/not hardcoded params - I think it would be good to stay on the safe side for now and make the feature available to the public as originally planned. If the feature sees wide adoption and requests for more flexibility come in that would be the right point to explore this further.
    • Should old hashes be removed from source code at some point? - Obviously not relevant for the first mainnet params
    • How frequently should we update testnet/signet params and is there any way to deal with the possibility that these chains may get nuked at some point?
  2. Sjors commented at 1:02 pm on March 11, 2024: member

    I think “Make params configurable/not hardcoded params” should be under “critical conceptual discussion”. Not the part about flexibility, which I agree is low priority.

    What’s more fundamental is the question of whether these parameters should be “blessed” by the Bitcoin Core project.

    One could argue that there is no “blessing”, because these snapshots can be objectively assessed. A snapshot hash is valid if and only if background validation yields the same result. Any other snapshot, if it somehow makes into the code, is either a bug or an exploit.

    This makes this feature different from checkpoints in a fundamental way. A checkpoint can be abused to override the most-work valid chain. Once enough people honor it, it becomes a fait accompli because miners are incentivised to work on it - eventually making it the most-work chain. (after which it can be deleted, and people can pretend it never happened)

    The same can’t be achieved with an invalid snapshot. Such forgery can always be demonstrated as long as a single person has an archive of the blockchain all the way back to genesis.

    Still, in a far future where perhaps few users actually perform the background validation, a technically sound argument may not matter in the political reality.

    A thought experiment to clarify this. The US Treasury finally issues their one trillion dollar coin. They do so by creating their own UTXO snapshot and forcing maintainers to include it. To avoid a chain split existing nodes on the network, they don’t actually move the coin, but borrow against it. A few generations go by, more and more nodes will have be reinstalled and synced based on this new snapshot.

    In this particular scenario I don’t think it matters at all if the hash is part of chainparams or if it’s flexible. But there may be a less extreme scenario’s where it does. Which is why I think some discussion is warranted.

    Meanwhile there is a major downside to not hardcoding the parameters, in that it makes the feature much harder to use. A user would have to find some place to download it, verify a hash and a PGP signature. It’s less obvious how we could add p2p snapshot functionality later, because we have no hash to ask for and check against.

    So on balance, because of the difference with checkpoints I explained above, my current thinking is that it’s alright to include these params.

    How frequently should we update testnet/signet params

    Imo those params are only there to test the functionality. They don’t need to be fresh. If the are nuked, we can add new params a year after the new genesis block (it’s hard to test this feature if the snapshot is too close to the start of the chain).

  3. maflcko commented at 4:17 pm on March 11, 2024: member
    Not sure about your “optional” issues. The nTx violations currently may lead to a crash of the node (https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1918947945). A crash doesn’t seem “optional” to fix, no?
  4. Sjors commented at 5:12 pm on March 11, 2024: member
    That PR also gives us a clean way to drop the BLOCK_ASSUMED_VALID flag, because it was never used on a mainnet node.
  5. fjahr commented at 10:35 pm on March 12, 2024: contributor
    Moved #29370 to the critical category
  6. fjahr commented at 3:34 pm on March 13, 2024: contributor

    Thanks @Sjors , I added some thoughts inline, then deleted most of it again because it was way too convoluted :) I think the tldr for me is: You ask if including the params in the code open up any new attack vectors. And in my mind the answer is no, because if an attacker can influence the code that users are actually running, then they have unlimited options to profit from that. For the alternative, configurable params, I am not so sure that it doesn’t introduce new attack vectors and I think this hasn’t been thought through enough because there was consensus to add the code with chainparams.

    I think “Make params configurable/not hardcoded params” should be under “critical conceptual discussion”. Not the part about flexibility, which I agree is low priority.

    My thinking is that we can have that discussion afterwards when the feature sees some adoption and there is interest in not relying on the params. In my mind it would only be critical if hardcoding for now would prevent us from moving away from it in the future. But this isn’t consensus, so there is no issue with doing this :) I am happy to have such discussion now but I don’t think we need a resolution before anyone can use au on mainnet. If there is consensus for moving away from hardcoded params before we are done with the “critical” section here, then there will be PR that is added to the critical list that implements that functionality and the mainnet params PR will be removed from the list. If we merge the params first and then agree on not hardcoding them anymore, the params will simply be removed with that PR later.

    Can you say what you mean with “the part about flexibility” that is different from configurable params? I am not sure we are thinking of the same thing there.

    What’s more fundamental is the question of whether these parameters should be “blessed” by the Bitcoin Core project.

    One could argue that there is no “blessing”, because these snapshots can be objectively assessed. A snapshot hash is valid if and only if background validation yields the same result. Any other snapshot, if it somehow makes into the code, is either a bug or an exploit.

    I would call it a “temporary blessing” if anything because it’s really all about the time window before the background sync finishes. If users could wait for the background validation to complete, they would just be doing IBD. It should be kept in mind that this was always the fundamental assumption (pun intended) of the project: the parameters in the code are a blessing just like the code itself is a blessing. If you trust the developers with a bitcoin core release and the code in it you can also trust them with the included chain params.

    This makes this feature different from checkpoints in a fundamental way. A checkpoint can be abused to override the most-work valid chain. Once enough people honor it, it becomes a fait accompli because miners are incentivised to work on it - eventually making it the most-work chain. (after which it can be deleted, and people can pretend it never happened)

    The same can’t be achieved with an invalid snapshot. Such forgery can always be demonstrated as long as a single person has an archive of the blockchain all the way back to genesis.

    Still, in a far future where perhaps few users actually perform the background validation, a technically sound argument may not matter in the political reality.

    A thought experiment to clarify this. The US Treasury finally issues their one trillion dollar coin. They do so by creating their own UTXO snapshot and forcing maintainers to include it. To avoid a chain split existing nodes on the network, they don’t actually move the coin, but borrow against it. A few generations go by, more and more nodes will have be reinstalled and synced based on this new snapshot.

    If the government can force core developers to include any code they like, assumeutxo params or something else, the project is kind of lonst and people should resort to running forks instead. Creating coins out of thin air should still be easily detectable if some check the snapshot against their own node at a specific height, even if nobody has the full chain anymore. So I don’t think assumeutxo itself adds a new attack vector here.

    In this particular scenario I don’t think it matters at all if the hash is part of chainparams or if it’s flexible. But there may be a less extreme scenario’s where it does. Which is why I think some discussion is warranted.

    Meanwhile there is a major downside to not hardcoding the parameters, in that it makes the feature much harder to use. A user would have to find some place to download it, verify a hash and a PGP signature. It’s less obvious how we could add p2p snapshot functionality later, because we have no hash to ask for and check against.

    So on balance, because of the difference with checkpoints I explained above, my current thinking is that it’s alright to include these params.

    How frequently should we update testnet/signet params

    Imo those params are only there to test the functionality. They don’t need to be fresh. If the are nuked, we can add new params a year after the new genesis block (it’s hard to test this feature if the snapshot is too close to the start of the chain).

    Ok, that would work for me.

  7. luke-jr commented at 0:15 am on March 14, 2024: member
    #28598 should be in critical
  8. fjahr commented at 12:56 pm on March 14, 2024: contributor

    #28598 should be in critical

    I considered this but so far it seems to me that all other reviewers of #28616 didn’t see it as critical (myself included) or even doubted a fix was needed at all. Examples: #28616 (comment) and #28616 (comment). If there are more reviews/comments added that are in support of making it critical, or if I have missed comments in that direction from other contributors, please let me know and I will move it.

  9. Sjors commented at 12:16 pm on March 18, 2024: member

    Can you say what you mean with “the part about flexibility” that is different from configurable params? I am not sure we are thinking of the same thing there.

    Flexibility of which assumed valid block to load seems like a nice-to-have feature. I don’t consider that important to discuss before merging mainnet.

    I am happy to have such discussion now but I don’t think we need a resolution before anyone can use au on mainnet. If there is consensus for moving away from hardcoded params before we are done with the “critical” section here, then there will be PR that is added to the critical list that implements that functionality and the mainnet params PR will be removed from the list. If we merge the params first and then agree on not hardcoding them anymore, the params will simply be removed with that PR later.

    It boils down to the age old wisdom that there’s no such thing as a temporary feature.

    For the alternative, configurable params, I am not so sure that it doesn’t introduce new attack vectors and I think this hasn’t been thought through enough because there was consensus to add the code with chainparams.

    I don’t think there was much discussion on the trade-offs between these two approaches.

    I’m still inclined to think that including them is fine. As are you. But it seems “critical” that more people think this through. If nobody else chimes in with counter arguments before the v28 branch-off then I think we should just merge the mainnet params.

  10. Sjors commented at 12:26 pm on March 18, 2024: member
    @luke-jr I think it can wait until there’s an easy GUI way to load a snapshot.
  11. fjahr commented at 4:24 pm on March 18, 2024: contributor

    It boils down to the age old wisdom that there’s no such thing as a temporary feature.

    I think the usual reason for this doesn’t apply here because the feature is not changing from a user perspective. After removing the requirement for the chainparams old snapshots that were previously committed in the codebase will still work and the RPCs don’t have to change either.

    But maybe you rather mean that contributors of bitcoin core will want to add the chainparams “because that’s what we have always done”? I think we are already a bit stuck in in that regard because “the proposal has been like this for 5 years” ;) And I believe that it can actually become an easier conversation after the feature is in use in its initial form because when this is regularly used I am pretty sure that we will get requests about not having to wait for a new release every time to be able to use a new snapshot. Such requests could be much more motivating than any theoretical discussion.

    I don’t think there was much discussion on the trade-offs between these two approaches.

    Yeah, from what I remember discussions were more focussed on whether this is a good idea/secure even with the chainparams, which is why it would actually surprise me if we would get a swing of opinion on this now. But maybe I am missing something and this was considered a viable option by some back then. Maybe @jamesob could share if there was such a discussion?

    I’m still inclined to think that including them is fine. As are you. But it seems “critical” that more people think this through. If nobody else chimes in with counter arguments before the v28 branch-off then I think we should just merge the mainnet params.

    Ok, so the critical part would be to get this in front of some more people but if they just shrug it’s also fine. Do you want to open a discussion on this on delving bitcoin maybe? I think that is the right forum to get some more people’s attention for such a question right now.

  12. Sjors commented at 4:27 pm on March 18, 2024: member

    Ok, so the critical part would be to get this in front of some more people but if they just shrug it’s also fine.

    Exactly.

  13. maflcko commented at 12:52 pm on April 23, 2024: member
    Why is #29720 / #29328 not mentioned here?
  14. fjahr commented at 2:52 pm on April 23, 2024: contributor

    Why is #29720 / #29328 not mentioned here?

    Because nobody else mentioned them here so far, tagged this issue in either or notified me about them.

  15. maflcko commented at 3:02 pm on April 23, 2024: member
    Thanks for adding!
  16. Sjors commented at 11:40 am on April 29, 2024: member
    Definitely not a blocker, but quite a nuisance when testing mainnet: #29993
  17. maflcko commented at 9:14 am on June 9, 2024: member
    29996 doesn’t seem optional, but possibly a blocker, see #29996 (review)
  18. fjahr commented at 9:07 am on June 11, 2024: contributor

    29996 doesn’t seem optional, but possibly a blocker, see #29996 (comment)

    Thanks for notifying about the conversation there. So far it’s still only a test PR and not a fix but if that changes I will move it. I think the actual issue discovered there is fixed in #30267 and I will put that in blockers for now.

  19. fjahr commented at 10:27 pm on June 22, 2024: contributor
    Moved up #29996 and added #30320 and #30288
  20. fjahr commented at 7:12 pm on August 27, 2024: contributor

    While there are still a few nice improvements to review and merge, this issue has achieved its main purpose and can be closed with the merge of #28553. If someone wants to open a follow-up issue that’s great but I think as things stand now the existing open PRs are easy enough to keep track of.

    It’s also possible that we will see more new feedback come in when the feature starts to get adoption after the release of v28 but we can still open a follow-up issue that matches whatever structure we may need when the time comes.

  21. fjahr closed this on Aug 27, 2024


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-11-21 12:12 UTC

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