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

[STACKED] Command to generate presigned exit message #55

Merged
merged 3 commits into from
Aug 23, 2024

Conversation

mksh
Copy link
Contributor

@mksh mksh commented Aug 21, 2024

Based on cleanup/refactor introduced in #54, create command to generate presigned exit message based on existing mnemonic

@mksh mksh marked this pull request as ready for review August 21, 2024 00:43
@mksh mksh changed the title Command to generate presigned exit message [STACKED] Command to generate presigned exit message Aug 21, 2024
### Command to send SignedBLSToExecutionChange request to Beacon node

```
curl -H "Content-Type: application/json" -d '{
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we showing curl command instead of cli cmd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The command right now does not support sending output to beacon node; it should be easy to add though given openssl-sys is already a dependency. I will do it in #54 as this PR only touches presigned exit message, but bls-to-execution-change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sending to beacon node was implemented in 64ef067

README.md Outdated
./target/debug/eth-staking-smith presigned-exit-message --chain mainnet --mnemonic "entire habit bottom mention spoil clown finger wheat motion fox axis mechanic country make garment bar blind stadium sugar water scissors canyon often ketchup" --validator_start_index 0 --validator_index 100 --epoch 300000
```

Note that --validator-index and --validator-start-index are two distinct parameter, the former being index of validator on Beacon chain, and the latter is the index of validator private key derived from the seed
Copy link
Contributor

Choose a reason for hiding this comment

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

hm can we not just rename them to make it more explicit? --validator-beacon-index or --validator_seed_index

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, renamed in 176cfba, will rename in bls-to-execution-change in nested PR

Copy link
Contributor

@jenpaff jenpaff left a comment

Choose a reason for hiding this comment

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

lgtm! i would consider renaming arguments to make them easily understandable as mentioned in the comments

@mksh mksh merged commit f4fb8fb into fixes-bls-to-execution-change Aug 23, 2024
2 checks passed
@mksh mksh deleted the voluntary-exit-message branch August 23, 2024 13:19
mksh added a commit that referenced this pull request Aug 23, 2024
…t-message (#54)

* Make bls-to-execution-change command work

* [STACKED] Command to generate presigned exit message (#55)

* Command to generate presigned exit message

* Docs for presigned-exit-message

* Code review feedback

* Add support for sending signed payloads to beacon node

* Add note on backwards compatibility policy

* Unify regex usage

* Adjustments for single payload cmds

- Fix presigned-exit-message format & beacon node sending
- Rename Operator traits -> validator
- Rename operator module -> operations

* Fix README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants