Confirmation rule prerequisite - fork choice filter change#3431
Confirmation rule prerequisite - fork choice filter change#3431djrtwo merged 13 commits intoethereum:devfrom
Conversation
Co-authored-by: Hsiao-Wei Wang <[email protected]>
|
The change looks good to me. I don't think Bellatrix FC is affected by the change introduce in this PR. A question, would we need to add new tests along fixing the existing ones? (I guess no but asking just in case) |
djrtwo
left a comment
djrtwo
left a comment
There was a problem hiding this comment.
I'm not sure I follow -- is this to pull out a bit of functionality into specifically defined functions so that the beacon-api's can just reference these functions instead of trying to (re)define the behaviour there?
|
|
||
| # The voting source should be at the same height as the store's justified checkpoint | ||
| # The voting source should be either at the same height as the store's justified checkpoint or | ||
| # not more than two epochs ago |
There was a problem hiding this comment.
Ah, this is the substantive change
djrtwo
left a comment
djrtwo
left a comment
There was a problem hiding this comment.
I'm not sure I understand the connection to the Beacon-APIs here. can you elaborate?
saltiniroberto
changed the title
saltiniroberto
changed the title
This PR includes the changes to the Fork Choice required to ensure the safety of the confirmation rule. This means that, once this PR is implemented, the confirmation rule can be safely executed via standard Beacon API calls as done here. I have updated the title and PR description accordingly. I hope that this clarifies. |
djrtwo
left a comment
djrtwo
left a comment
There was a problem hiding this comment.
This looks good/correct.
Need to port to non-Phase0 functions as needed and then we can merge
adiasg
changed the title
Just checked and it looks like that no change is required to any of the non-Phase 0 specs. |
Co-authored-by: Mikhail Kalinin <[email protected]>
|
The PR itself looks good to me. 👍 But since it's a substantive change, we may need to be clear about when it can be on devnet/testnet/mainnet before merging it. |
…firmation-rule update test checks
To support confirmation rule via beacon-APIs as described in spec PR, add `--debug-fork-choice-version=pr3431` option and enable when Deneb fork is scheduled. To opt-out, `--debug-fork-choice-version=stable`, or don't schedule Deneb. - ethereum/consensus-specs#3431 - ethereum/consensus-specs#3466 "will bundle this with deneb release": - ethereum/pm#844 (comment)
To support confirmation rule via beacon-APIs as described in spec PR, add `--debug-fork-choice-version=pr3431` option and enable when Deneb fork is scheduled. To opt-out, `--debug-fork-choice-version=stable`, or don't schedule Deneb. - ethereum/consensus-specs#3431 - ethereum/consensus-specs#3466 "will bundle this with deneb release": - ethereum/pm#844 (comment)
…5450) To support confirmation rule via beacon-APIs as described in spec PR, add `--debug-fork-choice-version=pr3431` option and enable when Deneb fork is scheduled. To opt-out, `--debug-fork-choice-version=stable`, or don't schedule Deneb. - ethereum/consensus-specs#3431 - ethereum/consensus-specs#3466 "will bundle this with deneb release": - ethereum/pm#844 (comment)
6 participants
This PR specifies the changes to the Fork Choice specification required for the safe execution of the confirmation rule specified in #3339.
By implementing these changes, the confirmation rule can be safely run by means of beacon APIs calls, though there would be no connection to the execution layer.
See sections 3.1 to 3.4 of this document for the rationale about the changes proposed and the related security analysis.
TODO: