Tools: Add 'algokey part keyreg' #3689
Conversation
|
what is a usage example? |
|
@rotemh please see the updated description for the help output. For a real example check out the e2e test: https://github.com/algorand/go-algorand/pull/3689/files#diff-41f523fc4a0e8cc8e6e8df028f9e5858ab083bcba4711d6328d7e3873cc192dcR24 |
cmd/algokey/keyreg.go
Outdated
| keyregCmd.Flags().StringVar(¶ms.addr, "account", "", "account address to bring offline, mutually exclusive with partkey-file") | ||
|
|
||
| // TODO: move 'bundleGenesisInject' into something that can be imported here instead of using constants. | ||
| validNetworks = map[string]networkGenesis{ |
There was a problem hiding this comment.
instead of hard-coding this, do this dynamically similar to .go-algorand/cmd/goal/node.go
There was a problem hiding this comment.
That command is also hard coded:
validNetworks := map[string][]byte{
"mainnet": genesisMainnet,
"testnet": genesisTestnet,
"betanet": genesisBetanet,
"devnet": genesisDevnet,
}
The reason I didn't use those objects is that they aren't readily accessible and I didn't want to refactor anything as part of this PR.
cmd/algokey/keyreg.go
Outdated
| } | ||
|
|
||
| keyregCmd.Flags().Uint64Var(¶ms.fee, "fee", minFee, "transaction fee") | ||
| keyregCmd.Flags().Uint64Var(¶ms.firstValid, "first-valid", 0, "first round where the transaction may be committed to the ledger") |
There was a problem hiding this comment.
please use the same parameter names ( when possible ), as in goal to retain some level of consistency. In this case first-valid -> firstvalid
There was a problem hiding this comment.
Thanks. It looks like there are 2 problems with this.
- algokey uses
firstandlastto define the key ranges. Technically this is slightly different, being the transaction ranges, so I'll keep consistent with goal on that one. - algokey refers to the key file as
keyfileandgoal account installpartkeyrefers to it aspartkey. For this I'll keep consistent with algokey.
cmd/algokey/keyreg.go
Outdated
| keyregCmd.Flags().StringVar(¶ms.network, "network", "mainnet", "the network where the provided keys will be registered, one of mainnet/testnet/betanet") | ||
| keyregCmd.MarkFlagRequired("network") // nolint:errcheck | ||
| keyregCmd.Flags().BoolVar(¶ms.offline, "offline", false, "set to bring an account offline") | ||
| keyregCmd.Flags().StringVar(¶ms.txFile, "tx-file", "", fmt.Sprintf("write signed transaction to this file, or '%s' to write to stdout", stdoutFilenameValue)) |
There was a problem hiding this comment.
using dynamic parameter output could cause issues with the automated documentation generator for these tools.
There was a problem hiding this comment.
I double-checked the output of algokey generate-docs <dir>. This is not a problem.
cmd/algokey/keyreg.go
Outdated
| part = &partkey.Participation | ||
|
|
||
| if params.firstValid < uint64(part.FirstValid) { | ||
| return fmt.Errorf("first-valid (%d) is earlier than the key first valid (%d)", params.firstValid, part.FirstValid) |
There was a problem hiding this comment.
could you please rephrase the error message so that it would be clear what is the expectation and what was violated ?
There was a problem hiding this comment.
( same issue with the rest of the error messages below )
There was a problem hiding this comment.
I'll reword this one, but the others look good to me
tsachiherman
left a comment
tsachiherman
left a comment
There was a problem hiding this comment.
see requests above.
Codecov Report
@@ Coverage Diff @@
## master #3689 +/- ##
==========================================
- Coverage 49.91% 49.91% -0.01%
==========================================
Files 382 382
Lines 64412 64412
==========================================
- Hits 32150 32149 -1
Misses 28832 28832
- Partials 3430 3431 +1
Continue to review full report at Codecov.
|
8 participants
Summary
New algokey subcommand to generate transactions for bringing an account online or offline.
Usage:
Test Plan
New e2e test.