Skip to content

Commit

Permalink
fix: handle skip_randao_verification query param as per spec (#6576)
Browse files Browse the repository at this point in the history
* Fix validator produceBlockV3 req param

* Update the lighthouse version for sim tests

* Include deneb in mixed-client sim tests

* Fix query param schema type

* Fix the unit tests

* Fix the code as per feedback

* Update the parse method

* Add cancun support to geth client for sim tests

* Fix the types

* Add deneb support to multi-fork sim tests
  • Loading branch information
nazarhussain authored Mar 21, 2024
1 parent 9b1f0af commit 129f300
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 30 deletions.
2 changes: 1 addition & 1 deletion .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
GETH_DOCKER_IMAGE=ethereum/client-go:v1.13.14
# Use either image or local binary for the testing
GETH_BINARY_DIR=
LIGHTHOUSE_DOCKER_IMAGE=sigp/lighthouse:v4.6.0-amd64-modern-dev
LIGHTHOUSE_DOCKER_IMAGE=sigp/lighthouse:v5.1.1-amd64-modern-dev

# We can't upgrade nethermind further due to genesis hash mismatch with the geth
# https://github.com/NethermindEth/nethermind/issues/6683
Expand Down
12 changes: 8 additions & 4 deletions packages/api/src/beacon/routes/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ export type ReqTypes = {
query: {
randao_reveal: string;
graffiti: string;
skip_randao_verification?: boolean;
skip_randao_verification?: string;
fee_recipient?: string;
builder_selection?: string;
builder_boost_factor?: string;
Expand Down Expand Up @@ -556,7 +556,7 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
randao_reveal: toHexString(randaoReveal),
graffiti: toGraffitiHex(graffiti),
fee_recipient: opts?.feeRecipient,
skip_randao_verification: skipRandaoVerification,
...(skipRandaoVerification && {skip_randao_verification: ""}),
builder_selection: opts?.builderSelection,
builder_boost_factor: opts?.builderBoostFactor?.toString(),
strict_fee_recipient_check: opts?.strictFeeRecipientCheck,
Expand All @@ -567,7 +567,7 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
params.slot,
fromHexString(query.randao_reveal),
fromGraffitiHex(query.graffiti),
query.skip_randao_verification,
parseSkipRandaoVerification(query.skip_randao_verification),
{
feeRecipient: query.fee_recipient,
builderSelection: query.builder_selection as BuilderSelection,
Expand All @@ -582,7 +582,7 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
randao_reveal: Schema.StringRequired,
graffiti: Schema.String,
fee_recipient: Schema.String,
skip_randao_verification: Schema.Boolean,
skip_randao_verification: Schema.String,
builder_selection: Schema.String,
builder_boost_factor: Schema.String,
strict_fee_recipient_check: Schema.Boolean,
Expand Down Expand Up @@ -795,3 +795,7 @@ export function getReturnTypes(): ReturnTypes<Api> {
function parseBuilderBoostFactor(builderBoostFactorInput?: string | number | bigint): bigint | undefined {
return builderBoostFactorInput !== undefined ? BigInt(builderBoostFactorInput) : undefined;
}

function parseSkipRandaoVerification(skipRandaoVerification?: string): boolean {
return skipRandaoVerification !== undefined && skipRandaoVerification === "";
}
6 changes: 3 additions & 3 deletions packages/api/test/unit/beacon/testData/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export const testData: GenericServerTestCases<Api> = {
32000,
randaoReveal,
graffiti,
undefined,
false,
{
feeRecipient,
builderSelection: undefined,
Expand All @@ -65,7 +65,7 @@ export const testData: GenericServerTestCases<Api> = {
32000,
randaoReveal,
graffiti,
undefined,
false,
{
feeRecipient,
builderSelection: undefined,
Expand Down Expand Up @@ -109,7 +109,7 @@ export const testData: GenericServerTestCases<Api> = {
32000,
randaoReveal,
graffiti,
undefined,
false,
{
feeRecipient,
builderSelection: undefined,
Expand Down
13 changes: 4 additions & 9 deletions packages/cli/test/sim/mixed_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import {connectAllNodes, waitForSlot} from "../utils/simulation/utils/network.js
const altairForkEpoch = 2;
const bellatrixForkEpoch = 4;
const capellaForkEpoch = 6;
const runTillEpoch = 8;
const denebForkEpoch = 8;
const runTillEpoch = 10;
const syncWaitEpoch = 2;

const {estimatedTimeoutMs, forkConfig} = defineSimTestConfig({
ALTAIR_FORK_EPOCH: altairForkEpoch,
BELLATRIX_FORK_EPOCH: bellatrixForkEpoch,
CAPELLA_FORK_EPOCH: capellaForkEpoch,
DENEB_FORK_EPOCH: denebForkEpoch,
runTillEpoch: runTillEpoch + syncWaitEpoch,
initialNodes: 2,
});
Expand All @@ -41,18 +43,11 @@ const env = await SimulationEnvironment.initWithDefaults(
keysCount: 32,
remote: true,
beacon: BeaconClient.Lighthouse,
// for cross client make sure lodestar doesn't use v3 for now untill lighthouse supports
validator: {
type: ValidatorClient.Lodestar,
options: {
clientOptions: {
useProduceBlockV3: false,
// this should cause usage of produceBlockV2
//
// but if blinded production is enabled in lighthouse beacon then this should cause
// usage of produce blinded block which should return execution block in blinded format
// but only enable that after testing lighthouse beacon
"builder.selection": "executiononly",
useProduceBlockV3: true,
},
},
},
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/test/sim/multi_fork.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ import {createWithdrawalAssertions} from "../utils/simulation/assertions/withdra
const altairForkEpoch = 2;
const bellatrixForkEpoch = 4;
const capellaForkEpoch = 6;
const runTillEpoch = 8;
const denebForkEpoch = 8;
const runTillEpoch = 10;
const syncWaitEpoch = 2;

const {estimatedTimeoutMs, forkConfig} = defineSimTestConfig({
ALTAIR_FORK_EPOCH: altairForkEpoch,
BELLATRIX_FORK_EPOCH: bellatrixForkEpoch,
CAPELLA_FORK_EPOCH: capellaForkEpoch,
DENEB_FORK_EPOCH: denebForkEpoch,
runTillEpoch: runTillEpoch + syncWaitEpoch,
initialNodes: 5,
});
Expand Down
14 changes: 11 additions & 3 deletions packages/cli/test/utils/simulation/execution_clients/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
ExecutionNode,
ExecutionStartMode,
} from "../interfaces.js";
import {getEstimatedShanghaiTime} from "../utils/index.js";
import {getEstimatedForkTime} from "../utils/index.js";
import {getGethGenesisBlock} from "../utils/execution_genesis.js";
import {ensureDirectories} from "../utils/paths.js";
import {generateGethNode} from "./geth.js";
Expand All @@ -28,8 +28,16 @@ export async function createExecutionNode<E extends ExecutionClient>(
cliqueSealingPeriod: options.cliqueSealingPeriod ?? CLIQUE_SEALING_PERIOD,
shanghaiTime:
options.shanghaiTime ??
getEstimatedShanghaiTime({
capellaForkEpoch: forkConfig.CAPELLA_FORK_EPOCH,
getEstimatedForkTime({
forkEpoch: forkConfig.CAPELLA_FORK_EPOCH,
genesisTime: options.genesisTime,
secondsPerSlot: forkConfig.SECONDS_PER_SLOT,
additionalSlots: 0,
}),
cancunTime:
options.cancunTime ??
getEstimatedForkTime({
forkEpoch: forkConfig.DENEB_FORK_EPOCH,
genesisTime: options.genesisTime,
secondsPerSlot: forkConfig.SECONDS_PER_SLOT,
additionalSlots: 0,
Expand Down
24 changes: 21 additions & 3 deletions packages/cli/test/utils/simulation/execution_clients/nethermind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,19 @@ export const generateNethermindNode: ExecutionNodeGenerator<ExecutionClient.Neth
throw Error(`EL ENV must be provided, NETHERMIND_DOCKER_IMAGE: ${process.env.NETHERMIND_DOCKER_IMAGE}`);
}

const {id, mode, ttd, address, mining, clientOptions, nodeIndex, cliqueSealingPeriod, shanghaiTime, genesisTime} =
opts;
const {
id,
mode,
ttd,
address,
mining,
clientOptions,
nodeIndex,
cliqueSealingPeriod,
shanghaiTime,
cancunTime,
genesisTime,
} = opts;
const ports = getNodePorts(nodeIndex);

const {rootDir, rootDirMounted, logFilePath, jwtsecretFilePathMounted} = getNodeMountedPaths(
Expand Down Expand Up @@ -48,7 +59,14 @@ export const generateNethermindNode: ExecutionNodeGenerator<ExecutionClient.Neth
await writeFile(
chainSpecPath,
JSON.stringify(
getNethermindChainSpec(mode, {ttd, cliqueSealingPeriod, shanghaiTime, genesisTime, clientOptions: []})
getNethermindChainSpec(mode, {
ttd,
cliqueSealingPeriod,
shanghaiTime,
cancunTime,
genesisTime,
clientOptions: [],
})
)
);
},
Expand Down
1 change: 1 addition & 0 deletions packages/cli/test/utils/simulation/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ export interface ExecutionGenesisOptions<E extends ExecutionClient = ExecutionCl
ttd: bigint;
cliqueSealingPeriod: number;
shanghaiTime: number;
cancunTime: number;
genesisTime: number;
clientOptions: ExecutionClientsOptions[E];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const getGethGenesisBlock = (
mode: ExecutionStartMode,
options: ExecutionGenesisOptions
): Record<string, unknown> => {
const {ttd, cliqueSealingPeriod, shanghaiTime, genesisTime} = options;
const {ttd, cliqueSealingPeriod, shanghaiTime, genesisTime, cancunTime} = options;

const genesis = {
config: {
Expand All @@ -24,6 +24,7 @@ export const getGethGenesisBlock = (
berlinBlock: 0,
londonBlock: 0,
shanghaiTime,
cancunTime,
terminalTotalDifficulty: Number(ttd as bigint),
clique: {period: cliqueSealingPeriod, epoch: 30000},
},
Expand Down
10 changes: 5 additions & 5 deletions packages/cli/test/utils/simulation/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,20 @@ export const getEstimatedTTD = ({
return BigInt(Math.ceil(secondsTillBellatrix / cliqueSealingPeriod) * ETH_TTD_INCREMENT);
};

export const getEstimatedShanghaiTime = ({
export const getEstimatedForkTime = ({
genesisTime,
secondsPerSlot,
capellaForkEpoch,
forkEpoch,
additionalSlots,
}: {
genesisTime: number;
secondsPerSlot: number;
capellaForkEpoch: number;
forkEpoch: number;
additionalSlots: number;
}): number => {
const secondsTillCapella = capellaForkEpoch * activePreset.SLOTS_PER_EPOCH * secondsPerSlot;
const secondsTillFork = forkEpoch * activePreset.SLOTS_PER_EPOCH * secondsPerSlot;

return genesisTime + secondsTillCapella + additionalSlots * secondsPerSlot;
return genesisTime + secondsTillFork + additionalSlots * secondsPerSlot;
};

export const squeezeString = (val: string, length: number, sep = "..."): string => {
Expand Down

0 comments on commit 129f300

Please sign in to comment.