Add state consistency check during witness generation #13412
Conversation
| func verifyStateConsistency(touchedPlainKeys [][]byte, stateReader state.StateReader, hph *commitment.HexPatriciaHashed) (bool, error) { | ||
| ctx := hph.GetPatriciaContext() | ||
| for _, key := range touchedPlainKeys { | ||
| if len(key) == 20 { //account address |
There was a problem hiding this comment.
No, that will never happen. The keys will either be account keys (20 bytes), or storage keys (20+32=52 bytes).
There was a problem hiding this comment.
that would happen during tests at least
| @@ -464,6 +464,55 @@ func verifyExecResult(execResult *core.EphemeralExecResult, block *types.Block) | |||
| return nil | |||
| } | |||
|
|
|||
| // verify the state consistency between the history state reader and the hph.Ctx reader | |||
| func verifyStateConsistency(touchedPlainKeys [][]byte, stateReader state.StateReader, hph *commitment.HexPatriciaHashed) (bool, error) { | |||
There was a problem hiding this comment.
add notice that this is for debug. in general ctx and state reader should have same view, so it's just comparison with another reader which may or may not share same view with hph.ctx. So when every detail with reading is cleaned out, this comparison is a self-comparison and waste of resources during witness calculation
| func verifyStateConsistency(touchedPlainKeys [][]byte, stateReader state.StateReader, hph *commitment.HexPatriciaHashed) (bool, error) { | ||
| ctx := hph.GetPatriciaContext() | ||
| for _, key := range touchedPlainKeys { | ||
| if len(key) == 20 { //account address |
There was a problem hiding this comment.
that would happen during tests at least
| // and the patricia context (hph.ctx) | ||
| // If there is any inconsistency there will inevitably be a root hash mismatch later on when | ||
| // we try to construct the witness trie | ||
| isStateConsistent, err := verifyStateConsistency(touchedPlainKeys, store.Tds.StateReader, hph) |
There was a problem hiding this comment.
id suppose to make code block like
{
// Verify that ....
...
}so it could catch an eye that this section actually is for debugging and may be omitted.
This is to prevent the witness algorithm from running if the account and storage data that will be read by
hph.ctx.Accountandhph.ctx.Storageis inconsistent with the data from the StateReader for that block, as that would result in a root hash mismatch.In general there are 2 cases where we can get a root hash mismatch:
hph.ctx.Accountandhph.ctx.Storagereturn the wrong value.Case 2 could happen because of for example stale data being read by
hph.ctxafter the unwind, or if the unwind was not applied properly, or a variety of other reasons. It's not clear what the underlying cause is and needs further investigation.