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 precompiles to support signature checking on provided and historic headers #772

Merged
merged 28 commits into from
Dec 23, 2019

Conversation

nategraf
Copy link
Contributor

@nategraf nategraf commented Dec 18, 2019

Description

This PR modifies the getValidatorAddress and numberValidators precompiles accept a block number to query a historic validator set, and adds the

Tested

Unit tests

Other changes

  • Change an instance of the name "infrastructure fund" to "community fund"
  • Change the unused VerifySeal function on the Istanbul consensus engine to have a more useful verification of the seal. (The previous definition was an incomplete implementation of verification and was almost certainly implemented to satisfy the consensus.Engine interface.)
  • Modified a few comments to describe more accurately the implementation.
  • Move core/evm.go to core/vm/context.go to allow code sharing with contract_comm/evm.go and testing in core/vm/contracts_test.go
  • Add helper function to calculate celo precompile addresses

Related issues

Backwards compatibility

Not backwards compatible due to a change in the validator set precompiles.

consensus/istanbul/backend/engine.go Outdated Show resolved Hide resolved
contract_comm/evm.go Outdated Show resolved Hide resolved
contract_comm/evm.go Outdated Show resolved Hide resolved
contract_comm/evm.go Outdated Show resolved Hide resolved
contract_comm/evm.go Outdated Show resolved Hide resolved
core/vm/contracts.go Show resolved Hide resolved
core/vm/contracts.go Outdated Show resolved Hide resolved
core/vm/contracts.go Outdated Show resolved Hide resolved
contract_comm/evm.go Outdated Show resolved Hide resolved
params/protocol_params.go Outdated Show resolved Hide resolved
@asaj asaj assigned nategraf and unassigned asaj Dec 19, 2019
@nategraf nategraf assigned asaj and unassigned nategraf Dec 19, 2019
@asaj asaj assigned nategraf and unassigned asaj Dec 19, 2019
Copy link
Contributor

@asaj asaj left a comment

Choose a reason for hiding this comment

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

Please update the "Other changes" section of the PR

consensus/istanbul/backend/engine.go Show resolved Hide resolved
consensus/istanbul/backend/engine.go Outdated Show resolved Hide resolved
core/vm/context.go Show resolved Hide resolved
@@ -691,7 +729,7 @@ func (c *blockNumberFromHeader) Run(input []byte, caller common.Address, evm *EV
var header types.Header
err = rlp.DecodeBytes(input, &header)
if err != nil {
return nil, gas, err
return nil, gas, ErrInputDecode
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm worried that returning a different error will mean we won't log the more informative error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's my understanding that errors returned from the EVM need to be deterministic so I wanted to avoid relaying error messages that may change in future updates.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, why do they need to be deterministic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had just assumed error messages to be included under the umbrella of EVM determinism, but maybe they aren't. If not, I'm happy to put the rlp errors back.

core/vm/contracts.go Outdated Show resolved Hide resolved
core/vm/contracts.go Outdated Show resolved Hide resolved
core/vm/contracts.go Outdated Show resolved Hide resolved
core/vm/contracts.go Outdated Show resolved Hide resolved
core/vm/contracts.go Show resolved Hide resolved
core/vm/contracts_test.go Show resolved Hide resolved
@nategraf nategraf assigned asaj and unassigned nategraf Dec 19, 2019
@asaj asaj assigned mcortesi and unassigned asaj Dec 20, 2019
@jfoutts-celo jfoutts-celo merged commit 31362b1 into master Dec 23, 2019
@mcortesi mcortesi deleted the victor/slashing-precompiles branch July 21, 2020 17:40
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