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

feat: allow keymanager to configure a validator pubkey utf8 graffiti #6083

Merged
merged 3 commits into from
Nov 3, 2023
Merged

feat: allow keymanager to configure a validator pubkey utf8 graffiti #6083

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a4f9c3
feat: allow keymanager to configure a validator pubkey utf8 graffiti
g11tech Nov 3, 2023
6e103ea
fix and tests
g11tech Nov 3, 2023
b15047b
Review code
nflaig Nov 3, 2023
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
55 changes: 55 additions & 0 deletions packages/api/src/keymanager/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ export type FeeRecipientData = {
pubkey: string;
ethaddress: string;
};
export type GraffitiData = {
pubkey: string;
graffiti: string;
};
export type GasLimitData = {
pubkey: string;
gasLimit: number;
Expand Down Expand Up @@ -205,6 +209,25 @@ export type Api = {
>
>;

listGraffiti(pubkey: string): Promise<ApiClientResponse<{[HttpStatusCode.OK]: {data: GraffitiData}}>>;
setGraffiti(
pubkey: string,
graffiti: string
): Promise<
ApiClientResponse<
{[HttpStatusCode.OK]: void; [HttpStatusCode.NO_CONTENT]: void},
HttpStatusCode.UNAUTHORIZED | HttpStatusCode.FORBIDDEN | HttpStatusCode.NOT_FOUND
>
>;
deleteGraffiti(
pubkey: string
): Promise<
ApiClientResponse<
{[HttpStatusCode.OK]: void; [HttpStatusCode.NO_CONTENT]: void},
HttpStatusCode.UNAUTHORIZED | HttpStatusCode.FORBIDDEN | HttpStatusCode.NOT_FOUND
>
>;

getGasLimit(pubkey: string): Promise<ApiClientResponse<{[HttpStatusCode.OK]: {data: GasLimitData}}>>;
setGasLimit(
pubkey: string,
Expand Down Expand Up @@ -259,6 +282,10 @@ export const routesData: RoutesData<Api> = {
setFeeRecipient: {url: "/eth/v1/validator/{pubkey}/feerecipient", method: "POST", statusOk: 202},
deleteFeeRecipient: {url: "/eth/v1/validator/{pubkey}/feerecipient", method: "DELETE", statusOk: 204},

listGraffiti: {url: "/eth/v1/validator/{pubkey}/graffiti", method: "GET"},
setGraffiti: {url: "/eth/v1/validator/{pubkey}/graffiti", method: "POST", statusOk: 202},
deleteGraffiti: {url: "/eth/v1/validator/{pubkey}/graffiti", method: "DELETE", statusOk: 204},

getGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "GET"},
setGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "POST", statusOk: 202},
deleteGasLimit: {url: "/eth/v1/validator/{pubkey}/gas_limit", method: "DELETE", statusOk: 204},
Expand Down Expand Up @@ -291,6 +318,10 @@ export type ReqTypes = {
setFeeRecipient: {params: {pubkey: string}; body: {ethaddress: string}};
deleteFeeRecipient: {params: {pubkey: string}};

listGraffiti: {params: {pubkey: string}};
setGraffiti: {params: {pubkey: string}; body: {graffiti: string}};
deleteGraffiti: {params: {pubkey: string}};

getGasLimit: {params: {pubkey: string}};
setGasLimit: {params: {pubkey: string}; body: {gas_limit: string}};
deleteGasLimit: {params: {pubkey: string}};
Expand Down Expand Up @@ -347,6 +378,29 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
},
},

listGraffiti: {
writeReq: (pubkey) => ({params: {pubkey}}),
parseReq: ({params: {pubkey}}) => [pubkey],
schema: {
params: {pubkey: Schema.StringRequired},
},
},
setGraffiti: {
writeReq: (pubkey, graffiti) => ({params: {pubkey}, body: {graffiti}}),
parseReq: ({params: {pubkey}, body: {graffiti}}) => [pubkey, graffiti],
schema: {
params: {pubkey: Schema.StringRequired},
body: Schema.Object,
},
},
deleteGraffiti: {
writeReq: (pubkey) => ({params: {pubkey}}),
parseReq: ({params: {pubkey}}) => [pubkey],
schema: {
params: {pubkey: Schema.StringRequired},
},
},

getGasLimit: {
writeReq: (pubkey) => ({params: {pubkey}}),
parseReq: ({params: {pubkey}}) => [pubkey],
Expand Down Expand Up @@ -391,6 +445,7 @@ export function getReturnTypes(): ReturnTypes<Api> {
deleteRemoteKeys: jsonType("snake"),

listFeeRecipient: jsonType("snake"),
listGraffiti: jsonType("snake"),
getGasLimit: ContainerData(
new ContainerType(
{
Expand Down
14 changes: 14 additions & 0 deletions packages/api/test/unit/keymanager/testData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {GenericServerTestCases} from "../../utils/genericServerTest.js";
// randomly pregenerated pubkey
const pubkeyRand = "0x84105a985058fc8740a48bf1ede9d223ef09e8c6b1735ba0a55cf4a9ff2ff92376b778798365e488dab07a652eb04576";
const ethaddressRand = "0xabcf8e0d4e9587369b2301d0790347320302cc09";
const graffitiRandUtf8 = "636861696e736166652f6c6f6465737461720000000000000000000000000000";
nflaig marked this conversation as resolved.
Show resolved Hide resolved
const gasLimitRand = 30_000_000;

export const testData: GenericServerTestCases<Api> = {
Expand Down Expand Up @@ -69,6 +70,19 @@ export const testData: GenericServerTestCases<Api> = {
res: undefined,
},

listGraffiti: {
args: [pubkeyRand],
res: {data: {pubkey: pubkeyRand, graffiti: graffitiRandUtf8}},
},
setGraffiti: {
args: [pubkeyRand, graffitiRandUtf8],
res: undefined,
},
deleteGraffiti: {
args: [pubkeyRand],
res: undefined,
},

getGasLimit: {
args: [pubkeyRand],
res: {data: {pubkey: pubkeyRand, gasLimit: gasLimitRand}},
Expand Down
22 changes: 22 additions & 0 deletions packages/cli/src/cmds/validator/keymanager/impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,28 @@ export class KeymanagerApi implements Api {
);
}

async listGraffiti(pubkeyHex: string): ReturnType<Api["listGraffiti"]> {
return {data: {pubkey: pubkeyHex, graffiti: this.validator.validatorStore.getGraffiti(pubkeyHex)}};
}

async setGraffiti(pubkeyHex: string, graffiti: string): Promise<void> {
this.checkIfProposerWriteEnabled();
this.validator.validatorStore.setGraffiti(pubkeyHex, graffiti);
this.persistedKeysBackend.writeProposerConfig(
pubkeyHex,
this.validator.validatorStore.getProposerConfig(pubkeyHex)
);
}

async deleteGraffiti(pubkeyHex: string): Promise<void> {
this.checkIfProposerWriteEnabled();
this.validator.validatorStore.deleteGraffiti(pubkeyHex);
this.persistedKeysBackend.writeProposerConfig(
pubkeyHex,
this.validator.validatorStore.getProposerConfig(pubkeyHex)
);
}

async getGasLimit(pubkeyHex: string): ReturnType<Api["getGasLimit"]> {
const gasLimit = this.validator.validatorStore.getGasLimit(pubkeyHex);
return {data: {pubkey: pubkeyHex, gasLimit}};
Expand Down
67 changes: 62 additions & 5 deletions packages/cli/test/e2e/propserConfigfromKeymanager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ describe("import keystores from api, test DefaultProposerConfig", function () {
const defaultOptions = {
suggestedFeeRecipient: "0x0000000000000000000000000000000000000000",
gasLimit: 30_000_000,
graffiti: "aaaa",
};

const updatedOptions = {
suggestedFeeRecipient: "0xcccccccccccccccccccccccccccccccccccccccc",
gasLimit: 35_000_000,
graffiti: "bbbb",
};

before("Clean dataDir", () => {
Expand All @@ -47,7 +49,10 @@ describe("import keystores from api, test DefaultProposerConfig", function () {
const slashingProtectionStr = JSON.stringify(slashingProtection);

it("1 . run 'validator' import keys from API, getdefaultfeeRecipient", async () => {
const {keymanagerClient} = await startValidatorWithKeyManager([], {dataDir, testContext});
const {keymanagerClient} = await startValidatorWithKeyManager([`--graffiti ${defaultOptions.graffiti}`], {
dataDir,
testContext,
});
// Produce and encrypt keystores
// Import test keys
const keystoresStr = await getKeystoresStr(passphrase, secretKeys);
Expand All @@ -73,6 +78,26 @@ describe("import keystores from api, test DefaultProposerConfig", function () {
"FeeRecipient Check updated"
);

//////////////// Graffiti

let graffiti0 = await keymanagerClient.listGraffiti(pubkeys[0]);
ApiError.assert(graffiti0);
expectDeepEquals(
graffiti0.response.data,
{pubkey: pubkeys[0], graffiti: defaultOptions.graffiti},
"Graffiti Check default"
);

// Set Graffiti to updatedOptions
ApiError.assert(await keymanagerClient.setGraffiti(pubkeys[0], updatedOptions.graffiti));
graffiti0 = await keymanagerClient.listGraffiti(pubkeys[0]);
ApiError.assert(graffiti0);
expectDeepEquals(
graffiti0.response.data,
{pubkey: pubkeys[0], graffiti: updatedOptions.graffiti},
"FeeRecipient Check updated"
);

/////////// GasLimit

let gasLimit0 = await keymanagerClient.getGasLimit(pubkeys[0]);
Expand All @@ -95,7 +120,10 @@ describe("import keystores from api, test DefaultProposerConfig", function () {
});

it("2 . run 'validator' Check last feeRecipient and gasLimit persists", async () => {
const {keymanagerClient} = await startValidatorWithKeyManager([], {dataDir, testContext});
const {keymanagerClient} = await startValidatorWithKeyManager([`--graffiti ${defaultOptions.graffiti}`], {
dataDir,
testContext,
});

// next time check edited feeRecipient persists
let feeRecipient0 = await keymanagerClient.listFeeRecipient(pubkeys[0]);
Expand All @@ -116,6 +144,25 @@ describe("import keystores from api, test DefaultProposerConfig", function () {
"FeeRecipient Check default after delete"
);

// graffiti persists
let graffiti0 = await keymanagerClient.listGraffiti(pubkeys[0]);
ApiError.assert(graffiti0);
expectDeepEquals(
graffiti0.response.data,
{pubkey: pubkeys[0], graffiti: updatedOptions.graffiti},
"FeeRecipient Check default persists"
);

// after deletion graffiti restored to default
ApiError.assert(await keymanagerClient.deleteGraffiti(pubkeys[0]));
graffiti0 = await keymanagerClient.listGraffiti(pubkeys[0]);
ApiError.assert(graffiti0);
expectDeepEquals(
graffiti0.response.data,
{pubkey: pubkeys[0], graffiti: defaultOptions.graffiti},
"FeeRecipient Check default after delete"
);

// gasLimit persists
let gasLimit0 = await keymanagerClient.getGasLimit(pubkeys[0]);
ApiError.assert(gasLimit0);
Expand All @@ -136,7 +183,10 @@ describe("import keystores from api, test DefaultProposerConfig", function () {
});

it("3 . run 'validator' FeeRecipient and GasLimit should be default after delete", async () => {
const {keymanagerClient} = await startValidatorWithKeyManager([], {dataDir, testContext});
const {keymanagerClient} = await startValidatorWithKeyManager([`--graffiti ${defaultOptions.graffiti}`], {
dataDir,
testContext,
});

const feeRecipient0 = await keymanagerClient.listFeeRecipient(pubkeys[0]);
ApiError.assert(feeRecipient0);
Expand All @@ -146,10 +196,17 @@ describe("import keystores from api, test DefaultProposerConfig", function () {
"FeeRecipient Check default persists"
);

let gasLimit0 = await keymanagerClient.getGasLimit(pubkeys[0]);
ApiError.assert(await keymanagerClient.deleteGraffiti(pubkeys[0]));
const graffiti0 = await keymanagerClient.listGraffiti(pubkeys[0]);
ApiError.assert(graffiti0);
expectDeepEquals(
graffiti0.response.data,
{pubkey: pubkeys[0], graffiti: defaultOptions.graffiti},
"FeeRecipient Check default persists"
);

ApiError.assert(await keymanagerClient.deleteGasLimit(pubkeys[0]));
gasLimit0 = await keymanagerClient.getGasLimit(pubkeys[0]);
const gasLimit0 = await keymanagerClient.getGasLimit(pubkeys[0]);
ApiError.assert(gasLimit0);
expectDeepEquals(
gasLimit0.response.data,
Expand Down
17 changes: 17 additions & 0 deletions packages/validator/src/services/validatorStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,23 @@ export class ValidatorStore {
return this.validators.get(pubkeyHex)?.graffiti ?? this.defaultProposerConfig.graffiti;
}

setGraffiti(pubkeyHex: PubkeyHex, graffiti: string): void {
const validatorData = this.validators.get(pubkeyHex);
if (validatorData === undefined) {
throw Error(`Validator pubkey ${pubkeyHex} not known`);
}
validatorData.graffiti = graffiti;
}

deleteGraffiti(pubkeyHex: PubkeyHex): void {
const validatorData = this.validators.get(pubkeyHex);
if (validatorData === undefined) {
throw Error(`Validator pubkey ${pubkeyHex} not known`);
}
// This should directly modify data in the map
nflaig marked this conversation as resolved.
Show resolved Hide resolved
delete validatorData["graffiti"];
}

getBuilderSelection(pubkeyHex: PubkeyHex): routes.validator.BuilderSelection {
return (this.validators.get(pubkeyHex)?.builder || {}).selection ?? this.defaultProposerConfig.builder.selection;
}
Expand Down