[EIP-7917] Add deterministic proposer lookahead changes to Fulu #9501
Conversation
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
…tle different as we cna't get the slot from the state Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
# Conflicts: # ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
# Conflicts: # ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/versions/fulu/helpers/MiscHelpersFulu.java
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
|
worth noting that on FULU, priming the proposerIndex doesn't make sense anymore (we don't use that transition cache anymore) |
good point, to be fair, but just calling that method is super quick now anyways that probably not even worth to put an if statement in my opinion |
tbenr
left a comment
tbenr
left a comment
There was a problem hiding this comment.
I think we can YOLO this.
Just a comment
| public int getBeaconProposerIndex(final BeaconState state, final UInt64 requestedSlot) { | ||
| final int lookaheadIndex = requestedSlot.mod(configFulu.getSlotsPerEpoch()).intValue(); | ||
| return BeaconStateFulu.required(state) | ||
| .getProposerLookahead() | ||
| .get(lookaheadIndex) | ||
| .get() | ||
| .intValue(); | ||
| } | ||
|
|
||
| public List<Integer> getBeaconProposerIndices(final BeaconState state, final UInt64 epoch) { | ||
| final IntList indices = getActiveValidatorIndices(state, epoch); | ||
| final Bytes32 seed = getSeed(state, epoch, Domain.BEACON_PROPOSER); | ||
| return miscHelpers.computeProposerIndices(state, epoch, seed, indices); | ||
| } |
There was a problem hiding this comment.
this is maybe a thing for spec, but I don't like the method names here. It should be clearer which method lookups the state and which computes the indices.
So i guess getBeaconProposerIndices should be computeBeaconProposerIndices too
There was a problem hiding this comment.
this has been automerged, lol
just want to make sure @StefanBratanov and @Gabriel-Trintinalia see this comment
There was a problem hiding this comment.
Hi @tbenr ! EIP and spec coauthor here.
but I don't like the method names here. It should be clearer which method lookups the state and which computes the indices.
So i guess getBeaconProposerIndices should be computeBeaconProposerIndices too
Actually if you look at current specs some of these getX functions are already just wrappers around the corresponding computeX functions. For example, get_beacon_committee calls compute_committee (code) and pre-Fulu get_beacon_proposer_index (code) calls compute_proposer_index. So while I do understand your point that "getters" doing computation can be misleading, I would say it is already established pattern that exists in the specs.
There was a problem hiding this comment.
@linoscope I see what you mean. I was focused on specific changes. Widening the look that's consistent with the existing patterns. all good.
5 participants
PR Description
Implementation of ethereum/consensus-specs#4190
Fixed Issue(s)
fixes #9378
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog