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

Removes 3rd signer from initalize signer set. #42

Closed
wants to merge 7 commits into from

Conversation

ckartik
Copy link

@ckartik ckartik commented Apr 26, 2024

I have confirmed the issue by initializing a new Geth instance with an alternative genesis.json file, revealing that our testnet frequently forks due to block production falling OUT_OF_TURN. This occurs because the genesis configuration includes three signers. This can be observed by executing a cast block call on our testnet, where it's noticeable that some blocks have a difficulty of 1 instead of the expected 2, indicating improper block production.

The genesis file specifies the signing set in the following structure:

0x[32 bytes of anything][20-byte address of signer * number of signers][65 bytes of 0 (typically the proposer signature, omitted in the genesis block)]
The instability can be corrected by either using an RPC call to facilitate a majority vote among proposers for the removal of a signer, or directly updating the genesis.json file to adjust the signer set. This approach is supported by comparative observations between testnet (right side in the attached image) and my custom Geth instance on Latitude with the corrected genesis file, where 2 indicates the correct block producer and 1 indicates an incorrect one.

image

Copy link

@shaspitz shaspitz left a comment

Choose a reason for hiding this comment

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

Nice catch! LGTM assuming 0xd9cd8E5DE6d55f796D980B818D350C0746C25b97 and 0x788EBABe5c3dD422Ef92Ca6714A69e2eabcE1Ee4 are the current two signers

@ckartik
Copy link
Author

ckartik commented May 1, 2024

We ended up removing both the 3rd and second so closing this PR.

@ckartik ckartik closed this May 1, 2024
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.

3 participants