PMMR size calculation on restart can be wrong

[ignopeverell]

On restart, we compute the size of the PMMR by checking the size of the underlying storage file. However in case of rewinds, the storage isn't truncated, which can lead to a corruption if:

• We're on branch A that extends the PMMR size by 10 (for example)
• Branch B has more work, making us switch
• Branch B extends the PMMR by only 5
• Node is restarted

[antiochp]

After our discussion - this makes sense to me now. The file has got out of sync with our rewound view of it before the restart...

[yeastplume]

Just on this, I think we should probably fix this as it may lead to very subtle and difficult-to-track errors.

What do you think the best way to approach this is? Slamming it into the PMMR file would be counterproductive, but perhaps a metadata store file beside the PMMR file that just keeps track of the expected size (for now)? We'd also need to make sure that updating this persisted size and updates/rewinds to the sum tree is an atomic transaction, so they don't get out of sync.. think this issue is slightly harder than it appears on the surface

[ignopeverell]

Yes, and there's the issue of pruning too (not activated yet). I don't think we can get perfect atomicity but we can try to limit the window with proper use of OS sync. I like the idea of the metadata file

[sesam]

Better to loose sync work on crash/restart, than have wrong data. EDITED for brevity: Storing last head/body/sync positions in a rocksdb key could be a quick and simple fix

[antiochp]

Is this something we want to tackle for testnet2?
This and #544 seem to be the two issue outstanding that can cause corrupted sumtrees and/or consensus failures due to sumtree differences.

[ignopeverell]

I've been thinking about this some more, it's actually very unlikely to occur. So definitely not as critical as #544, even though it would still be nice to have.

[yeastplume]

#735 should hopefeully address this.. will find out in testnet2 :D