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

Make withdrawal probes bounded #4916

Merged
merged 6 commits into from
Dec 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/beacon-node/test/spec/presets/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {transition} from "./transition.js";
// Just disable all capella tests as well and renable when new vectors are released
// because the latest withdrawals we implemented are a breaking change
const skipOpts: SkipOpts = {
skippedForks: ["capella"],
skippedForks: ["eip4844"],
skippedRunners: ["light_client", "sync"],
skippedHandlers: ["full_withdrawals", "partial_withdrawals", "bls_to_execution_change", "withdrawals"],
};
Expand Down
2 changes: 1 addition & 1 deletion packages/beacon-node/test/spec/presets/sanity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const sanityBlocks: TestRunnerFn<SanityBlocksTestCase, BeaconStateAllFork
verifyProposer: verify,
verifySignatures: verify,
assertCorrectProgressiveBalances,
disabledWithdrawals: true,
disabledWithdrawals: fork === ForkName.eip4844,
});
}
return wrappedState;
Expand Down
2 changes: 1 addition & 1 deletion packages/beacon-node/test/spec/specTestVersioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {IDownloadTestsOptions} from "@lodestar/spec-test-util";
const __dirname = path.dirname(fileURLToPath(import.meta.url));

export const ethereumConsensusSpecsTests: IDownloadTestsOptions = {
specVersion: "v1.3.0-alpha.1-hotfix.2",
specVersion: "v1.3.0-alpha.2",
// Target directory is the host package root: 'packages/*/spec-tests'
outputDir: path.join(__dirname, "../../spec-tests"),
specTestsRepoUrl: "https://github.com/ethereum/consensus-spec-tests",
Expand Down
1 change: 1 addition & 0 deletions packages/params/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export const {

MAX_BLS_TO_EXECUTION_CHANGES,
MAX_WITHDRAWALS_PER_PAYLOAD,
MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP,

FIELD_ELEMENTS_PER_BLOB,
MAX_BLOBS_PER_BLOCK,
Expand Down
1 change: 1 addition & 0 deletions packages/params/src/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export interface BeaconPreset {
//////////
MAX_BLS_TO_EXECUTION_CHANGES: number;
MAX_WITHDRAWALS_PER_PAYLOAD: number;
MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP: number;

// EIP-4844
///////////
Expand Down
3 changes: 2 additions & 1 deletion packages/params/src/presets/mainnet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,11 @@ export const mainnetPreset: BeaconPreset = {
//////////
MAX_BLS_TO_EXECUTION_CHANGES: 16,
MAX_WITHDRAWALS_PER_PAYLOAD: 16,
MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP: 16384,

// EIP-4844
///////////
// https://github.com/ethereum/consensus-specs/blob/dev/presets/mainnet/eip4844.yaml
FIELD_ELEMENTS_PER_BLOB: 4096,
MAX_BLOBS_PER_BLOCK: 16,
MAX_BLOBS_PER_BLOCK: 4,
};
3 changes: 2 additions & 1 deletion packages/params/src/presets/minimal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,11 @@ export const minimalPreset: BeaconPreset = {
//////////
MAX_BLS_TO_EXECUTION_CHANGES: 16,
MAX_WITHDRAWALS_PER_PAYLOAD: 4,
MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP: 16,

// EIP-4844
///////////
// https://github.com/ethereum/consensus-specs/blob/dev/presets/minimal/eip4844.yaml
FIELD_ELEMENTS_PER_BLOB: 4,
MAX_BLOBS_PER_BLOCK: 16,
MAX_BLOBS_PER_BLOCK: 4,
};
2 changes: 1 addition & 1 deletion packages/params/test/e2e/ensure-config-is-synced.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {loadConfigYaml} from "../yaml.js";
// Not e2e, but slow. Run with e2e tests

/** https://github.com/ethereum/consensus-specs/releases */
const specConfigCommit = "v1.3.0-alpha.1";
const specConfigCommit = "v1.3.0-alpha.2";

describe("Ensure config is synced", function () {
this.timeout(60 * 1000);
Expand Down
23 changes: 20 additions & 3 deletions packages/state-transition/src/block/processWithdrawals.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import {ssz, capella} from "@lodestar/types";
import {MAX_EFFECTIVE_BALANCE, MAX_WITHDRAWALS_PER_PAYLOAD} from "@lodestar/params";
import {
MAX_EFFECTIVE_BALANCE,
MAX_WITHDRAWALS_PER_PAYLOAD,
MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP,
} from "@lodestar/params";

import {CachedBeaconStateCapella} from "../types.js";
import {decreaseBalance, hasEth1WithdrawalCredential} from "../util/index.js";
Expand All @@ -21,10 +25,23 @@ export function processWithdrawals(
}
decreaseBalance(state, withdrawal.validatorIndex, Number(withdrawal.amount));
}

// Update the nextWithdrawalIndex
if (expectedWithdrawals.length > 0) {
const latestWithdrawal = expectedWithdrawals[expectedWithdrawals.length - 1];
state.nextWithdrawalIndex = latestWithdrawal.index + 1;
}

// Update the nextWithdrawalValidatorIndex
if (expectedWithdrawals.length === MAX_WITHDRAWALS_PER_PAYLOAD) {
// All slots filled, nextWithdrawalValidatorIndex should be validatorIndex having next turn
const latestWithdrawal = expectedWithdrawals[expectedWithdrawals.length - 1];
state.nextWithdrawalValidatorIndex = (latestWithdrawal.validatorIndex + 1) % state.validators.length;
} else {
// expected withdrawals came up short in the bound, so we move nextWithdrawalValidatorIndex to
// the next post the bound
state.nextWithdrawalValidatorIndex =
(state.nextWithdrawalValidatorIndex + MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP) % state.validators.length;
}
}

Expand All @@ -34,14 +51,14 @@ export function getExpectedWithdrawals(
const epoch = state.epochCtx.epoch;
let withdrawalIndex = state.nextWithdrawalIndex;
const {validators, balances, nextWithdrawalValidatorIndex} = state;
const validatorCount = validators.length;
const bound = Math.min(validators.length, MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP);

let n = 0;

const withdrawals: capella.Withdrawal[] = [];
// Just run a bounded loop max iterating over all withdrawals
// however breaks out once we have MAX_WITHDRAWALS_PER_PAYLOAD
for (n = 0; n < validatorCount; n++) {
for (n = 0; n < bound; n++) {
// Get next validator in turn
const validatorIndex = (nextWithdrawalValidatorIndex + n) % validators.length;

Expand Down
2 changes: 1 addition & 1 deletion packages/state-transition/src/util/genesis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ export function initializeBeaconStateFromEth1(
ssz.capella.ExecutionPayloadHeader.defaultViewDU();
}

if (GENESIS_SLOT >= config.CAPELLA_FORK_EPOCH) {
if (GENESIS_SLOT >= config.EIP4844_FORK_EPOCH) {
const stateEip4844 = state as CompositeViewDU<typeof ssz.eip4844.BeaconState>;
stateEip4844.fork.previousVersion = config.EIP4844_FORK_VERSION;
stateEip4844.fork.currentVersion = config.EIP4844_FORK_VERSION;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ describe("getExpectedWithdrawals", () => {
// Intermediate bad case
{excessBalance: 0.1, eth1Credentials: 0.1, withdrawable: 0, withdrawn: 0, cache: true, sampled: 1_020},
{excessBalance: 0.03, eth1Credentials: 0.03, withdrawable: 0, withdrawn: 0, cache: true, sampled: 11_777},
{excessBalance: 0.01, eth1Credentials: 0.01, withdrawable: 0, withdrawn: 0, cache: true, sampled: 141_069},
// Worst case: All validators need to be probed
{excessBalance: 0, eth1Credentials: 0.0, withdrawable: 0, withdrawn: 0, cache: true, sampled: 250_000},
{excessBalance: 0, eth1Credentials: 0.0, withdrawable: 0, withdrawn: 0, cache: false, sampled: 250_000},
{excessBalance: 0, eth1Credentials: 1, withdrawable: 0, withdrawn: 0, cache: true, sampled: 250_000},
{excessBalance: 0, eth1Credentials: 1, withdrawable: 0, withdrawn: 0, cache: false, sampled: 250_000},
// Expected 141_069 but gets bounded at 16_384
{excessBalance: 0.01, eth1Credentials: 0.01, withdrawable: 0, withdrawn: 0, cache: true, sampled: 16_384},
// Worst case: All validators 250_000 need to be probed but get bounded at 16_384
{excessBalance: 0, eth1Credentials: 0.0, withdrawable: 0, withdrawn: 0, cache: true, sampled: 16_384},
{excessBalance: 0, eth1Credentials: 0.0, withdrawable: 0, withdrawn: 0, cache: false, sampled: 16_384},
{excessBalance: 0, eth1Credentials: 1, withdrawable: 0, withdrawn: 0, cache: true, sampled: 16_384},
{excessBalance: 0, eth1Credentials: 1, withdrawable: 0, withdrawn: 0, cache: false, sampled: 16_384},
];

for (const opts of testCases) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ describe("getExpectedWithdrawals", () => {
{excessBalance: 0.95, eth1Credentials: 0.7, withdrawable: 0.05, withdrawn: 0, withdrawals: 16, sampled: 18},
// Intermediate bad case
{excessBalance: 0.1, eth1Credentials: 0.1, withdrawable: 0, withdrawn: 0, withdrawals: 16, sampled: 1020},
{excessBalance: 0.01, eth1Credentials: 0.01, withdrawable: 0, withdrawn: 0, withdrawals: 16, sampled: 141069},
// Worst case: All validators need to be probed
{excessBalance: 0, eth1Credentials: 0.0, withdrawable: 0, withdrawn: 0, withdrawals: 0, sampled: vc},
// Expected 141069 but gets bounded by 16384
{excessBalance: 0.01, eth1Credentials: 0.01, withdrawable: 0, withdrawn: 0, withdrawals: 2, sampled: 16384},
// Expected 250000 but gets bounded by 16384
{excessBalance: 0, eth1Credentials: 0.0, withdrawable: 0, withdrawn: 0, withdrawals: 0, sampled: 16384},
];

for (const opts of testCases) {
Expand Down
1 change: 1 addition & 0 deletions packages/validator/src/util/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ function getSpecCriticalParams(localConfig: IChainConfig): Record<keyof ConfigWi
/////////////////
MAX_BLS_TO_EXECUTION_CHANGES: capellaForkRelevant,
MAX_WITHDRAWALS_PER_PAYLOAD: capellaForkRelevant,
MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP: capellaForkRelevant,

// # EIP4844Preset
/////////////////
Expand Down