-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add graffiti API #63
Merged
Merged
Add graffiti API #63
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
e0332eb
Add graffiti API
nflaig 861deb1
Update graffiti API descriptions
nflaig 70d3ce1
Update title of graffiti response schema
nflaig e1c7031
Update Lodestar versions in API support table
nflaig f7ea96f
More detailed graffiti API descriptions
nflaig 327c747
Remove graffiti precedence for block proposal from API descriptions
nflaig 7e9f228
Update description of getGraffiti API
nflaig File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| get: | ||
| operationId: getGraffiti | ||
| summary: Get Graffiti | ||
| description: | | ||
| Get the graffiti for an individual validator. This graffiti is the one used by the | ||
| validator when proposing blocks. If no graffiti is set explicitly, returns | ||
| the process-wide default. | ||
| security: | ||
| - bearerAuth: [] | ||
| tags: | ||
| - Graffiti | ||
| parameters: | ||
| - in: path | ||
| name: pubkey | ||
| schema: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Pubkey" | ||
| required: true | ||
| responses: | ||
| "200": | ||
| description: success response | ||
| content: | ||
| application/json: | ||
| schema: | ||
| title: GraffitiResponse | ||
| type: object | ||
| required: [data] | ||
| properties: | ||
| data: | ||
nflaig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| type: object | ||
| required: [graffiti] | ||
| properties: | ||
| pubkey: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Pubkey" | ||
| graffiti: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Graffiti" | ||
| "400": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/BadRequest" | ||
| "401": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/Unauthorized" | ||
| "403": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/Forbidden" | ||
| "404": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/NotFound" | ||
| "500": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/InternalError" | ||
|
|
||
| post: | ||
nflaig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| operationId: setGraffiti | ||
| summary: Set Graffiti | ||
| description: | | ||
| Set the graffiti for an individual validator. | ||
| security: | ||
| - bearerAuth: [] | ||
| tags: | ||
| - Graffiti | ||
| parameters: | ||
| - in: path | ||
| name: pubkey | ||
| schema: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Pubkey" | ||
| required: true | ||
| requestBody: | ||
| content: | ||
| application/json: | ||
| schema: | ||
| title: SetGraffitiRequest | ||
| type: object | ||
| required: [graffiti] | ||
| properties: | ||
| graffiti: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Graffiti" | ||
| responses: | ||
| "202": | ||
| description: successfully updated | ||
| "400": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/BadRequest" | ||
| "401": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/Unauthorized" | ||
| "403": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/Forbidden" | ||
| "404": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/NotFound" | ||
| "500": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/InternalError" | ||
|
|
||
| delete: | ||
| operationId: deleteGraffiti | ||
| summary: Delete Configured Graffiti | ||
| description: | | ||
| Delete the configured graffiti for the specified public key. | ||
nflaig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| security: | ||
| - bearerAuth: [] | ||
| tags: | ||
| - Graffiti | ||
| parameters: | ||
| - in: path | ||
| name: pubkey | ||
| schema: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Pubkey" | ||
| required: true | ||
| responses: | ||
| "204": | ||
| description: Successfully removed the graffiti, or there was no graffiti set for the requested public key. | ||
| "400": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/BadRequest" | ||
| "401": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/Unauthorized" | ||
| "403": | ||
| description: A graffiti was found, but cannot be removed. This may be because the graffit was in configuration files that cannot be updated. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/ErrorResponse" | ||
| "404": | ||
| description: The key was not found on the server, nothing to delete. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/ErrorResponse" | ||
| "500": | ||
| $ref: "../keymanager-oapi.yaml#/components/responses/InternalError" | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| type: string | ||
| description: "Arbitrary data to set in the graffiti field of BeaconBlockBody" | ||
| example: "plain text value" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lighthouse has a custom graffiti endpoint (used by our Siren UI) that returns all graffitis of each validator. It would be nice to consider returning all graffitis and allow user to query specific pubkeys if needed, so user don't have to make multiple requests if they are interested in all validators running on a client. This would be useful for nodes running a larger number of validators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to have the same behavior as current fee recipient and gas limit API but if there is a use case for it we could add an additional API.
How should that one look like,
GET /eth/v1/validators/graffiti?pubkeys=0x01,0x02? Passing the pubkeys in query string might be problematic as it suffers from the same issue as getStateValidators API.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. I think the main use case to return multiple graffitis is when users have a large number of validators - so perhaps it make sense to have a separate endpoint to return all (
GET /eth/v1/validators/graffiti), and one to return a specified validator (what you have proposed)?Siren currently uses a custom endpoint that returns all graffitis, so it would be useful if we have something similar in the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not opposed to adding this but might be good to get feedback from other client teams on this. Maybe it makes sense to add this in a separate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this whole api is based on individual validators, we can do a fetch all, but it needs to be a different pattern to this endpoint.
Previously the argument for doing individual is its simple, and MVP, and it's never needed to be expanded upon, so I guess the real question is 'why provide get all for this specific interface?'
I get the desire to get all data back may be convenient, especially for large players, but equally so, the ability to set them all in one call (or set the new global default) may be desirable, and it can snowball quickly...
If it's needed and clients want to support it, thats fine. In the first instance, MVP is probably just the already defined interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me!