cmd: clean up create cluster deposit data #561
Conversation
Codecov Report
@@ Coverage Diff @@
## main #561 +/- ##
==========================================
- Coverage 55.26% 54.67% -0.59%
==========================================
Files 92 92
Lines 8675 8664 -11
==========================================
- Hits 4794 4737 -57
- Misses 3241 3287 +46
Partials 640 640
Continue to review full report at Codecov.
|
| @@ -94,7 +94,6 @@ type clusterConfig struct { | |||
| Clean bool | |||
|
|
|||
| NumNodes int | |||
| NumDVs int | |||
| @@ -168,6 +167,10 @@ func runCreateCluster(w io.Writer, conf clusterConfig) error { | |||
| } | |||
| } | |||
|
|
|||
| if err := validateClusterConfig(conf); err != nil { | |||
There was a problem hiding this comment.
fail fast: do validation at the start
| } | ||
|
|
||
| if err = writeDepositData(conf, pubkeys, msgSigs, conf.WithdrawalAddr, conf.Network); err != nil { | ||
| if err = writeDepositData(conf, secrets); err != nil { |
There was a problem hiding this comment.
do all deposit data stuff in one place
| var pubkeys []eth2p0.BLSPubKey | ||
| var msgSigs []eth2p0.BLSSignature | ||
| // signDepositDatas returns a map of deposit data signatures by DV pubkey. | ||
| func signDepositDatas(secrets []*bls_sig.SecretKey, withdrawalAddr string, network string) (map[eth2p0.BLSPubKey]eth2p0.BLSSignature, error) { |
There was a problem hiding this comment.
instead of two slices that are implicitly linked, explicitly link pubkeys and signatures in a single map
There was a problem hiding this comment.
noted! is this the preferred approach?
|
|
||
| for i := 0; i < numDVs; i++ { |
There was a problem hiding this comment.
use secrets directly (instead of another variable)
| return nil, nil, err | ||
| } | ||
|
|
||
| withdrawalAddr, err := checksumAddr(addr) |
There was a problem hiding this comment.
this only needs to be done once
| @@ -105,6 +106,10 @@ func MarshalDepositData(pubkeys []eth2p0.BLSPubKey, msgSigs []eth2p0.BLSSignatur | |||
| }) | |||
| } | |||
|
|
|||
| sort.Slice(ddList, func(i, j int) bool { | |||
| return ddList[i].PubKey < ddList[j].PubKey | |||
There was a problem hiding this comment.
make output deterministic
| @@ -1,4 +1,15 @@ | |||
| [ | |||
| { | |||
There was a problem hiding this comment.
had to reorder by pubkey
| @@ -362,15 +346,21 @@ func getKeys(conf clusterConfig, numDVs int) ([]*bls_sig.SecretKey, error) { | |||
| } | |||
|
|
|||
| // writeDepositData writes deposit data to disk for the DVs in a cluster. | |||
| func writeDepositData(config clusterConfig, pubkeys []eth2p0.BLSPubKey, msgSigs []eth2p0.BLSSignature, withdrawalAddr, network string) error { | |||
There was a problem hiding this comment.
withdrawal address and network already provided in config
corverroos
added
the
merge when ready
Small cleanups of deposit data integration in create cluster.
category: refactor
ticket: #549