This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
Aura: Do not verify on state import #13058
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When we import the state, we can not fetch authorities to verify the seal etc. So, we can directly skip any verification.
bkchr
added
A0-please_review
|
Thanks for this @bkchr! Using this allowed the chain to be synced up but we still are receiving the same error around verification I seem to also be stuck on |
arkpar
approved these changes
Jan 4, 2023
|
@samelamin I think I know what the problem is. The gap sync needs to be supported as well. |
Yup, looks like its being used in the common import queue too |
|
@arkpar @andresilva I now skip any checks for when we are importing blocks in the gap. Given that we properly warp synced to the block after the gap, this should be safe? As if someone can sign fake warp sync proofs, they could also produce/sign fake blocks in between? If we would import any fake block in between, we should end up with some invalid chain. Is my reasoning okay or should we do it properly with verifying all signatures etc? |
|
FYI these changes work. I am able to produce blocks after warp sync |
|
@bkchr I think your reasoning is correct, assuming we checked a finality proof before warping to this block then it should not be possible to fool us into syncing a fake chain before the warp (you'd have to break the block hash function). We can create an issue as a nice-to-have to validate these signatures but we'd need to change Aura to keep track of authorities on the client-side (using digests) like we do for BABE, this way we wouldn't require the state to be available to figure out the authorities. |
andresilva
approved these changes
Jan 4, 2023
arkpar
reviewed
Jan 4, 2023
| // | ||
| // It is expected that the block after the gap was checked/chosen properly, e.g. by warp | ||
| // syncing to this block. | ||
| if info |
There was a problem hiding this comment.
Instead of checking for gaps specifically, just check for block.state_action.skip_execution_checks()
It returns true when all checks that involve calling into parent runtime should be skipped.
There was a problem hiding this comment.
Might as well remove the same check on line 249 below.
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
arkpar
approved these changes
Jan 4, 2023
|
bot merge |
paritytech-processbot
bot
deleted the
bkchr-aura-do-not-verify-on-state-import
branch
dandanlen
pushed a commit
to chainflip-io/substrate
that referenced
this pull request
Jan 23, 2023
* Aura: Do not verify on state import When we import the state, we can not fetch authorities to verify the seal etc. So, we can directly skip any verification. * Skip checks as well for gap sync * Update client/consensus/aura/src/import_queue.rs Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> * Review comment Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
dandanlen
pushed a commit
to chainflip-io/substrate
that referenced
this pull request
Jan 23, 2023
* Aura: Do not verify on state import When we import the state, we can not fetch authorities to verify the seal etc. So, we can directly skip any verification. * Skip checks as well for gap sync * Update client/consensus/aura/src/import_queue.rs Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> * Review comment Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
2 tasks
ltfschoen
pushed a commit
to ltfschoen/substrate
that referenced
this pull request
Feb 22, 2023
* Aura: Do not verify on state import When we import the state, we can not fetch authorities to verify the seal etc. So, we can directly skip any verification. * Skip checks as well for gap sync * Update client/consensus/aura/src/import_queue.rs Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> * Review comment Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
ark0f
pushed a commit
to gear-tech/substrate
that referenced
this pull request
Feb 27, 2023
* Aura: Do not verify on state import When we import the state, we can not fetch authorities to verify the seal etc. So, we can directly skip any verification. * Skip checks as well for gap sync * Update client/consensus/aura/src/import_queue.rs Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> * Review comment Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we import the state, we can not fetch authorities to verify the seal etc. So, we can directly skip any verification.