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

[Merged by Bors] - Update the voluntary exit endpoint to comply with the key manager specification #4679

Closed

Conversation

jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Aug 30, 2023

Issue Addressed

#4635

Proposed Changes

Wrap the SignedVoluntaryExit object in a GenericResponse container, adding an additional data layer, to ensure compliance with the key manager API specification.

The new response would look like this:

{"data":{"message":{"epoch":"196868","validator_index":"505597"},"signature":"0xhexsig"}}

This is a backward incompatible change and will affect Siren as well.

@jimmygchen jimmygchen added val-client Relates to the validator client binary HTTP-API backwards-incompat Backwards-incompatible API change ready-for-review The code is ready for review labels Aug 30, 2023
@jimmygchen
Copy link
Member Author

jimmygchen commented Sep 10, 2023

Marking this as "do not merge" for now, as there's a discussion (on Discord R&D #api channel) going on about the this endpoint spec.

@jimmygchen
Copy link
Member Author

Looks like it's unlikely that the spec for the endpoint will change as it's been implemented by multiple clients and it's been around for a bit (see discussions here), if there's a strong desire to move over to GET, it would be new a V2 endpoint.

@jimmygchen jimmygchen added the v4.5.0 ETA Q4 2023 label Sep 21, 2023
@rickimoore
Copy link
Member

sigp/siren#220

going to do some tests then merge this

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Let's merge this now that we have a Siren impl.

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Sep 21, 2023
@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Sep 22, 2023
…cification (#4679)

## Issue Addressed

#4635 

## Proposed Changes

Wrap the `SignedVoluntaryExit` object in a `GenericResponse` container, adding an additional `data` layer, to ensure compliance with the key manager API specification.

The new response would look like this:

```json
{"data":{"message":{"epoch":"196868","validator_index":"505597"},"signature":"0xhexsig"}}
```

This is a backward incompatible change and will affect Siren as well.
@bors
Copy link

bors bot commented Sep 22, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Sep 22, 2023
…cification (#4679)

## Issue Addressed

#4635 

## Proposed Changes

Wrap the `SignedVoluntaryExit` object in a `GenericResponse` container, adding an additional `data` layer, to ensure compliance with the key manager API specification.

The new response would look like this:

```json
{"data":{"message":{"epoch":"196868","validator_index":"505597"},"signature":"0xhexsig"}}
```

This is a backward incompatible change and will affect Siren as well.
@michaelsproul
Copy link
Member

bors r-
bors r+

@bors
Copy link

bors bot commented Sep 22, 2023

This PR was included in a batch that was canceled, it will be automatically retried

@bors
Copy link

bors bot commented Sep 22, 2023

Canceled.

bors bot pushed a commit that referenced this pull request Sep 22, 2023
…cification (#4679)

## Issue Addressed

#4635 

## Proposed Changes

Wrap the `SignedVoluntaryExit` object in a `GenericResponse` container, adding an additional `data` layer, to ensure compliance with the key manager API specification.

The new response would look like this:

```json
{"data":{"message":{"epoch":"196868","validator_index":"505597"},"signature":"0xhexsig"}}
```

This is a backward incompatible change and will affect Siren as well.
@bors
Copy link

bors bot commented Sep 22, 2023

This PR was included in a batch that was canceled, it will be automatically retried

bors bot pushed a commit that referenced this pull request Sep 22, 2023
…cification (#4679)

## Issue Addressed

#4635 

## Proposed Changes

Wrap the `SignedVoluntaryExit` object in a `GenericResponse` container, adding an additional `data` layer, to ensure compliance with the key manager API specification.

The new response would look like this:

```json
{"data":{"message":{"epoch":"196868","validator_index":"505597"},"signature":"0xhexsig"}}
```

This is a backward incompatible change and will affect Siren as well.
@bors
Copy link

bors bot commented Sep 22, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Update the voluntary exit endpoint to comply with the key manager specification [Merged by Bors] - Update the voluntary exit endpoint to comply with the key manager specification Sep 22, 2023
@bors bors bot closed this Sep 22, 2023
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
…cification (sigp#4679)

## Issue Addressed

sigp#4635 

## Proposed Changes

Wrap the `SignedVoluntaryExit` object in a `GenericResponse` container, adding an additional `data` layer, to ensure compliance with the key manager API specification.

The new response would look like this:

```json
{"data":{"message":{"epoch":"196868","validator_index":"505597"},"signature":"0xhexsig"}}
```

This is a backward incompatible change and will affect Siren as well.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
…cification (sigp#4679)

## Issue Addressed

sigp#4635 

## Proposed Changes

Wrap the `SignedVoluntaryExit` object in a `GenericResponse` container, adding an additional `data` layer, to ensure compliance with the key manager API specification.

The new response would look like this:

```json
{"data":{"message":{"epoch":"196868","validator_index":"505597"},"signature":"0xhexsig"}}
```

This is a backward incompatible change and will affect Siren as well.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
…cification (sigp#4679)

## Issue Addressed

sigp#4635 

## Proposed Changes

Wrap the `SignedVoluntaryExit` object in a `GenericResponse` container, adding an additional `data` layer, to ensure compliance with the key manager API specification.

The new response would look like this:

```json
{"data":{"message":{"epoch":"196868","validator_index":"505597"},"signature":"0xhexsig"}}
```

This is a backward incompatible change and will affect Siren as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompat Backwards-incompatible API change HTTP-API ready-for-merge This PR is ready to merge. v4.5.0 ETA Q4 2023 val-client Relates to the validator client binary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants