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.

Sign up for GitHub

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

fix: handle skip_randao_verification query param as per spec #6576

Merged
merged 10 commits into from
Mar 21, 2024
Merged

fix: handle skip_randao_verification query param as per spec #6576

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
df9e7de
Fix validator produceBlockV3 req param
nazarhussain Mar 21, 2024
3e1ee02
Update the lighthouse version for sim tests
nazarhussain Mar 21, 2024
f1a5e44
Include deneb in mixed-client sim tests
nazarhussain Mar 21, 2024
e765907
Fix query param schema type
nazarhussain Mar 21, 2024
31a699e
Fix the unit tests
nazarhussain Mar 21, 2024
df95b69
Fix the code as per feedback
nazarhussain Mar 21, 2024
3266f45
Update the parse method
nazarhussain Mar 21, 2024
6fde9d9
Add cancun support to geth client for sim tests
nazarhussain Mar 21, 2024
1418586
Fix the types
nazarhussain Mar 21, 2024
f8f7898
Add deneb support to multi-fork sim tests
nazarhussain Mar 21, 2024
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 .env.test
View file
Open in desktop
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
View file
Open in desktop
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;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nimbus also treated this attribute as string.

https://github.com/status-im/nimbus-eth2/blob/fc9c72f0ebc2013858f6b28c1940838c97a2f814/beacon_chain/rpc/rest_validator_api.nim#L566

nazarhussain marked this conversation as resolved.
Show resolved Hide resolved
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,
nazarhussain marked this conversation as resolved.
Show resolved Hide resolved
...(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 && ["", "true"].includes(skipRandaoVerification) ? true : false;
nazarhussain marked this conversation as resolved.
Show resolved Hide resolved
}
6 changes: 3 additions & 3 deletions packages/api/test/unit/beacon/testData/validator.ts
View file
Open in desktop
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
View file
Open in desktop
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,
Copy link
Member

Choose a reason for hiding this comment

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

do we even have to set this explicitly, or do we want to test as it works in production, i.e. pre deneb use v2 and post deneb v3?

Copy link
Member

Choose a reason for hiding this comment

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

I don't believe we need to explicitly state this anymore now that we're post Deneb and use it by default. Everything on our side should've flipped to v3 automatically at Deneb unless explicitly stated false.

Copy link
Member

Choose a reason for hiding this comment

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

yes but sim tests go through all forks from phase0, just wondering if it makes more sense to test this more closely to production behavior were this only switches to v3 post deneb. On the other hand, there is no public network I know of that is pre deneb anymore, so it just matters for private / testnets or maybe emphemery which does not start genesis at deneb right now iirc

},
},
},
Expand Down
Loading