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

[Testing Infra] V6 genesis recovery state test suite #1226

Open
wants to merge 19 commits into
base: v6
Choose a base branch
from

Conversation

0o-de-lally
Copy link
Collaborator

@0o-de-lally 0o-de-lally commented Jan 27, 2023

Adding functional tests for ol-genesis-tools for:

  • parsing a db backup,
  • exporting a JSON intermediary file,
  • generating a blob,
  • and finally starting a test node from that blob.
  • plus one e2e test which does the whole flow.
  • test a single validator import from json and check the genesis matches
  • * test a sample recovery file, and check the genesis matches
  • the test infrastructure is done, but the test is failing.

implement functional tests for:
- exporting db backup to json
- creating genesis blob from json
- creating genesis blob in one shot from db backup

TODO: launch test node from from genesis.blob
TODO: e2e tests from a fixture file
@0o-de-lally
Copy link
Collaborator Author

0o-de-lally commented Jan 27, 2023

@simsekgokhan
Copy link
Contributor

R4R @simsekgokhan @nelaturuk @sirouk @corynthian

LGTM

Copy link

@corynthian corynthian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpicks, other than these (maybe I missed something) but I am not seeing where the account state post initialization is compared with the state used for recovery (e.g. checking that balances are the same / state is the same as expected).

ol/genesis-tools/src/fork_genesis.rs Outdated Show resolved Hide resolved
ol/genesis-tools/src/lib.rs Outdated Show resolved Hide resolved
@0o-de-lally
Copy link
Collaborator Author

Minor nitpicks, other than these (maybe I missed something) but I am not seeing where the account state post initialization is compared with the state used for recovery (e.g. checking that balances are the same / state is the same as expected).

Good call. Yeah checking the state should be done (I have a todo: somewhere). For now I'll do a single query, since real tests of this need some better introspection tools.

The best testsuite to do that is Forge or the higher level Smoke Tests. Though I'm not entirely sure how to start a Forge instance with an arbitrary Genesis blob. Maybe a volunteer of @simsekgokhan can figure this out?

@0o-de-lally
Copy link
Collaborator Author

@corynthian I've pushed the changes you asked for including an onchain verifcation.

@simsekgokhan Please confirm that running diem-node --test actually uses all the genesis data, or only the stdlib code. I think this may not be the best way to check onchain state. See my changes to e2e_test.rs.

I think Forge, or Smoke test may be the only way to introspect this correctly. But I think some refactoring is needed to have it use a genesis.blob

@0o-de-lally 0o-de-lally changed the title V6 genesis recovery tests [Testing Infra] V6 genesis recovery state test suite Feb 7, 2023
@0o-de-lally
Copy link
Collaborator Author

@sirouk @0xzoz @hemulin I think this is ready to merge. I depend on it for the next PR #1231

@0o-de-lally
Copy link
Collaborator Author

@0xzoz can you merge?

@0xzoz
Copy link
Collaborator

0xzoz commented Feb 7, 2023

@0xzoz can you merge?

I dont have write access. It would be good to get one more approval as well. My eyes are not as well trained as others🙂

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

Successfully merging this pull request may close these issues.

4 participants