Attestation Using Head State instead of Latest State #2156
Conversation
beacon-chain/rpc/attester_server.go
Outdated
| if err != nil { | ||
| return nil, fmt.Errorf("could not tree hash beacon block: %v", err) | ||
| } | ||
| beaconState, err := as.beaconDB.State(ctx) | ||
| headState, err := as.beaconDB.HistoricalStateFromSlot(ctx, head.Slot) |
There was a problem hiding this comment.
Bug1: Suppose to use head state here instead of last processed state
| } else { | ||
| epochBoundaryRoot, err = blocks.BlockRoot(beaconState, epochStartSlot) | ||
| epochBoundaryRoot, err = blocks.BlockRoot(headState, epochStartSlot) |
There was a problem hiding this comment.
Bug2: Because of 1, it gives us an incorrect epoch boundary root
| @@ -87,9 +76,9 @@ func (as *AttesterServer) AttestationDataAtSlot(ctx context.Context, req *pb.Att | |||
| // On the server side, this is fetched by calling get_block_root(state, justified_epoch). | |||
| // If the last justified boundary slot is the same as state current slot (ex: slot 0), | |||
| // we set justified block root to an empty root. | |||
| lastJustifiedSlot := helpers.StartSlot(beaconState.JustifiedEpoch) | |||
| lastJustifiedSlot := helpers.StartSlot(headState.JustifiedEpoch) | |||
There was a problem hiding this comment.
Bug3: Because of 1, it gives us an incorrect justified slot
validator/client/validator_attest.go
Outdated
| @@ -68,6 +67,9 @@ func (v *validator) AttestToBlockHead(ctx context.Context, slot uint64) { | |||
| slot-params.BeaconConfig().GenesisSlot, err) | |||
| return | |||
| } | |||
| // Set the attestation data's slot to head_state.slot where the slot | |||
| // is the canonical head of the beacon chain. | |||
| attData.Slot = infoRes.Slot | |||
There was a problem hiding this comment.
Bug4: attestation slot should be the head slot, not the current slot
proto/beacon/rpc/v1/services.proto
Outdated
| @@ -68,6 +68,7 @@ message AttestationDataResponse { | |||
| uint64 justified_epoch = 3; | |||
| bytes justified_block_root_hash32 = 4; | |||
| ethereum.beacon.p2p.v1.Crosslink latest_crosslink = 5; | |||
| uint64 slot = 6; | |||
There was a problem hiding this comment.
Attester needs to know the head slot and it has no one of knowing unless we include head slot in one of the response
There was a problem hiding this comment.
can we call this head slot then
Codecov Report
@@ Coverage Diff @@
## master #2156 +/- ##
==========================================
- Coverage 70.17% 69.54% -0.64%
==========================================
Files 116 115 -1
Lines 9139 8851 -288
==========================================
- Hits 6413 6155 -258
+ Misses 2083 2064 -19
+ Partials 643 632 -11 |
| if err := s.dht.BootstrapWithConfig(ctx, bcfg); err != nil { | ||
| log.Errorf("Failed to bootstrap DHT: %v", err) | ||
| } | ||
| //if err := s.dht.BootstrapWithConfig(ctx, bcfg); err != nil { |
There was a problem hiding this comment.
This disables bootstrap discovery. Please remove.
beacon-chain/rpc/attester_server.go
Outdated
| beaconState, err := as.beaconDB.State(ctx) | ||
|
|
||
| // Let head state be the state of head block processed through empty slots up to assigned slot. | ||
| headState, err := as.beaconDB.HistoricalStateFromSlot(ctx, head.Slot) |
There was a problem hiding this comment.
Please use as.beaconDB.State(ctx) and have an assertion that these slots are the same.
There was a problem hiding this comment.
We also don't need the assertion, from the spec:
Let head_statebe the state ofhead_block processed through any empty slots up to the assigned slot.
What we need, what I added is this:
for headState.Slot < req.Slot {
headState, err = state.ExecuteStateTransition(
ctx, headState, nil /* block */, headRoot, state.DefaultConfig(),
)
if err != nil {
return nil, fmt.Errorf("could not execute head transition: %v", err)
}
}
proto/beacon/rpc/v1/services.proto
Outdated
| @@ -68,6 +68,7 @@ message AttestationDataResponse { | |||
| uint64 justified_epoch = 3; | |||
| bytes justified_block_root_hash32 = 4; | |||
| ethereum.beacon.p2p.v1.Crosslink latest_crosslink = 5; | |||
| uint64 slot = 6; | |||
There was a problem hiding this comment.
can we call this head slot then
This PR fixes the incorrect
AttestationDataResponsefrom attester server to validator. Attester server was using current state (last processed state) to construct these attestation but it is suppose to use head state.