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 information form epoch header chain in epoch SNARK messages #1158

Closed
wants to merge 9 commits into from

Conversation

nategraf
Copy link
Contributor

Description

In the current implementation of the epoch SNARK messages, which are the data used to create proofs for the SNARKs within the Plumo system, it is possible to predict far in advance what these messages will be. As a result, there exists a scenario in which, if an attacker is able to compromise a number of validators, and those validators form a quorum at any future block, the attacker may create a valid proof branching the chain to a validator set of their choice. Key to this scenario is that the attacker need not have control of the validators (or more specifically their signing oracles) at the point they branch from, and instead can use the predictability of the messages to sign constructed epoch messages for blocks far in the future. As a result, the attack is feasible for an attacker who had access to the each validator in the quorum at any point in the past.

In order to address this scenario, this PR adds the epoch block hash, and previous epoch block hash to the epoch SNARK data. Because the block hash summarizes the state root, and therefore all on-chain state including the on-chain randomness values, the block hash is considered unpredictable*. This unpredictability makes the the generation of a epoch SNARK messages to fork from a future block infeasible.

  • Note that this value is not pseudo-random because anyone with the ability to, for example, submit a transaction can bias the block hash value.

Tested

WIP

Backwards compatibility

Enacting this change will require a hard-fork in the validator consensus algorithm. (WIP)

Copy link
Contributor

@kobigurk kobigurk left a comment

Choose a reason for hiding this comment

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

Concept looks great! Had some small comments and questions.

Just to clarify - the changes in the consumed libs aren't there yet, right?

consensus/istanbul/backend/backend.go Show resolved Hide resolved
consensus/istanbul/core/commit.go Outdated Show resolved Hide resolved
consensus/istanbul/core/commit.go Outdated Show resolved Hide resolved
@nategraf
Copy link
Contributor Author

@kobigurk Question related to this PR. It seems that care is taken to make the epoch index uint16 instead of uint32 or uint64. Should maxNonSigners also be compressed to uint16?

@nategraf
Copy link
Contributor Author

nategraf commented Sep 11, 2020

Here are the changes to celo-bls-go so far. celo-org/celo-bls-go#14
Here are the changes so far to celo-bls-snark-rs celo-org/celo-bls-snark-rs#183 (Currently does not build)

@nategraf nategraf marked this pull request as ready for review September 23, 2020 00:01
@kobigurk
Copy link
Contributor

kobigurk commented Dec 1, 2020

Superseded by #1231

@kobigurk kobigurk closed this Dec 1, 2020
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.

None yet

3 participants