WIP multisign cli command #6185
Conversation
jaekwon
requested review from
aaronc,
alessio,
alexanderbez,
clevinson,
fedekunze,
hschoenburg,
jgimeno and
nylira
as code owners
|
👋 Thanks for creating a PR! Before we can merge this PR, please make sure that all the following items have been For contributor use:
For admin use:
Thank you for your contribution to the Cosmos-SDK! 🚀 |
| @@ -37,6 +37,8 @@ const ( | |||
| flagKeyAlgo = "algo" | |||
|
|
|||
| // DefaultKeyPass contains the default key password for genesis transactions | |||
| // XXX What is this?! | |||
| // It's being used for the signing cli! | |||
There was a problem hiding this comment.
It's no longer used. With the new keyring passwords are handled outside of the SDK
| @@ -352,6 +352,7 @@ func PrepareFactory(ctx context.CLIContext, txf Factory) (Factory, error) { | |||
| // | |||
| // Note, It is assumed the Factory has the necessary fields set that are required | |||
| // by the CanonicalSignBytes call. | |||
| // XXX The passphrase isn't even being used... | |||
There was a problem hiding this comment.
That's correct, see my previous comment
|
|
||
| Read signature(s) from [signature] file(s), generate a multisig signature compliant to the | ||
| multisig key [name], and attach it to the transaction read from [file]. | ||
| func GetMultiSignCommand(codec *codec.Codec) *cobra.Command { |
There was a problem hiding this comment.
I think we should call this command something different. multisign exists already and is used to build a mulsig signature from multiple signatures
There was a problem hiding this comment.
What if we called it batch-sign or sign-batch?
| @@ -332,6 +332,7 @@ func (ctx CLIContext) PrintOutput(toPrint interface{}) error { | |||
| // GetFromFields returns a from account address and Keybase name given either | |||
| // an address or key name. If genOnly is true, only a valid Bech32 cosmos | |||
| // address is returned. | |||
| // XXX What does genOnly mean? Why is it called that? | |||
There was a problem hiding this comment.
genOnly == GenerateOnly. The godoc states its use. I'd advise against adding a XXX and actually creating an issue to revise the API.
|
|
||
| Read signature(s) from [signature] file(s), generate a multisig signature compliant to the | ||
| multisig key [name], and attach it to the transaction read from [file]. | ||
| func GetMultiSignCommand(codec *codec.Codec) *cobra.Command { |
|
|
| return parseStdSignDocs(string(body)) | ||
| } | ||
|
|
||
| func parseStdSignDocs(lines string) (docs []types.StdSignDoc) { |
There was a problem hiding this comment.
--generate-only generates StdTxs. This command seems to be expecting types.StdSignDocs as input instead.
I'm not sure it would work (unless we amend --generate-only's behaviour)
|
#6350 closes this |
We need a way to sign multiple transactions at once, as we will be signing a lot of transactions.
This should run on cosmoshub-3 so release/v0.37.4 may not be appropriate, but I'm not sure.
We may need to back-port this to v0.37.4. I meant to do so but made a mistake during git checkout.
In other words, this is a temporary draft PR while we discuss what to do going forward.