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 TBH_ACTIVATION_EPOCH #2682

Merged
merged 3 commits into from
Oct 19, 2021
Merged

add TBH_ACTIVATION_EPOCH #2682

merged 3 commits into from
Oct 19, 2021

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented Oct 18, 2021

If you use TERMINAL_BLOCK_HASH (a block in the past) as an override for the merge, then you must have an activation epoch in the future at which point to activate this overrde. Otherwise, it would activate locally immediately for anyone that set the epoch and all nodes would be out of sync.

  • Add TBH_ACTIVATION_EPOCH
  • Ensure that if TERMINAL_BLOCK_HASH is overridden that the forkchoice ignores valid TTD terminal blocks (was a minor bug prior to this PR)

EDIT: Below was fixed by @hwwhww in ba582b3

Annoying note, I initially named this TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH (I know, a verbose name), but setup.py would then see it as two vars and build the file as config.config.TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH. I just changed the name for now to avoid this build error. BUT we generally don't use acronyms in our names so we either need to fix the build script or have a non-colliding var-name. Maybe -- TERMINAL_OVERRIDE_ACTIVATION_EPOCH

Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

@djrtwo

I fixed setup.py and restore it to TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH.

specs/merge/validator.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@mkalinin mkalinin left a comment

Choose a reason for hiding this comment

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

LGTM

specs/merge/fork-choice.md Outdated Show resolved Hide resolved
@djrtwo djrtwo merged commit def46dd into dev Oct 19, 2021
@djrtwo djrtwo deleted the TBH-epoch branch October 19, 2021 15:37
g11tech added a commit to g11tech/lodestar that referenced this pull request Nov 5, 2021
@tbenr tbenr mentioned this pull request Nov 8, 2021
25 tasks
dapplion pushed a commit to ChainSafe/lodestar that referenced this pull request Nov 15, 2021
* completing ethereum/consensus-specs#2617

* add TBH_ACTIVATION_EPOCH ethereum/consensus-specs#2682

* remove prepare payload specs 2682, add payload id as return to notify_forkchoice... 2711

* remove uniion from transaction (2683), fix gossip and tx size (2686), remove gas validation(2699)

* remove extraneous p2p condition (2687)

* params e2e test fix

* update penalty params for Merge (2698)

* updating spec version

* spec runner merge sanity and operations fixes

* removing the beacon block gossip validations as per 1.1.4

* feedback cleanup

* spec v1.1.5, fixed blockhash (2710), payloadid (2711) already changed, tbh activation check(2712) already correct

* kintsugi geth interop

* ee test fixes

* assetterminalpowblock refac and root comparision fix

* runGethPreMerge test case

* runGethPreMerge scenario with ignoring geth side ttd not reached error

* assertvalidterminalpow block fix

* tracker in comments for geth preMerge to postMerge issue

* merge transition scenario with ttd > 0 check fix deployed on geth:kintsugi-spec

* cleanup as merge-interop test file scenaros updated and working

* handling prepare payload failure scenarios

* seperating optimistic sync, fixing and testing the transaction submission/execution
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bellatrix CL+EL Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants