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

*: add --all for exit sign command #3272

Merged
merged 1 commit into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/obolapi/exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ func fullExitURL(valPubkey, lockHash string, shareIndex uint64) string {
).Replace(fullExitTmpl)
}

// PostPartialExit POSTs the set of msg's to the Obol API, for a given lock hash.
// PostPartialExits POSTs the set of msg's to the Obol API, for a given lock hash.
// It respects the timeout specified in the Client instance.
func (c Client) PostPartialExit(ctx context.Context, lockHash []byte, shareIndex uint64, identityKey *k1.PrivateKey, exitBlobs ...ExitBlob) error {
func (c Client) PostPartialExits(ctx context.Context, lockHash []byte, shareIndex uint64, identityKey *k1.PrivateKey, exitBlobs ...ExitBlob) error {
lockHashStr := "0x" + hex.EncodeToString(lockHash)

path := partialExitURL(lockHashStr)
Expand Down
4 changes: 2 additions & 2 deletions app/obolapi/exit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestAPIFlow(t *testing.T) {

// send all the partial exits
for idx, exit := range exits {
require.NoError(t, cl.PostPartialExit(ctx, lock.LockHash, uint64(idx+1), identityKeys[idx], exit), "share index: %d", idx+1)
require.NoError(t, cl.PostPartialExits(ctx, lock.LockHash, uint64(idx+1), identityKeys[idx], exit), "share index: %d", idx+1)
}

for idx := range exits {
Expand Down Expand Up @@ -188,7 +188,7 @@ func TestAPIFlowMissingSig(t *testing.T) {

// send all the partial exits
for idx, exit := range exits {
require.NoError(t, cl.PostPartialExit(ctx, lock.LockHash, uint64(idx+1), identityKeys[idx], exit), "share index: %d", idx+1)
require.NoError(t, cl.PostPartialExits(ctx, lock.LockHash, uint64(idx+1), identityKeys[idx], exit), "share index: %d", idx+1)
}

for idx := range exits {
Expand Down
8 changes: 8 additions & 0 deletions cmd/exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
BeaconNodeTimeout time.Duration
ExitFromFilePath string
Log log.Config
All bool
}

func newExitCmd(cmds ...*cobra.Command) *cobra.Command {
Expand Down Expand Up @@ -63,6 +64,7 @@
fetchedExitPath
publishTimeout
validatorIndex
all
)

func (ef exitFlag) String() string {
Expand Down Expand Up @@ -91,6 +93,8 @@
return "publish-timeout"
case validatorIndex:
return "validator-index"
case all:
return "all"

Check warning on line 97 in cmd/exit.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit.go#L96-L97

Added lines #L96 - L97 were not covered by tests
default:
return "unknown"
}
Expand All @@ -113,6 +117,7 @@
return s
}

//nolint:exhaustive // `all` is not yet implemented
switch flag {
case publishAddress:
cmd.Flags().StringVar(&config.PublishAddress, publishAddress.String(), "https://api.obol.tech/v1", maybeRequired("The URL of the remote API."))
Expand All @@ -138,6 +143,9 @@
cmd.Flags().DurationVar(&config.PublishTimeout, publishTimeout.String(), 30*time.Second, "Timeout for publishing a signed exit to the publish-address API.")
case validatorIndex:
cmd.Flags().Uint64Var(&config.ValidatorIndex, validatorIndex.String(), 0, "Validator index of the validator to exit, the associated public key must be present in the cluster lock manifest. If --validator-public-key is also provided, validator existence won't be checked on the beacon chain.")
// TODO: enable after all functionalities for --all are ready
// case all:
// cmd.Flags().BoolVar(&config.All, all.String(), false, "Exit all currently active validators in the cluster.")
}

if f.required {
Expand Down
130 changes: 103 additions & 27 deletions cmd/exit_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"fmt"

eth2api "github.com/attestantio/go-eth2-client/api"
eth2v1 "github.com/attestantio/go-eth2-client/api/v1"
eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0"
libp2plog "github.com/ipfs/go-log/v2"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -52,6 +53,7 @@
{beaconNodeEndpoints, true},
{beaconNodeTimeout, false},
{publishTimeout, false},
{all, false},

Check warning on line 56 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L56

Added line #L56 was not covered by tests
})

bindLogFlags(cmd.Flags(), &config.Log)
Expand All @@ -60,11 +62,16 @@
valIdxPresent := cmd.Flags().Lookup(validatorIndex.String()).Changed
valPubkPresent := cmd.Flags().Lookup(validatorPubkey.String()).Changed

if !valPubkPresent && !valIdxPresent {
if !valPubkPresent && !valIdxPresent && !config.All {

Check warning on line 65 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L65

Added line #L65 was not covered by tests
//nolint:revive // we use our own version of the errors package.
return errors.New(fmt.Sprintf("either %s or %s must be specified at least.", validatorIndex.String(), validatorPubkey.String()))
}

if config.All && (valIdxPresent || valPubkPresent) {
//nolint:revive // we use our own version of the errors package.
return errors.New(fmt.Sprintf("%s or %s should not be specified when %s is, as they are obsolete and misleading.", validatorIndex.String(), validatorPubkey.String(), all.String()))
}

Check warning on line 73 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L70-L73

Added lines #L70 - L73 were not covered by tests

config.ValidatorIndexPresent = valIdxPresent
config.SkipBeaconNodeCheck = valIdxPresent && valPubkPresent

Expand Down Expand Up @@ -126,60 +133,119 @@
log.Info(ctx, "Both public key and index are specified, beacon node won't be checked for validator existence/liveness")
}

var exitBlobs []obolapi.ExitBlob
if config.All {
exitBlobs, err = signAllValidatorsExits(ctx, config, eth2Cl, shares)
if err != nil {
return errors.Wrap(err, "could not sign exits for all validators")
}

Check warning on line 141 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L140-L141

Added lines #L140 - L141 were not covered by tests
} else {
exitBlobs, err = signSingleValidatorExit(ctx, config, eth2Cl, shares)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use signAllValidatorsExits here, and instead of passing all shares we pass a slice with the single share we're interested in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've been thinking about that, but when we are signing for single validator we are checking the config if the user passed pubkey or index or both, in exit all scenario we don't have none of those. We can still use the same function for both cases, but I will have to add some flags and checks which to determine if we have to check the config or not. It will make the code less readable imo.

if err != nil {
return errors.Wrap(err, "could not sign exit for validator")
}
}

if err := oAPI.PostPartialExits(ctx, cl.GetInitialMutationHash(), shareIdx, identityKey, exitBlobs...); err != nil {
return errors.Wrap(err, "could not POST partial exit message to Obol API")
}

Check warning on line 151 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L150-L151

Added lines #L150 - L151 were not covered by tests

return nil
}

func signSingleValidatorExit(ctx context.Context, config exitConfig, eth2Cl eth2wrap.Client, shares keystore.ValidatorShares) ([]obolapi.ExitBlob, error) {
valEth2, err := fetchValidatorBLSPubKey(ctx, config, eth2Cl)
if err != nil {
return errors.Wrap(err, "cannot fetch validator public key")
return nil, errors.Wrap(err, "cannot fetch validator public key")
}

validator := core.PubKeyFrom48Bytes(valEth2)

ourShare, ok := shares[validator]
if !ok {
return errors.New("validator not present in cluster lock", z.Str("validator", validator.String()))
return nil, errors.New("validator not present in cluster lock", z.Str("validator", validator.String()))
}

valIndex, err := fetchValidatorIndex(ctx, config, eth2Cl)
if err != nil {
return errors.Wrap(err, "cannot fetch validator index")
return nil, errors.Wrap(err, "cannot fetch validator index")

Check warning on line 171 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L171

Added line #L171 was not covered by tests
}

log.Info(ctx, "Signing exit message for validator")

exitMsg, err := signExit(ctx, eth2Cl, valIndex, ourShare.Share, eth2p0.Epoch(config.ExitEpoch))
if err != nil {
return errors.Wrap(err, "cannot sign partial exit message")
return nil, errors.Wrap(err, "cannot sign partial exit message")

Check warning on line 178 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L178

Added line #L178 was not covered by tests
}

exitBlob := obolapi.ExitBlob{
PublicKey: valEth2.String(),
SignedExitMessage: exitMsg,
return []obolapi.ExitBlob{
{
PublicKey: valEth2.String(),
SignedExitMessage: exitMsg,
},
}, nil
}

func signAllValidatorsExits(ctx context.Context, config exitConfig, eth2Cl eth2wrap.Client, shares keystore.ValidatorShares) ([]obolapi.ExitBlob, error) {
var valsEth2 []eth2p0.BLSPubKey
for pk := range shares {
eth2PK, err := pk.ToETH2()
if err != nil {
return nil, errors.Wrap(err, "cannot convert core pubkey to eth2 pubkey")
}

Check warning on line 195 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L194-L195

Added lines #L194 - L195 were not covered by tests
valsEth2 = append(valsEth2, eth2PK)
}

if err := oAPI.PostPartialExit(ctx, cl.GetInitialMutationHash(), shareIdx, identityKey, exitBlob); err != nil {
return errors.Wrap(err, "could not POST partial exit message to Obol API")
rawValData, err := queryBeaconForValidator(ctx, eth2Cl, valsEth2, nil)
if err != nil {
return nil, errors.Wrap(err, "fetch validator indices from beacon")

Check warning on line 201 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L201

Added line #L201 was not covered by tests
}

return nil
for _, val := range rawValData.Data {
share, ok := shares[core.PubKeyFrom48Bytes(val.Validator.PublicKey)]
if !ok {
//nolint:revive // we use our own version of the errors package.
return nil, errors.New(fmt.Sprintf("validator public key %s not found in cluster lock", val.Validator.PublicKey))
}

Check warning on line 209 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L207-L209

Added lines #L207 - L209 were not covered by tests
share.Index = int(val.Index)
shares[core.PubKeyFrom48Bytes(val.Validator.PublicKey)] = share
}

log.Info(ctx, "Signing exit message for all validators")

var exitBlobs []obolapi.ExitBlob
for pk, share := range shares {
exitMsg, err := signExit(ctx, eth2Cl, eth2p0.ValidatorIndex(share.Index), share.Share, eth2p0.Epoch(config.ExitEpoch))
if err != nil {
return nil, errors.Wrap(err, "cannot sign partial exit message")
}

Check warning on line 221 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L220-L221

Added lines #L220 - L221 were not covered by tests
eth2PK, err := pk.ToETH2()
if err != nil {
return nil, errors.Wrap(err, "cannot convert core pubkey to eth2 pubkey")
}

Check warning on line 225 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L224-L225

Added lines #L224 - L225 were not covered by tests
exitBlob := obolapi.ExitBlob{
PublicKey: eth2PK.String(),
SignedExitMessage: exitMsg,
}
exitBlobs = append(exitBlobs, exitBlob)
}

return exitBlobs, nil
}

func fetchValidatorBLSPubKey(ctx context.Context, config exitConfig, eth2Cl eth2wrap.Client) (eth2p0.BLSPubKey, error) {
if config.ValidatorPubkey != "" {
valEth2, err := core.PubKey(config.ValidatorPubkey).ToETH2()
if err != nil {
return eth2p0.BLSPubKey{}, errors.Wrap(err, "cannot convert validator pubkey to bytes")
return eth2p0.BLSPubKey{}, errors.Wrap(err, "cannot convert core pubkey to eth2 pubkey")
}

return valEth2, nil
}

valAPICallOpts := &eth2api.ValidatorsOpts{
State: "head",
Indices: []eth2p0.ValidatorIndex{eth2p0.ValidatorIndex(config.ValidatorIndex)},
}

rawValData, err := eth2Cl.Validators(ctx, valAPICallOpts)
rawValData, err := queryBeaconForValidator(ctx, eth2Cl, nil, []eth2p0.ValidatorIndex{eth2p0.ValidatorIndex(config.ValidatorIndex)})
if err != nil {
return eth2p0.BLSPubKey{}, errors.Wrap(err, "cannot fetch validators")
return eth2p0.BLSPubKey{}, errors.Wrap(err, "fetch validator pubkey from beacon")

Check warning on line 248 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L248

Added line #L248 was not covered by tests
}

for _, val := range rawValData.Data {
Expand All @@ -198,17 +264,12 @@

valEth2, err := core.PubKey(config.ValidatorPubkey).ToETH2()
if err != nil {
return 0, errors.Wrap(err, "cannot convert validator pubkey to bytes")
}

valAPICallOpts := &eth2api.ValidatorsOpts{
State: "head",
PubKeys: []eth2p0.BLSPubKey{valEth2},
return 0, errors.Wrap(err, "cannot convert core pubkey to eth2 pubkey")

Check warning on line 267 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L267

Added line #L267 was not covered by tests
}

rawValData, err := eth2Cl.Validators(ctx, valAPICallOpts)
rawValData, err := queryBeaconForValidator(ctx, eth2Cl, []eth2p0.BLSPubKey{valEth2}, nil)
if err != nil {
return 0, errors.Wrap(err, "cannot fetch validators")
return 0, errors.Wrap(err, "cannot fetch validator index from beacon")

Check warning on line 272 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L272

Added line #L272 was not covered by tests
}

for _, val := range rawValData.Data {
Expand All @@ -219,3 +280,18 @@

return 0, errors.New("validator public key not found in beacon node response")
}

func queryBeaconForValidator(ctx context.Context, eth2Cl eth2wrap.Client, pubKeys []eth2p0.BLSPubKey, indices []eth2p0.ValidatorIndex) (*eth2api.Response[map[eth2p0.ValidatorIndex]*eth2v1.Validator], error) {
valAPICallOpts := &eth2api.ValidatorsOpts{
State: "head",
PubKeys: pubKeys,
Indices: indices,
}

rawValData, err := eth2Cl.Validators(ctx, valAPICallOpts)
if err != nil {
return nil, errors.Wrap(err, "fetch validators from beacon")
}

Check warning on line 294 in cmd/exit_sign.go

View check run for this annotation

Codecov / codecov/patch

cmd/exit_sign.go#L293-L294

Added lines #L293 - L294 were not covered by tests

return rawValData, nil
}
23 changes: 16 additions & 7 deletions cmd/exit_sign_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ func Test_runSubmitPartialExit(t *testing.T) {
false,
"test",
0,
"cannot convert validator pubkey to bytes",
"cannot convert core pubkey to eth2 pubkey",
false,
)
})

Expand All @@ -78,6 +79,7 @@ func Test_runSubmitPartialExit(t *testing.T) {
testutil.RandomEth2PubKey(t).String(),
0,
"validator not present in cluster lock",
false,
)
})

Expand All @@ -89,6 +91,7 @@ func Test_runSubmitPartialExit(t *testing.T) {
"",
9999,
"validator index not found in beacon node response",
false,
)
})

Expand All @@ -99,7 +102,8 @@ func Test_runSubmitPartialExit(t *testing.T) {
true,
"test",
9999,
"cannot convert validator pubkey to bytes",
"cannot convert core pubkey to eth2 pubkey",
false,
)
})

Expand All @@ -111,23 +115,27 @@ func Test_runSubmitPartialExit(t *testing.T) {
testutil.RandomEth2PubKey(t).String(),
9999,
"validator not present in cluster lock",
false,
)
})

t.Run("main flow with pubkey", func(t *testing.T) {
runSubmitPartialExitFlowTest(t, false, false, "", 0, "")
runSubmitPartialExitFlowTest(t, false, false, "", 0, "", false)
})
t.Run("main flow with validator index", func(t *testing.T) {
runSubmitPartialExitFlowTest(t, true, false, "", 0, "")
runSubmitPartialExitFlowTest(t, true, false, "", 0, "", false)
})
t.Run("main flow with skipBeaconNodeCheck mode", func(t *testing.T) {
runSubmitPartialExitFlowTest(t, true, true, "", 0, "")
runSubmitPartialExitFlowTest(t, true, true, "", 0, "", false)
})
t.Run("main flow with all mode", func(t *testing.T) {
runSubmitPartialExitFlowTest(t, false, false, "", 0, "", true)
})

t.Run("config", Test_runSubmitPartialExit_Config)
}

func runSubmitPartialExitFlowTest(t *testing.T, useValIdx bool, skipBeaconNodeCheck bool, valPubkey string, valIndex uint64, errString string) {
func runSubmitPartialExitFlowTest(t *testing.T, useValIdx bool, skipBeaconNodeCheck bool, valPubkey string, valIndex uint64, errString string, all bool) {
t.Helper()
t.Parallel()
ctx := context.Background()
Expand Down Expand Up @@ -202,6 +210,7 @@ func runSubmitPartialExitFlowTest(t *testing.T, useValIdx bool, skipBeaconNodeCh
ExitEpoch: 194048,
BeaconNodeTimeout: 30 * time.Second,
PublishTimeout: 10 * time.Second,
All: all,
}

index := uint64(0)
Expand Down Expand Up @@ -279,7 +288,7 @@ func Test_runSubmitPartialExit_Config(t *testing.T) {
{
name: "Bad validator address",
badValidatorAddr: true,
errData: "cannot convert validator pubkey to bytes",
errData: "cannot convert core pubkey to eth2 pubkey",
},
}

Expand Down
Loading