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

Convert YoloV2 flags to EIP-2315 and EIP-2929 #246

Merged

Conversation

ziogaschr
Copy link
Member

No description provided.

params/config.go Outdated
@@ -30,8 +30,7 @@ var (
RopstenGenesisHash = common.HexToHash("0x41941023680923e0fe4d74a34bdac8141f2540e3ae90623718e47d66d1ca4a2d")
RinkebyGenesisHash = common.HexToHash("0x6341fd3daf94b748c72ced5a5b26028f2474f5f00d824504e4fa37a75767e177")
GoerliGenesisHash = common.HexToHash("0xbf7e331f7f7c1dd2e05159666b3bf8bc7a8a3a9eb1d518969eab529dd9b88c1a")
// TODO: update with yolov2 values
YoloV2GenesisHash = common.HexToHash("0x498a7239036dd2cd09e2bb8a80922b78632017958c332b42044c250d603a8a3e")
YoloV2GenesisHash = common.HexToHash("0xc3fd235071f24f93865b0850bd2a2119b30f7224d18a0e34c7bbf549ad7e3d36") // TODO: check if yolov2 is correct
Copy link
Member Author

Choose a reason for hiding this comment

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

At go-ethereum they left this on yolov1 until they get the new value. Though we have a testcase that fails. So I temporarily used the output of the test to make it happy

Copy link
Contributor

@holiman holiman Dec 4, 2020

Choose a reason for hiding this comment

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

Not really, we just chose to use the exact same genesis (alloc + time but not chainId) for yolov2 as yolov1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @holiman for jumping in, this clears up everything. I will fix anything needed to make the test pass then.

EIP2537Transition *ParityU64 `json:"eip2537Transition,omitempty"`
EIP1706Transition *ParityU64 `json:"-"` // FIXME, when and if i'm implemented in Parity
EIP2929Transition *ParityU64 `json:"-"` // FIXME, when and if i'm implemented in Parity
Copy link
Member Author

@ziogaschr ziogaschr Dec 4, 2020

Choose a reason for hiding this comment

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

We have a test case on forks (TestEquivalent_Features) that fails on this transition. The thing is that OpenEthereum is not going to implement this EIP as they are out of resources at this moment.

What is the best way to handle this test case @meowsbits?

Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing func (spec *ParityChainSpec) SetEIP2929Transition(n *uint64) error { needs to actually set EIP2929Transition, in the same way as EIP2315 is handled for Parity.

Copy link
Member

Choose a reason for hiding this comment

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

Resolved with f3aff0e

Copy link
Member Author

@ziogaschr ziogaschr Dec 4, 2020

Choose a reason for hiding this comment

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

This resolves our test but I though of leaving it unsupported based on the comment here saying that they are not going to implement this.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, roger that. I've reverted that commit and just modified the tests to skip Parity :: [Berlin|YOLO] tests. See 186e3cb and subsequent.

meowsbits and others added 2 commits December 4, 2020 06:52
Fixes ./integration/#TestEquivalent_Features

Resolves https://github.com/etclabscore/core-geth/pull/246/files#r535740801

Date: 2020-12-04 06:52:01-06:00
Signed-off-by: meows <b5c6@protonmail.com>
…forks

EIP2929 is and will be unsupported by Parity, so
we cannot test chain config feature equivalence for
forks that demand that feature. Skip those tests.

Date: 2020-12-04 07:45:40-06:00
Signed-off-by: meows <b5c6@protonmail.com>
Date: 2020-12-04 07:57:11-06:00
Signed-off-by: meows <b5c6@protonmail.com>
@meowsbits meowsbits merged commit 760dd39 into merge/foundation-master-20201111 Dec 4, 2020
@meowsbits meowsbits deleted the feat/support-eips-2315-2929 branch December 4, 2020 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants