Skip to content
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

[feature] Introduce command 'verify-chain' to verify the whole chain state #3609

Closed
phyro opened this issue Mar 20, 2021 · 6 comments
Closed

Comments

@phyro
Copy link
Member

phyro commented Mar 20, 2021

Is your feature request related to a problem? Please describe.
There is no way to verify the chain state from the command line. We assume the UTXO set, the set of kernels and the offset produce a valid state. In general this should be true, but as was seen in the recent events, it is possible to produce an invalid state through block processing due to the use of various optimization techniques.

Describe the solution you'd like

As a user, I would like to run ./grin verify-chain which would verify the whole state of the blockchain at that point in time.

Verification processing includes:

  1. Verification of block headers
  2. Verification of kernels
  3. Verification of the UTXO set
  4. Validity of the chain by verifying that the sum(kernels) = sum(utxo_set) + height*H + offset*G (this formula is not precise but you get the point)

It is possible I missed some elements of the chain that would need to be verified.

Describe alternatives you've considered
I think the alternative right now is to resync the node - although I'm not sure if this does such verification until the horizon point and then block validation for the last week which may include the optimizations that I would like to avoid.

Additional context
This feature may be useful for catching live processing bugs due to optimizations or other reasons e.g. an inflation bug could have been caught earlier if someone was running this command periodically for their local chain.

Note: Since this command requires a lot of computation, it would probably mean that the node would need to stop processing and propagating blocks to avoid changing the state while we verify it. An example of what we don't want is to prune the rangeproofs or outputs that are in the UTXO set at the time we started verifying the chain state. Stopping processing seems conceptually simpler and safer than labeling merkle entries as removed until the computation is finished and later running a garbage collector.

@trevyn
Copy link
Contributor

trevyn commented Mar 20, 2021

This could be a useful command for an “extended test” CI job as well.

@trevyn
Copy link
Contributor

trevyn commented Apr 2, 2021

There is a validate_chain Owner API call, I wonder what that does and how it differs from the desired behavior.

@phyro
Copy link
Member Author

phyro commented Apr 3, 2021

There is a validate_chain Owner API call, I wonder what that does and how it differs from the desired behavior.

Good question and good find. I'm not familiar with the code, but it seems that it calls this https://github.com/mimblewimble/grin/blob/master/chain/src/chain.rs#L616-L624 . The interesting part is that the fast_validation argument is set to true, which means that it skips verification of kernel sigs and and rangeproofs in this case as can be seen here https://github.com/mimblewimble/grin/blob/master/chain/src/txhashset/txhashset.rs#L1575-L1582
I think that the only call site for the fast_validation is through these API calls, so it should be ok.

My understanding is that the pipe::rewind_and_apply_fork(&header, ext, batch)?; rewinds the pmmr structures to some 'height' and then applies new blocks, but I'm not sure how this is used for validation. I wonder if calling it with fast_validation=false would be close to what I was mentioning here. From my understanding, it verifies the balance of the equation which is point 4. on my list, I think that turning off fast validation would also check kernel sigs and rangeproofs which cover 2. and 3, so it would leave only 1. out.

@antiochp
Copy link
Member

antiochp commented Apr 6, 2021

Yes validate_chain was mainly for "quick" testing during early development.
I think there used to be a way to pass fast_validation=true|false in via the api (or at least that was the intent).
I think I recall us actually changing this between true|false multiple times as various testing became more or less important.

This is not actively used currently and I see no reason why we don't want to simply default this to fast_validation=false and use it as proposed in this issue.

@antiochp
Copy link
Member

antiochp commented Apr 6, 2021

rewind_and_apply_fork is used here to ensure the chain state is in a consistent state prior to full validation.
Technically this is not required but probably no harm in doing this - its effectively a no-op assuming chain head is valid and correct with respect to the local MMR structures on disk.

@phyro
Copy link
Member Author

phyro commented Jan 7, 2022

This seems to have been resolved.

@phyro phyro closed this as completed Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants