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

Westlad/fix restart nightfall #564

Merged
merged 11 commits into from
Mar 21, 2022
Merged

Westlad/fix restart nightfall #564

merged 11 commits into from
Mar 21, 2022

Conversation

Westlad
Copy link
Contributor

@Westlad Westlad commented Mar 4, 2022

fixes #375
When nightfall is stopped and restarted, its containers should re-sync with the blockchain and it should carry on working. This was buggy for two reasons, which this PR fixes:

  1. If optimist stops and loses its database, it does not know which proposers are registered with it. Therefore it will not make blocks for them when they are the current proposer. The proposers are still registered with the blockchain, however, and so should be able to make blocks when it's their turn in the round-robin. This PR adds a check to the /register endpoint. It checks whether the proposer is (a) already registered with the blockchain and (b) already registered with the optimist instance. It then registers on an as-needed basis. This is transparent to the caller.
  2. client did not sync if its commitment database had been dropped (or a new instance was started). This meant that its Timber Merkle tree was out of sync with the blockchain and it was unable to perform transfer L2 transactions.

One result of the above fix is that the ping-pong test can now be run repeatedly without resetting nightfall.

One can demonstrate the effectiveness with a ping-pong test:

  • Start and run the ping pong test as described in the readme.
  • When the test has finished bring the containers down: ./pong-down or docker-compose down (the former will drop client's DB, the latter will not). Do not do ./pong-down -v because this will remove the trusted setup and smart contract addresses., effectively starting again with a clean environment.
  • Restart nightfall with either ./pong-nightfall -s or ./pong-nightfall -d -s. The former will run up nightfall without running deployer first, thus guaranteeing no changes to the blockchain or trusted setup; the second will run deployer but deployer will work out that it has no work to do and will eventually exit without making changes. They are therefore equivalent.
  • Wait for nightfall optimist and client to finish syncing with the blockchain (maybe we should provide a syncing endpoint so we can test for this programmatically).
  • Re-run ./pong-apps. The test will succeed, whereas previously it would have failed.

@Westlad Westlad added the DNM Do not merge label Mar 10, 2022
@Westlad Westlad removed the DNM Do not merge label Mar 10, 2022
Copy link
Contributor

@druiz0992 druiz0992 left a comment

Choose a reason for hiding this comment

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

Will some of these changes be also required for a challenger registration?

@Westlad
Copy link
Contributor Author

Westlad commented Mar 14, 2022

@druiz0992 I think not because challengers don't register on-chain - but it remains to be seen.

@Westlad Westlad temporarily deployed to AWS March 16, 2022 11:47 Inactive
@Westlad Westlad temporarily deployed to AWS March 18, 2022 13:09 Inactive
@druiz0992 druiz0992 added the One more approval needed One reviewer has approved this PR but another is needed label Mar 19, 2022
@ChaitanyaKonda ChaitanyaKonda added PRIORITY 0 and removed One more approval needed One reviewer has approved this PR but another is needed labels Mar 21, 2022
@Westlad Westlad merged commit db0e90f into master Mar 21, 2022
@Westlad Westlad deleted the westlad/fix-restart-nightfall branch March 21, 2022 11:24
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.

Ping-pong test is not idempotent
5 participants