-
Notifications
You must be signed in to change notification settings - Fork 40
feat: support moocow #177
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
base: master
Are you sure you want to change the base?
feat: support moocow #177
Conversation
* refactor: consolidate status print statements
* make verbose flag a per-command option instead of global (it was hidden when running subcommand help) * fix verbosity in status command * remove extraneous print in status command
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.
Minor comments, I think we should:
- Add a usage guide for getting relevant validators to pod (so we can pass in the indices)
- Give error to user if attempt to consolidate or withdraw with validator type that will be a no-op on beacon chain
eigenpodValidators, err := utils.GetEigenPodValidatorsByIndex(args.EigenpodAddress, headState) | ||
utils.PanicOnError("failed to fetch source validators for eigenpod", err) | ||
|
||
targetValidator, exists := eigenpodValidators[args.TargetValidator] |
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.
Shouldn't we assert target validator is 0x02?
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.
(For user convenience)
headState, err := utils.GetBeaconHeadState(ctx, beaconClient) | ||
utils.PanicOnError("failed to fetch beacon chain head state", err) | ||
|
||
eigenpodValidators, err := utils.GetEigenPodValidatorsByIndex(args.EigenpodAddress, headState) |
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.
Should we prevent 0x01 validators from partial requests?
// with the system address updating the fee between each block. | ||
// | ||
// The returned array is the "fee per request" for each chunk | ||
func EstimateWithdrawalFeePerChunk( |
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.
Realized we don't use this anywhere?
type TConsolidateToTargetCommandArgs struct { | ||
ConsolidateBaseCommandArgs | ||
|
||
TargetValidator uint64 |
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.
Do we publicly expose command for retrieving the indices pointed to a pod? FindAllValidatorsForEigenpod
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.
Update: we use the status
command here, which also prints out the withdrawal prefix. It would be nice to be able to say "get me all my 0x01 validators" for future consolidations, but not needed
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.
Can we also update the readme with usage guide in the PR description?
Adds support for upcoming v1.6.0 core contract release, allowing EigenPods to request consolidations and withdrawals.
See EigenPod notes, docs, etc in the relevant PR: Layr-Labs/eigenlayer-contracts#1375
TODO: it's quite difficult to test this at the moment, since the upgrade isn't live on testnet yet. I don't have time this week to spin up a foundry forked network / anvil environment, so for now I've done some manual validation but it's possible there are bugs.
New commands and usage examples follow.
Notes on Safety/Predeploy Fees
Quick note on batching. One of the main things I want to avoid with the CLI is footguns due to the predeploy's exponential fee increase. For example, if someone submits a ton of consolidations at the right time, my predeploy fee might suddenly be much higher than I expected. The CLI needs to guarantee the user an upper bound on the predeploy fee. For this reason:
--no-prompt
flag--fee-overestimate-factor
flag for each of the new commands. This is "1.5" by default, which will have the user send 1.5X the currently-measured predeploy fee when calling the contract. The user can tweak this if they want more/less wiggle room.Required Flags
Outside of arguments required for specific commands, all new commands require the following flags:
Consolidate - Switch
Pass in a list of validator indices. This will initiate switch requests to change each validator's withdrawal prefix from 0x01 to 0x02. In order to be a consolidation target, a validator must have 0x02 credentials.
Consolidate - Source to Target
Pass in a target index and a list of source indices. This will initiate consolidations from each source to the specified target.
Request Withdrawal - Full Exit
Pass in a list of validator indices. This will initiate full exits from the beacon chain.
Request Withdrawal - Partial
Pass in a list of validator indices and an equally-sized list of amounts in gwei. This will initiate partial withdrawals from the beacon chain. Note that this method will NOT allow
amountGwei == 0
, as that is a full exit. We want to be extra sure that CLI users are aware if they are initiating a full exit. Also note that in order to successfully process a partial withdrawal via this method, the beacon chain requires the validator to have 0x02 credentials. 0x01 validators can only full-exit.