Set initial canonical block root #9203
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
tbenr
force-pushed
the
initial-canonical-block-root
branch
from
2684ed2 to
fca73a8
Compare
|
|
||
| @Override | ||
| public void onSlot(final UInt64 slot) { | ||
| if (!slot.mod(UInt64.valueOf(32)).equals(ONE)) { |
There was a problem hiding this comment.
Wouldn't be better to use spec.getSlotsPerEpoch(slot) instead of hardcoded 32?
There was a problem hiding this comment.
we could store slotsPerEpoch at initialisation, it wont change with current spec
| final StoreTransaction transaction = recentChainData.startStoreTransaction(); | ||
| recentChainData.getBestBlockRoot().ifPresent(transaction::setLatestCanonicalBlockRoot); | ||
| transaction | ||
| .commit() | ||
| .finish(error -> LOG.error("Failed to store latest canonical block root", error)); |
There was a problem hiding this comment.
One thought: should we compare our current canonical block root value before starting a tx? Just thinking that if it hasn't changed there is no benefit in updating it.
There was a problem hiding this comment.
if it hasn't changed we may as well just update the variable regardless, its simpler imo...
|
|
||
| applyToNodes(this::updateBestDescendantOfParent); | ||
|
|
||
| // let's peak the best descendant of the initial canonical block root |
| .map(this::getNodeByIndex) | ||
| .orElse(initialCanonicalProtoNode.get()); | ||
|
|
||
| // add a single weight to from the best descendant up to the root |
There was a problem hiding this comment.
NIT: add a single weight to ??? Maybe missing "it" or "the node"?
| private static Optional<Bytes32> getInitialCanonicalBlockRoot( | ||
| final StoreConfig config, final Optional<Bytes32> initialCanonicalBlockRoot) { | ||
| if (config.getInitialCanonicalBlockRoot().isPresent()) { | ||
| LOG.warn( |
There was a problem hiding this comment.
Why are we using WARN? Isn't this something we want to happen? It does not require intervention and is expected. I'd move it to INFO.
Signed-off-by: Paul Harris <[email protected]>
StoredLatestCanonicalBlockUpdateron everyslot%32 == 1protoArray.setInitialCanonicalBlockRootit "breaks the tie" by adding 1 to weight on all the nodes on the canonical chain identified by that block rootfixes #9198
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog