Skip to content

feat: presign for validator account #6747

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
merged 4 commits into from
Apr 24, 2025
Merged

Conversation

hamdiallam
Copy link

@hamdiallam hamdiallam commented Dec 30, 2024

Issue Addressed

#6746

Proposed Changes

Add a --presign flag to emit the json output to stdout instead of publishing the exit

Additional Info

The validator-manager is also a surface area to support via the http client interface

@CLAassistant
Copy link

CLAassistant commented Dec 30, 2024

CLA assistant check
All committers have signed the CLA.

@chong-he chong-he added the ready-for-review The code is ready for review label Jan 13, 2025
@chong-he chong-he self-requested a review January 13, 2025 02:46
Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I have tested it and the feature is working. I have left some comments.

The CI check on formatting failed, also it would be great if you can sign the CLA.

I think it will also be nice to have a note highlighting the feature in the documentation, maybe before this section: https://lighthouse-book.sigmaprime.io/voluntary-exit.html#initiating-a-voluntary-exit

Thanks!

let string_output = serde_json::to_string_pretty(&signed_voluntary_exit)
.map_err(|e| format!("Unable to convert to JSON: {}", e))?;

println!("{}", string_output);
Copy link
Member

Choose a reason for hiding this comment

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

While working on #6612 , initially I also printed out the signature. But then I realise it probably isn't much useful in practice - if users want to pre-sign an exit, they probably want the signature saved to a file. So I modified in this commit in that PR:
5a4f928

We can hear from others about this and edit accordingly.

Copy link
Member

@michaelsproul michaelsproul Apr 23, 2025

Choose a reason for hiding this comment

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

I think this is fine for now. Users can still pipe stdout to a file

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Feb 10, 2025
@michaelsproul
Copy link
Member

Marking this waiting-on-author as we need @hamdiallam to please sign the contributor agreement 🙏

@hamdiallam
Copy link
Author

Ah sorry for dropping the ball on this! Will address when I get home tonight

@hamdiallam
Copy link
Author

done!

@chong-he chong-he added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Feb 13, 2025
@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 23, 2025
mergify bot added a commit that referenced this pull request Apr 24, 2025
@mergify mergify bot merged commit 6fad186 into sigp:unstable Apr 24, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants