-
Notifications
You must be signed in to change notification settings - Fork 5
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
Verifiable Consensus Data #40
base: main
Are you sure you want to change the base?
Verifiable Consensus Data #40
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!!
sips/verifiable_consensus_data.md
Outdated
// addition | ||
if bytes.Equal(attestationDataByts, data) { | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nil
or empty check right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the attestation data we got from our own node is the same as the leader's (it should happen if the nodes are synced to the same head) then no need to perform any more validation and we stop
This assumes the operator trusts his beacon node (most of the time they run their own I think).
If the data
is nil
it should fail I think when we do deocode
or validate
sips/verifiable_consensus_data.md
Outdated
} | ||
blockEpoch := getBeaconNode().getEpochBySlot(proof.Message.slot) | ||
domain := GetBeaconNode().DomainData(blockEpoch, spec.DomainProposer) | ||
root := spec.ComputeEthSigningRoot(beaconRoot, domain) | ||
validator := GetBeaconNode.GetValidator("HEAD", proof.Message.ProposerIndex) | ||
return bls.Verify(sig, validator.PubKey, root) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this check, we're sure that the block proposed as head exists. But we, still, can't be sure if a malicious operator is sending us an old block to attest, correct?
Perhaps, if we had some state that persisted through all duties, we could do a lower limit check for the slot/epoch. This is some future discussion but what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can compare against the block we got from our node simply without persisting state.
But I see it as a future improvement outside this SIP
I need to add a check that the beaconHeaderProof is assigned by a validator that was assigned the duty |
Why can't a local node request the block from the local beacon node and see it exists? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't a local node request the block from the local beacon node and see it exists?
it can but the block may be missing there. We first check the local node and then the proof
func isSourceJustified(attestationData)) error { | ||
currentEpoch := network.EstimatedCurrentEpoch() | ||
// https://ethereum.github.io/beacon-APIs/#/Beacon/getStateFinalityCheckpoints | ||
checkpoints := getBeaconNode().getFinalityCheckpoints() | ||
if attestationData.Target.Epoch == currentEpoch { | ||
if attestationData.Source.Root != checkpoints.data.currentJustifiedRoot { | ||
return Errors.New("source is not expected currently justified root") | ||
} | ||
} else if attestationData.Source.Root!= checkpoints.data.previousJustifiedRoot { | ||
return Errors.New("source is not expected previously justified root") | ||
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify that attestation/sync-committee consensus data will be included by beacon chain