Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Even more snapshot validity checks #2935

Merged
merged 8 commits into from
Oct 28, 2016
Merged

Even more snapshot validity checks #2935

merged 8 commits into from
Oct 28, 2016

Conversation

rphmeier
Copy link
Contributor

Future work:

  • restart sync on bad ancient block.
  • sync service should find canonical block hashes which the service will use for additional verification.

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Oct 28, 2016
@arkpar
Copy link
Collaborator

arkpar commented Oct 28, 2016

Additional changes to ethash DAG caching are required. Latest DAG should be preserved and never evicted. Otherwise we'll have significant block delays when importing new blocks with background download because the latest gets evicted by ancient checks.

@rphmeier
Copy link
Contributor Author

Good point. I'll see what can be done about it. In the case of manually restarted sync we will want to clear everything though; a simple rule like "always keep the DAG with the latest era" probably won't be enough.

@rphmeier rphmeier added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 28, 2016
@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Oct 28, 2016
@rphmeier
Copy link
Contributor Author

rphmeier commented Oct 28, 2016

@gavofyork also suggested verifying total difficulty of the best block compared to what peers on the network say about it. If possible, this data should be collected before beginning the restoration of the snapshot. The manifest's block hash should also be checked against the network before beginning whenever we've got peers. Together it should be practically impossible to produce an invalid snapshot.

@arkpar init_restore(manifest, td: Option<U256>) seem ok?

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 28, 2016
@gavofyork
Copy link
Contributor

some tests would be good at some point.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants