-
Notifications
You must be signed in to change notification settings - Fork 35.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AssumeUTXO Mainnet Readiness Tracking #29616
Comments
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.
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). |
Not sure about your "optional" issues. The nTx violations currently may lead to a crash of the node (#29261 (comment)). A crash doesn't seem "optional" to fix, no? |
That PR also gives us a clean way to drop the |
Moved #29370 to the critical category |
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.
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.
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.
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.
Ok, that would work for me. |
#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. |
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.
It boils down to the age old wisdom that there's no such thing as a temporary feature.
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. |
@luke-jr I think it can wait until there's an easy GUI way to load a snapshot. |
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.
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?
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. |
Exactly. |
Thanks for adding! |
Definitely not a blocker, but quite a nuisance when testing mainnet: #29993 |
Goals of this issue
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
dumptxoutset
serialization format (~20%) #25675) - Improves size of dump, critical because it is not backwards compatibleOptional Issues/PRs
(Attempted a rough ordering by importance)
dumptxoutset
with height paramloadtxoutset
loadtxoutset
Critical conceptual discussion
Optional conceptual discussion
These were taken from #28553 and the original assumeutxo PR.
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.The text was updated successfully, but these errors were encountered: