Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Replay skip checks should respect --force-all-checks flag #3336

Closed
arhag opened this issue May 23, 2018 · 5 comments
Closed

Replay skip checks should respect --force-all-checks flag #3336

arhag opened this issue May 23, 2018 · 5 comments

Comments

@arhag
Copy link
Contributor

arhag commented May 23, 2018

By default, replay should be able to skip various checks (block signatures, Merkle root verification, authorization checks, etc.) on blocks it has already confirmed and saved to disk.

The --force-all-checks flag can be passed into nodeos to tell it to not skip any of these checks during replay.

This can be useful if, for example, a node has downloaded a "blocks.log" snapshot file from the internet (as a faster alternative to synchronizing from the p2p network) and wants to use it to quickly catch up with the network. Because these blocks have not already been confirmed by software the node operator trusts, it would be unsafe to replay that block log with the default of some checks skip. So, the node operator would want to run nodeos with --replay --force-all-checks the first time to make sure the blocks are good. After that first check, further replays of the persisted block log on that node could leave off the --force-all-checks to speed up replay.

Currently only the authorization check skip respects the --force-all-checks flag. Other check skips (currently I think the only other one implemented is skipping the block signature check) need to be updated to also respect that flag.

@arhag arhag added this to the Verison 1.1 milestone May 23, 2018
@dhaneshvb dhaneshvb self-assigned this Jun 5, 2018
@dhaneshvb
Copy link
Contributor

@arhag , please correct me if I am wrong.
I could not find any checks for merkle root a at present both in replay mode and in regular mode. If I am not wrong, how it works is, if the merkle root doesnt match, it could end up failing on signature check.

Hence, if we disable the signature check, isnt that implicitly disabling the merkle checks as well?

dhaneshvb added a commit to dhaneshvb/eos that referenced this issue Jun 7, 2018
@arhag
Copy link
Contributor Author

arhag commented Jun 7, 2018

@dhaneshvb, that appears to be correct.

We could add additional earlier checks on the Merkle roots (as I recently did with transaction receipts in PR #3921), to provide more meaningful error messages than the infamous signature check assertion failure message. And those additional checks shouldn't be consensus-breaking changes. But as of now, disabling signature checks also disables a bunch of checks including Merkle root checks.

dhaneshvb added a commit to dhaneshvb/eos that referenced this issue Jun 14, 2018
dhaneshvb added a commit to dhaneshvb/eos that referenced this issue Jun 14, 2018
dhaneshvb added a commit to dhaneshvb/eos that referenced this issue Jun 14, 2018
@brianjohnson5972
Copy link
Contributor

@arhag is this still a problem we need to address?

@arhag
Copy link
Contributor Author

arhag commented Nov 11, 2018

@brianjohnson5972:

This appears to still be relevant since, for example, skip_trx_checks() doesn't respect --force-all-checks.

@aclark-b1
Copy link

In order to focus our efforts on issues that are currently creating difficulty for the community we are closing tickets that were created prior to the EOSIO 2.0 release. If you believe this issue is still relevant please feel free to reopen it or create a new one. Thank you for your continued support of EOSIO!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants