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

Add integration tests for syncing a verifier #604

Closed
karlfloersch opened this issue Apr 23, 2021 · 1 comment
Closed

Add integration tests for syncing a verifier #604

karlfloersch opened this issue Apr 23, 2021 · 1 comment
Assignees
Labels
C-feature Category: features

Comments

@karlfloersch
Copy link
Contributor

Is your feature request related to a problem? Please describe.
We currently do not have test coverage for syncing a verifier. This is a pretty big hole! To my knowledge, we would not be able to detect an error in the transaction batch submitter that causes verifiers to sync incorrectly. For instance, I don't think we'd detect it if the batch submitter submitted junk data for all transactions in our integration tests -- that's not good! Or if there was an error in the DTL or Geth, we similarly would not detect it!

Describe the solution you'd like
One quick way to solve this problem is by adding sync tests. These tests would behave in the following way:

  1. Perform a few transactions on L2 with an L2 sequencer.
  2. Wait until batches are submitted
  3. Record the final state root in the sequencer.
  4. Spin up a verifier which syncs the chain that was recently created
  5. Check the final state root in the sequencer vs the verifier, ensure equality.

Adding these tests would allow us to catch a lot of common-sense errors in both the batch submitter and Geth's sync service. Long term we can have sync tests which sync mainnet & compare state roots, but even syncing a few blocks will catch a wide class of errors.

Note on adding a fork detection service

Note that this relates to a service that we should have but currently do not: the fork detection service. The fork detector service is a simple node script which takes a set of web3 URLs & repeatedly queries them to verify that they all have the same state root. It can then be queried to check the status of the network as: in consensus or forked. It should list out all of the head state state roots for all of the nodes that it is polling and if the network is forked it should be easy to spot which nodes are in the majority vs minority. Theoretically, this service can be used in the sync tests, but that might be overkill.

@snario snario changed the title Add simple sync test to integration-tests Add integration tests for syncing a verifier May 4, 2021
@karlfloersch
Copy link
Contributor Author

karlfloersch commented May 9, 2021

Got a report of someone syncing, it stopping before it got to the tip, and then it was accepting transactions. Need to document / create a repro case for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature Category: features
Projects
None yet
Development

No branches or pull requests

2 participants