Fix Head References #4852
Fix Head References #4852
Conversation
nisdas
added
Ready For Review
nisdas
requested review from
terencechain,
prestonvanloon and
rauljordan
and removed request for
terencechain and
prestonvanloon
Codecov Report
@@ Coverage Diff @@
## master #4852 +/- ##
=========================================
Coverage ? 44.21%
=========================================
Files ? 203
Lines ? 15078
Branches ? 0
=========================================
Hits ? 6666
Misses ? 7339
Partials ? 1073 |
Co-Authored-By: shayzluf <thezluf@gmail.com>
There was a problem hiding this comment.
I need some convincing on why make(map[uint64][]byte) wouldn't work. This tracks the canonical root of the slot. Sure there can be multiple block roots for a given slot but there can only be one head root for a slot which is the point of this map, one head root for one slot because we don't need to care rest of the orphaned block roots in that slot
| @@ -5,6 +5,8 @@ import ( | |||
| "context" | |||
| "time" | |||
|
|
|||
| "github.com/prysmaticlabs/prysm/shared/bytesutil" | |||
|
|
|||
| r := [32]byte{'A'} | ||
| service.canonicalRoots[0] = r[:] | ||
|
|
||
| copy(identifier[:8], bytesutil.Bytes8(0)) |
There was a problem hiding this comment.
I think a lot of these can be refactored into its own function to reduce disparity down the line
|
What is the status of this? Was this resolved in #4869 ? |
|
Yes, that supersedes this PR. I am closing this |
Previously we were referring to the head root and head slot via a field in the blockchain service and its corresponding root in
canonical_roots. However this had a flaw, as it assumed that each slot had only 1 root. However during periods without finality and a lot of forks, each slot can have multiple blocks.This PR instead removes head slot and replaces with a head identifier, so that we can easily glean the current head slot and root from it. This also makes sure that we return the right data when forks occur.