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

Upstream merge fixes #1273

Merged
merged 1 commit into from
Nov 2, 2021
Merged

Conversation

chris-j-h
Copy link
Collaborator

@chris-j-h chris-j-h commented Oct 28, 2021

Fixes relating to recent upstream geth merges.

  • EIP-2929: Gas cost increases for state access opcodes + YoloV2 ethereum/go-ethereum#21509
    • The upstream merge introduced ActivePrecompiles() however there is a typo causing tests added in this PR to fail. This bug was fixed in upstream 1.10.2 - the content of that fix has been included in this change. To simplify future merges the content of the commit has been copied instead of cherry-picking the commit.
    • The Quorum Privacy Precompile address has been added to ActivePrecompiles() list. This is currently only used for the experimental YoloV2 EIP 2929 (where all active precompiles for the current block height must be known) so not critical but worth fixing ahead of the concrete release that will be pulled in from a future upstream geth merge.

@CLAassistant
Copy link

CLAassistant commented Oct 28, 2021

CLA assistant check
All committers have signed the CLA.

Krish1979
Krish1979 previously approved these changes Oct 28, 2021
Copy link
Collaborator

@Krish1979 Krish1979 left a comment

Choose a reason for hiding this comment

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

LGTM

@chris-j-h chris-j-h marked this pull request as ready for review October 29, 2021 09:38
Copy link
Contributor

@antonydenyer antonydenyer left a comment

Choose a reason for hiding this comment

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

I don't think cherry-picking upstream changes makes sense. We should endeavour to stay up to date with geth changes. Pulling in an additional changes that aren't part of geth v1.9.25 makes it difficult to track and maintain what is going on with quorum.

@chris-j-h
Copy link
Collaborator Author

@antonydenyer would you rather getting rid of the upstream cherry-pick and making the bug fix as part of commit a8338d8?

@antonydenyer
Copy link
Contributor

👍 Yeah let's do that for now.

This is currently only used for the experimental YoloV2 EIP 2929 (where all active precompiles for the current block height must be known) so not critical but worth fixing ahead of the concrete release that will be pulled in from a future upstream geth merge

Also includes core/vm: fix Byzantium address list (#22603) (cherry picked from commit 44fe466)
Copy link
Collaborator

@Krish1979 Krish1979 left a comment

Choose a reason for hiding this comment

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

LGTM

@antonydenyer antonydenyer merged commit 3bb2ae6 into Consensys:master Nov 2, 2021
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

5 participants