Skip to content
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

Implement support for validator next-epoch proposer duties #3782

Merged
merged 29 commits into from
May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9a0a792
Implementation to be able to get block proposer an epoch ahead - stil…
dadepo Feb 21, 2022
5a1426a
revert changes made to waitForSlot
dadepo Feb 21, 2022
bcdfbb6
caching the results of computing future proposers. Also extended test
dadepo Feb 21, 2022
4846918
using effectiveBalanceIncrements from state instead of recomputing it
dadepo Feb 24, 2022
18cde25
fix lint errors
dadepo Feb 25, 2022
9bee893
revert check not needed in getBeaconProposer
dadepo Feb 25, 2022
5a632b1
Update tests to include assertion messages
dadepo Feb 25, 2022
0a1aba3
Move caching of next proposer duties to BeaconChain class
dadepo Feb 28, 2022
ac83ab8
Delete the block proposer previously cached when next proposer was re…
dadepo Mar 1, 2022
fafca52
moved next epoch proposers from the chain to the state
dadepo Mar 7, 2022
7d08ada
Compute next proposer on demand and cache
dadepo Mar 14, 2022
f0a9c3f
Fix lint errors
dadepo Mar 15, 2022
520c749
Merged in master and fix conflicts.
dadepo Apr 26, 2022
d54e124
update implementation to work with changes from master
dadepo Apr 26, 2022
02a722a
caching epoch seed in context so that getNextEpochBeaconProposer can …
dadepo Apr 29, 2022
b7e526a
Revert "caching epoch seed in context so that getNextEpochBeaconPropo…
dadepo Apr 29, 2022
72fd6ea
caching epoch seed in context so that getNextEpochBeaconProposer can …
dadepo Apr 29, 2022
12aa60d
removing the need to delete from nextEpochProposers in call to getBea…
dadepo Apr 29, 2022
b6b1b8c
no need to recompute currrentProposerSeed again
dadepo May 2, 2022
c75d926
Revert "no need to recompute currrentProposerSeed again"
dadepo May 2, 2022
3c252a7
removed empty file left after fixing merge conflicts
dadepo May 5, 2022
162a66b
remove some unnecessary variable from the epoch context.
dadepo May 10, 2022
8b58292
add some comments
dadepo May 12, 2022
99e973f
Fix lint
dadepo May 12, 2022
be65566
import from the right location
dadepo May 13, 2022
ab35439
Review PR
dapplion May 16, 2022
1ec1ccb
Merge imports
dapplion May 16, 2022
736cef7
Delete get proposers api impl test
dapplion May 16, 2022
9a51aa4
Remove duplicated comment
dapplion May 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 87 additions & 3 deletions packages/beacon-state-transition/src/cache/epochContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {BLSSignature, CommitteeIndex, Epoch, Slot, ValidatorIndex, phase0, SyncP
import {createIBeaconConfig, IBeaconConfig, IChainConfig} from "@chainsafe/lodestar-config";
import {
ATTESTATION_SUBNET_COUNT,
DOMAIN_BEACON_PROPOSER,
EFFECTIVE_BALANCE_INCREMENT,
FAR_FUTURE_EPOCH,
GENESIS_EPOCH,
Expand All @@ -18,8 +19,9 @@ import {
getChurnLimit,
isActiveValidator,
isAggregatorFromCommitteeLength,
computeProposers,
computeSyncPeriodAtEpoch,
getSeed,
computeProposers,
} from "../util";
import {computeEpochShuffling, IEpochShuffling} from "../util/epochShuffling";
import {EffectiveBalanceIncrements, getEffectiveBalanceIncrementsWithLen} from "./effectiveBalanceIncrements";
Expand Down Expand Up @@ -47,6 +49,9 @@ export type EpochContextOpts = {
skipSyncPubkeys?: boolean;
};

/** Defers computing proposers by persisting only the seed, and dropping it once indexes are computed */
type ProposersDeferred = {computed: false; seed: Uint8Array} | {computed: true; indexes: ValidatorIndex[]};

/**
* EpochContext is the parent object of:
* - Any data-structures not part of the spec'ed BeaconState
Expand Down Expand Up @@ -95,6 +100,14 @@ export class EpochContext {
* 32 x Number
*/
proposers: ValidatorIndex[];

/**
* The next proposer seed is only used in the getBeaconProposersNextEpoch call. It cannot be moved into
* getBeaconProposersNextEpoch because it needs state as input and all data needed by getBeaconProposersNextEpoch
* should be in the epoch context.
*/
proposersNextEpoch: ProposersDeferred;

/**
* Shuffling of validator indexes. Immutable through the epoch, then it's replaced entirely.
* Note: Per spec definition, shuffling will always be defined. They are never called before loadState()
Expand Down Expand Up @@ -155,6 +168,7 @@ export class EpochContext {
pubkey2index: PubkeyIndexMap;
index2pubkey: Index2PubkeyCache;
proposers: number[];
proposersNextEpoch: ProposersDeferred;
previousShuffling: IEpochShuffling;
currentShuffling: IEpochShuffling;
nextShuffling: IEpochShuffling;
Expand All @@ -175,6 +189,7 @@ export class EpochContext {
this.pubkey2index = data.pubkey2index;
this.index2pubkey = data.index2pubkey;
this.proposers = data.proposers;
this.proposersNextEpoch = data.proposersNextEpoch;
this.previousShuffling = data.previousShuffling;
this.currentShuffling = data.currentShuffling;
this.nextShuffling = data.nextShuffling;
Expand Down Expand Up @@ -269,9 +284,18 @@ export class EpochContext {
: computeEpochShuffling(state, previousActiveIndices, previousEpoch);
const nextShuffling = computeEpochShuffling(state, nextActiveIndices, nextEpoch);

const currentProposerSeed = getSeed(state, currentEpoch, DOMAIN_BEACON_PROPOSER);

// Allow to create CachedBeaconState for empty states
const proposers =
state.validators.length > 0 ? computeProposers(state, currentShuffling, effectiveBalanceIncrements) : [];
state.validators.length > 0
? computeProposers(currentProposerSeed, currentShuffling, effectiveBalanceIncrements)
: [];

const proposersNextEpoch: ProposersDeferred = {
computed: false,
seed: getSeed(state, nextEpoch, DOMAIN_BEACON_PROPOSER),
};

// Only after altair, compute the indices of the current sync committee
const afterAltairFork = currentEpoch >= config.ALTAIR_FORK_EPOCH;
Expand Down Expand Up @@ -319,6 +343,7 @@ export class EpochContext {
pubkey2index,
index2pubkey,
proposers,
proposersNextEpoch,
previousShuffling,
currentShuffling,
nextShuffling,
Expand Down Expand Up @@ -351,6 +376,7 @@ export class EpochContext {
index2pubkey: this.index2pubkey,
// Immutable data
proposers: this.proposers,
proposersNextEpoch: this.proposersNextEpoch,
previousShuffling: this.previousShuffling,
currentShuffling: this.currentShuffling,
nextShuffling: this.nextShuffling,
Expand Down Expand Up @@ -389,7 +415,11 @@ export class EpochContext {
const nextEpoch = currEpoch + 1;

this.nextShuffling = computeEpochShuffling(state, epochProcess.nextEpochShufflingActiveValidatorIndices, nextEpoch);
this.proposers = computeProposers(state, this.currentShuffling, this.effectiveBalanceIncrements);
const currentProposerSeed = getSeed(state, this.currentShuffling.epoch, DOMAIN_BEACON_PROPOSER);
this.proposers = computeProposers(currentProposerSeed, this.currentShuffling, this.effectiveBalanceIncrements);

// Only pre-compute the seed since it's very cheap. Do the expensive computeProposers() call only on demand.
this.proposersNextEpoch = {computed: false, seed: getSeed(state, this.nextShuffling.epoch, DOMAIN_BEACON_PROPOSER)};

// TODO: DEDUPLICATE from createEpochContext
//
Expand Down Expand Up @@ -477,6 +507,60 @@ export class EpochContext {
return this.proposers[slot % SLOTS_PER_EPOCH];
}

getBeaconProposers(): ValidatorIndex[] {
return this.proposers;
}

/**
* We allow requesting proposal duties 1 epoch in the future as in normal network conditions it's possible to predict
* the correct shuffling with high probability. While knowing the proposers in advance is not useful for consensus,
* users want to know it to plan manteinance and avoid missing block proposals.
*
* **How to predict future proposers**
*
* Proposer duties for epoch N are guaranteed to be known at epoch N. Proposer duties depend exclusively on:
* 1. seed (from randao_mix): known 2 epochs ahead
* 2. active validator set: known 4 epochs ahead
* 3. effective balance: not known ahead
*
* ```python
* def get_beacon_proposer_index(state: BeaconState) -> ValidatorIndex:
* epoch = get_current_epoch(state)
* seed = hash(get_seed(state, epoch, DOMAIN_BEACON_PROPOSER) + uint_to_bytes(state.slot))
* indices = get_active_validator_indices(state, epoch)
* return compute_proposer_index(state, indices, seed)
* ```
*
* **1**: If `MIN_SEED_LOOKAHEAD = 1` the randao_mix used for the seed is from 2 epochs ago. So at epoch N, the seed
* is known and unchangable for duties at epoch N+1 and N+2 for proposer duties.
*
* ```python
* def get_seed(state: BeaconState, epoch: Epoch, domain_type: DomainType) -> Bytes32:
* mix = get_randao_mix(state, Epoch(epoch - MIN_SEED_LOOKAHEAD - 1))
* return hash(domain_type + uint_to_bytes(epoch) + mix)
* ```
*
* **2**: The active validator set can be predicted `MAX_SEED_LOOKAHEAD` in advance due to how activations are
* processed. We already compute the active validator set for the next epoch to optimize epoch processing, so it's
* reused here.
*
* **3**: Effective balance is not known ahead of time, but it rarely changes. Even if it changes, only a few
* balances are sampled to adjust the probability of the next selection (32 per epoch on average). So to invalidate
* the prediction the effective of one of those 32 samples should change and change the random_byte inequality.
*/
getBeaconProposersNextEpoch(): ValidatorIndex[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a rationale on when and why the proposer prediction stands or is invalidated

if (!this.proposersNextEpoch.computed) {
const indexes = computeProposers(
this.proposersNextEpoch.seed,
this.nextShuffling,
this.effectiveBalanceIncrements
);
this.proposersNextEpoch = {computed: true, indexes};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now computation of proposers is deferred AND cached when computed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dapplion this won't cache though, and on every request to get next proposers, the this.proposersNextEpoch.computed check here will always return false, and the if branch check always run.

You can confirm this by running the application, and trigger the end point multiple times, the if branch will always run.

I noticed this while working on the implementation and I think the reason why this is the case is how the chain.getHeadStateAtCurrentEpoch here works, but I did not go ahead with caching when I realised that the request to the endpoint itself was actually pretty fast and no need to even cache.

Copy link
Contributor

@dapplion dapplion May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this while working on the implementation and I think the reason why this is the case is how the chain.getHeadStateAtCurrentEpoch here works

If this is true, then it's a big issue! Can you open a dedicated issue for it? It should be investigated latter

I did not go ahead with caching when I realised that the request to the endpoint itself was actually pretty fast and no need to even cache.

This code runs on the main thread and according to benchmarks it takes ~100ms. That's still a significant amount of time to block the main thread and repeated computations must be avoided if possible

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And please link back to this context after opening a new dedicated issue. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


return this.proposersNextEpoch.indexes;
}

/**
* Return the indexed attestation corresponding to ``attestation``.
*/
Expand Down
7 changes: 6 additions & 1 deletion packages/beacon-state-transition/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,9 @@ export {EpochProcess, beforeProcessEpoch} from "./cache/epochProcess";

// Aux data-structures
export {PubkeyIndexMap, Index2PubkeyCache} from "./cache/pubkeyCache";
export {EffectiveBalanceIncrements, getEffectiveBalanceIncrementsZeroed} from "./cache/effectiveBalanceIncrements";

export {
EffectiveBalanceIncrements,
getEffectiveBalanceIncrementsZeroed,
getEffectiveBalanceIncrementsWithLen,
} from "./cache/effectiveBalanceIncrements";
4 changes: 1 addition & 3 deletions packages/beacon-state-transition/src/util/seed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {digest} from "@chainsafe/as-sha256";
import {Epoch, Bytes32, DomainType, ValidatorIndex} from "@chainsafe/lodestar-types";
import {assert, bytesToBigInt, intToBytes} from "@chainsafe/lodestar-utils";
import {
DOMAIN_BEACON_PROPOSER,
DOMAIN_SYNC_COMMITTEE,
EFFECTIVE_BALANCE_INCREMENT,
EPOCHS_PER_HISTORICAL_VECTOR,
Expand All @@ -25,11 +24,10 @@ import {computeEpochAtSlot} from "./epoch";
* Compute proposer indices for an epoch
*/
export function computeProposers(
state: BeaconStateAllForks,
epochSeed: Uint8Array,
shuffling: {epoch: Epoch; activeIndices: ValidatorIndex[]},
effectiveBalanceIncrements: EffectiveBalanceIncrements
): number[] {
const epochSeed = getSeed(state, shuffling.epoch, DOMAIN_BEACON_PROPOSER);
const startSlot = computeStartSlotAtEpoch(shuffling.epoch);
const proposers = [];
for (let slot = startSlot; slot < startSlot + SLOTS_PER_EPOCH; slot++) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import {itBench} from "@dapplion/benchmark";
import {Epoch} from "@chainsafe/lodestar-types";
import {DOMAIN_BEACON_PROPOSER} from "@chainsafe/lodestar-params";
import {
computeEpochAtSlot,
CachedBeaconStateAllForks,
computeEpochShuffling,
getNextSyncCommittee,
computeProposers,
getSeed,
} from "../../../../src";
import {generatePerfTestCachedStatePhase0, numValidators} from "../../util";
import {computeProposers} from "../../../../src/util/seed";

describe("epoch shufflings", () => {
let state: CachedBeaconStateAllForks;
Expand All @@ -25,7 +27,8 @@ describe("epoch shufflings", () => {
itBench({
id: `computeProposers - vc ${numValidators}`,
fn: () => {
computeProposers(state, state.epochCtx.nextShuffling, state.epochCtx.effectiveBalanceIncrements);
const epochSeed = getSeed(state, state.epochCtx.nextShuffling.epoch, DOMAIN_BEACON_PROPOSER);
computeProposers(epochSeed, state.epochCtx.nextShuffling, state.epochCtx.effectiveBalanceIncrements);
},
});

Expand Down
20 changes: 12 additions & 8 deletions packages/lodestar/src/api/impl/validator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,14 +278,17 @@ export function getValidatorApi({chain, config, logger, metrics, network, sync}:

const state = await chain.getHeadStateAtCurrentEpoch();

const duties: routes.validator.ProposerDuty[] = [];
const indexes: ValidatorIndex[] = [];

// Gather indexes to get pubkeys in batch (performance optimization)
for (let i = 0; i < SLOTS_PER_EPOCH; i++) {
// getBeaconProposer ensures the requested epoch is correct
const validatorIndex = state.epochCtx.getBeaconProposer(startSlot + i);
indexes.push(validatorIndex);
const stateEpoch = state.epochCtx.epoch;
let indexes: ValidatorIndex[] = [];

if (epoch === stateEpoch) {
indexes = state.epochCtx.getBeaconProposers();
} else if (epoch === stateEpoch + 1) {
// Requesting duties for next epoch is allow since they can be predicted with high probabilities.
// @see `epochCtx.getBeaconProposersNextEpoch` JSDocs for rationale.
indexes = state.epochCtx.getBeaconProposersNextEpoch();
} else {
throw Error(`Proposer duties for epoch ${epoch} not supported, current epoch ${stateEpoch}`);
}

// NOTE: this is the fastest way of getting compressed pubkeys.
Expand All @@ -294,6 +297,7 @@ export function getValidatorApi({chain, config, logger, metrics, network, sync}:
// TODO: Add a flag to just send 0x00 as pubkeys since the Lodestar validator does not need them.
const pubkeys = getPubkeysForIndices(state.validators, indexes);

const duties: routes.validator.ProposerDuty[] = [];
for (let i = 0; i < SLOTS_PER_EPOCH; i++) {
duties.push({slot: startSlot + i, validatorIndex: indexes[i], pubkey: pubkeys[i]});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,64 @@ describe("get proposers api impl", function () {
api = getValidatorApi(modules);
});

it("should get proposers", async function () {
it("should get proposers for next epoch", async function () {
syncStub.isSynced.returns(true);
server.sandbox.stub(chainStub.clock, "currentEpoch").get(() => 0);
server.sandbox.stub(chainStub.clock, "currentSlot").get(() => 0);
dbStub.block.get.resolves({message: {stateRoot: Buffer.alloc(32)}} as any);
const state = generateState(
{
slot: 0,
validators: generateValidators(25, {
effectiveBalance: MAX_EFFECTIVE_BALANCE,
activationEpoch: 0,
exitEpoch: FAR_FUTURE_EPOCH,
}),
balances: Array.from({length: 25}, () => MAX_EFFECTIVE_BALANCE),
},
config
);

const cachedState = createCachedBeaconStateTest(state, config);
chainStub.getHeadStateAtCurrentEpoch.resolves(cachedState);
const stubGetNextBeaconProposer = sinon.stub(cachedState.epochCtx, "getBeaconProposersNextEpoch");
const stubGetBeaconProposer = sinon.stub(cachedState.epochCtx, "getBeaconProposer");
stubGetNextBeaconProposer.returns([1]);
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access
const {data: result} = await api.getProposerDuties(1);
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
expect(result.length).to.be.equal(SLOTS_PER_EPOCH, "result should be equals to slots per epoch");
expect(stubGetNextBeaconProposer.called, "stubGetBeaconProposer function should not have been called").to.be.true;
expect(stubGetBeaconProposer.called, "stubGetBeaconProposer function should have been called").to.be.false;
});

it("should have different proposer for current and next epoch", async function () {
syncStub.isSynced.returns(true);
server.sandbox.stub(chainStub.clock, "currentEpoch").get(() => 0);
server.sandbox.stub(chainStub.clock, "currentSlot").get(() => 0);
dbStub.block.get.resolves({message: {stateRoot: Buffer.alloc(32)}} as any);
const state = generateState(
{
slot: 0,
validators: generateValidators(25, {
effectiveBalance: MAX_EFFECTIVE_BALANCE,
activationEpoch: 0,
exitEpoch: FAR_FUTURE_EPOCH,
}),
balances: Array.from({length: 25}, () => MAX_EFFECTIVE_BALANCE),
},
config
);
const cachedState = createCachedBeaconStateTest(state, config);
chainStub.getHeadStateAtCurrentEpoch.resolves(cachedState);
const stubGetBeaconProposer = sinon.stub(cachedState.epochCtx, "getBeaconProposer");
stubGetBeaconProposer.returns(1);
const {data: currentProposers} = await api.getProposerDuties(0);
const {data: nextProposers} = await api.getProposerDuties(1);
expect(currentProposers).to.not.deep.equal(nextProposers, "current proposer and next proposer should be different");
});

it("should not get proposers for more than one epoch in the future", async function () {
syncStub.isSynced.returns(true);
server.sandbox.stub(chainStub.clock, "currentEpoch").get(() => 0);
server.sandbox.stub(chainStub.clock, "currentSlot").get(() => 0);
Expand All @@ -71,8 +128,8 @@ describe("get proposers api impl", function () {
);
const cachedState = createCachedBeaconStateTest(state, config);
chainStub.getHeadStateAtCurrentEpoch.resolves(cachedState);
sinon.stub(cachedState.epochCtx, "getBeaconProposer").returns(1);
const {data: result} = await api.getProposerDuties(0);
expect(result.length).to.be.equal(SLOTS_PER_EPOCH);
const stubGetBeaconProposer = sinon.stub(cachedState.epochCtx, "getBeaconProposer");
stubGetBeaconProposer.throws();
expect(api.getProposerDuties(2), "calling getProposerDuties should throw").to.eventually.throws;
});
});