From 38f80119639df46aa9899949735de45e47b0da5d Mon Sep 17 00:00:00 2001 From: Lane Rettig Date: Tue, 19 Sep 2023 15:16:49 -0400 Subject: [PATCH 1/2] Require -numUnits in postcli Before performing a potentially destructive action such as initialization --- cmd/postcli/main.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/cmd/postcli/main.go b/cmd/postcli/main.go index d2a3326f9..dd464fdfb 100644 --- a/cmd/postcli/main.go +++ b/cmd/postcli/main.go @@ -47,6 +47,9 @@ var ( commitmentAtxId []byte reset bool + numUnits uint64 + defaultNumUnits bool + logLevel zapcore.Level ErrKeyFileExists = errors.New("key file already exists") @@ -72,7 +75,7 @@ func parseFlags() { flag.BoolVar(&reset, "reset", false, "whether to reset the datadir before starting") flag.StringVar(&idHex, "id", "", "miner's id (public key), in hex (will be auto-generated if not provided)") flag.StringVar(&commitmentAtxIdHex, "commitmentAtxId", "", "commitment atx id, in hex (required)") - numUnits := flag.Uint64("numUnits", uint64(opts.NumUnits), "number of units") + flag.Uint64Var(&numUnits, "numUnits", 0, "number of units") flag.IntVar(&opts.FromFileIdx, "fromFile", 0, "index of the first file to init (inclusive)") var to int @@ -89,10 +92,22 @@ func parseFlags() { *opts.ProviderID = uint32(providerID) } - opts.NumUnits = uint32(*numUnits) // workaround the missing type support for uint32 + // record whether the user specified numUnits + if numUnits == 0 { + defaultNumUnits = true + } else { + opts.NumUnits = uint32(numUnits) // workaround the missing type support for uint32 + } } func processFlags() error { + // we require the user to explicitly pass numunits to avoid erasing existing data + if defaultNumUnits { + return fmt.Errorf("-numUnits must be specified to perform initialization. to use the default value, "+ + "run with -numUnits %d. note: if there's more than this amount of data on disk, "+ + "THIS WILL ERASE EXISTING DATA. MAKE ABSOLUTELY SURE YOU SPECIFY THE CORRECT VALUE", opts.NumUnits) + } + if opts.ProviderID == nil { return errors.New("-provider flag is required") } @@ -206,7 +221,7 @@ func main() { case errors.Is(err, ErrKeyFileExists): log.Fatalln("cli: key file already exists. This appears to be a mistake. If you're trying to initialize a new identity delete key.bin and try again otherwise specify identity with `-id` flag") case err != nil: - log.Fatalln("failed to process flags", err) + log.Fatalln("failed to process flags:", err) } init, err := initialization.NewInitializer( From 9f47dc035feb1428c3a873ddeee8b4a5c04bfbaf Mon Sep 17 00:00:00 2001 From: Lane Rettig Date: Tue, 19 Sep 2023 15:50:05 -0400 Subject: [PATCH 2/2] Improve logic Explicitly check whether value has been set. In previous code user could still pass -numUnits 0. --- cmd/postcli/main.go | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/cmd/postcli/main.go b/cmd/postcli/main.go index dd464fdfb..26e71f0d5 100644 --- a/cmd/postcli/main.go +++ b/cmd/postcli/main.go @@ -13,6 +13,7 @@ import ( "os" "os/signal" "path/filepath" + "strconv" "github.com/davecgh/go-spew/spew" "go.uber.org/zap" @@ -28,6 +29,27 @@ import ( const edKeyFileName = "key.bin" +type numUnitsFlag struct { + set bool + value uint32 +} + +func (nu *numUnitsFlag) Set(s string) error { + val, err := strconv.ParseUint(s, 10, 32) + if err != nil { + return err + } + *nu = numUnitsFlag{ + set: true, + value: uint32(val), + } + return nil +} + +func (nu *numUnitsFlag) String() string { + return fmt.Sprintf("%d", nu.value) +} + var ( cfg = config.MainnetConfig() opts = config.MainnetInitOpts() @@ -46,9 +68,7 @@ var ( commitmentAtxIdHex string commitmentAtxId []byte reset bool - - numUnits uint64 - defaultNumUnits bool + numUnits numUnitsFlag logLevel zapcore.Level @@ -75,7 +95,7 @@ func parseFlags() { flag.BoolVar(&reset, "reset", false, "whether to reset the datadir before starting") flag.StringVar(&idHex, "id", "", "miner's id (public key), in hex (will be auto-generated if not provided)") flag.StringVar(&commitmentAtxIdHex, "commitmentAtxId", "", "commitment atx id, in hex (required)") - flag.Uint64Var(&numUnits, "numUnits", 0, "number of units") + flag.Var(&numUnits, "numUnits", "number of units") flag.IntVar(&opts.FromFileIdx, "fromFile", 0, "index of the first file to init (inclusive)") var to int @@ -92,17 +112,14 @@ func parseFlags() { *opts.ProviderID = uint32(providerID) } - // record whether the user specified numUnits - if numUnits == 0 { - defaultNumUnits = true - } else { - opts.NumUnits = uint32(numUnits) // workaround the missing type support for uint32 + if numUnits.set { + opts.NumUnits = numUnits.value } } func processFlags() error { // we require the user to explicitly pass numunits to avoid erasing existing data - if defaultNumUnits { + if !numUnits.set { return fmt.Errorf("-numUnits must be specified to perform initialization. to use the default value, "+ "run with -numUnits %d. note: if there's more than this amount of data on disk, "+ "THIS WILL ERASE EXISTING DATA. MAKE ABSOLUTELY SURE YOU SPECIFY THE CORRECT VALUE", opts.NumUnits)