rpc: rollback about bitcoin

[maflcko]

Not sure about adding a heavy and complicated feature such as this to real code, when this feature will only be used in tests. AU will likely use #29553, IIUC?

[Sjors]

My suggestion in #29553 was to use a proper rollback mechanism (for which you might as well add an RPC call). Right now that PR effectively uses invalidateblock and reconsiderblock internally, as the shell scripts do, which is brittle.

[maflcko]

Deleting block files and headers from storage (and memory) seems more brittle and unclean than, let's say, simply rolling back a view of the utxo set (similar to validatechain). I guess this requires RAM (not sure how easy it would be to change), but otherwise this seems cleaner.

[Sjors]

@maflcko maybe "clean" in terms of code, but not in terms of behaviour. Our validation and p2p code behaves different when blocks are stored or headers are known. We also disconnect peers that send us "invalid" blocks. This is why the UTXO snapshot generation script has to turn off the network. But if anything goes wrong, you might end up with blocks marked invalid.

If we want e.g. 50 people to generate and attest their own snapshot, it seems quite likely that at least some of them will kill the node halfway because it takes too long. It seems safer to me if the node can recover from that with regular IBD behavior, and we don't have to tell users to call getchaintips and call reconsiderblock on various blocks.

Also, the only way at the moment to test if a given snapshot is valid, is to start a new node. With a rollback RPC you can test it on an existing node.

I'd like to see if the code for such an RPC is actually messy and complicated. Perhaps it can be done in a straight-forward way. I haven't tried (yet).

[sipa]

Could this be accomplished using a (test-only) RPC that marks block data of an inactive block deleted, and/or one that deletes the block index entry of an inactive block?

Then you could use invalidateblock + deleteblockdata to simulate the behavior you want?

Because internally, that's how I'd imagine the functionality you're asking for anyway.

[maflcko]

Then you could use invalidateblock + deleteblockdata to simulate the behavior you want?

I don't think this works, in the unlikely case, as explained by sjors, where a stale block (a sibling of the invalidated block) happens to exist and is activated.

[maflcko]

@maflcko maybe "clean" in terms of code, but not in terms of behaviour. Our validation and p2p code behaves different when blocks are stored or headers are known. We also disconnect peers that send us "invalid" blocks. This is why the UTXO snapshot generation script has to turn off the network. But if anything goes wrong, you might end up with blocks marked invalid.

Why would it be cleaner in terms of behavior to delete blocks when rolling back the utxo set? And the p2p issues you mention would remain present with your suggested solution, unless you also disable the network.

[Sjors]

It would still be better to disable the network during the rollback, because otherwise you'd immediately get the headers again and start downloading. We would not disconnect peers, though they might replace us for not having recent stuff (or for not delivering blocks at a height that we previously announced).

Why would it be cleaner in terms of behavior to delete blocks when rolling back the utxo set?

For the use case of assume utxo, there might be alternative ways to roll back the UTXO set. Doing it purely in memory (as a cache) - as you suggested above - would have the advantage of making it safe and quick to abort.

For the use case of p2p test code (compact blocks, modified blocks, header spam, etc), you need to delete blocks and headers.

I don't know a priori which approach is easier to implement.

in the unlikely case

I agree it seems unlikely on mainnet, unless someone does it on purpose to frustrate the process. That could happen if we were to pick a predictable interval. That could be useful if we share (partial) snapshots via the p2p network, where nodes know in advance at which height the next snapshot will be.

[maflcko]

For the use case of p2p test code (compact blocks, modified blocks, header spam, etc), you need to delete blocks and headers.

For p2p tests, my preference would to just use what has been used previously: Spin up a node and sync the blocks/headers from scratch, when invalidateblock doesn't work. There needs to be a substantial benefit, to warrant writing a complex, new, test-only RPC for an alternative solution to an already solved edge case.