From f58262c8b0aea6c1184714b4fbde72189d2ab6ac Mon Sep 17 00:00:00 2001 From: rhuairahrighairigh Date: Sat, 1 Sep 2018 22:22:50 -0400 Subject: [PATCH] improve command UX --- cmd/kvcli/main.go | 1 + internal/x/paychan/README.md | 4 + internal/x/paychan/client/cmd/cmd.go | 215 +++++++-------------------- internal/x/paychan/wire.go | 6 +- 4 files changed, 64 insertions(+), 162 deletions(-) diff --git a/cmd/kvcli/main.go b/cmd/kvcli/main.go index 06f2cc66..acde7bc0 100644 --- a/cmd/kvcli/main.go +++ b/cmd/kvcli/main.go @@ -141,6 +141,7 @@ func main() { paychanCmd.AddCommand( client.PostCommands( paychancmd.CreateChannelCmd(cdc), + paychancmd.GetChannelCmd(cdc, "paychan"), // pass in storeKey paychancmd.GeneratePaymentCmd(cdc), paychancmd.VerifyPaymentCmd(cdc, "paychan"), // pass in storeKey paychancmd.SubmitPaymentCmd(cdc), diff --git a/internal/x/paychan/README.md b/internal/x/paychan/README.md index 858c9217..c380ba21 100644 --- a/internal/x/paychan/README.md +++ b/internal/x/paychan/README.md @@ -11,10 +11,14 @@ Simplifications: - Tidy up - method descriptions, heading comments, remove uneccessary comments, README/docs - Find a better name for Queue - clarify distinction between int slice and abstract queue concept - write some sort of integration test + - possible bug in submitting same update repeatedly - find nicer name for payout - add Gas usage - add tags (return channel id on creation) - refactor cmds to be able to test them, then test them + - verify doesn’t throw json parsing error on invalid json + - can’t submit an update from an unitialised account + - pay without a --from returns confusing error - use custom errors instead of using sdk.ErrInternal - split off signatures from update as with txs/msgs - testing easier, code easier to use, doesn't store sigs unecessarily on chain - consider removing pubKey from UpdateSignature - instead let channel module access accountMapper diff --git a/internal/x/paychan/client/cmd/cmd.go b/internal/x/paychan/client/cmd/cmd.go index d10f4883..12dfd541 100644 --- a/internal/x/paychan/client/cmd/cmd.go +++ b/internal/x/paychan/client/cmd/cmd.go @@ -3,7 +3,6 @@ package cli import ( "fmt" "io/ioutil" - "os" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -34,7 +33,8 @@ func CreateChannelCmd(cdc *wire.Codec) *cobra.Command { // Create a "client context" stuct populated with info from common flags ctx := context.NewCoreContextFromViper().WithDecoder(authcmd.GetAccountDecoder(cdc)) - // ctx.PrintResponse = true TODO is this needed for channelID + // TODO is this needed for channelID + // ctx.PrintResponse = true // Get sender adress sender, err := ctx.GetFromAddress() @@ -80,26 +80,20 @@ func CreateChannelCmd(cdc *wire.Codec) *cobra.Command { } func GeneratePaymentCmd(cdc *wire.Codec) *cobra.Command { - flagId := "id" // ChannelID - flagReceiverAmount := "r-amount" // amount the receiver should received on closing the channel - flagSenderAmount := "s-amount" // + flagId := "chan-id" + flagReceiverAmount := "rec-amt" // amount the receiver should received on closing the channel + flagSenderAmount := "sen-amt" + flagPaymentFile := "filename" cmd := &cobra.Command{ Use: "pay", - Short: "Generate a .", // TODO descriptions - Long: "Generate a new ", + Short: "Generate a new payment.", // TODO descriptions + Long: "Generate a payment file (json) to send to the receiver as a payment.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { // Create a "client context" stuct populated with info from common flags ctx := context.NewCoreContextFromViper().WithDecoder(authcmd.GetAccountDecoder(cdc)) - // ctx.PrintResponse = false TODO is this needed to stop any other output messing up json? - - // Get sender adress - // senderAddress, err := ctx.GetFromAddress() - // if err != nil { - // return err - // } // Get the paychan id id := paychan.ChannelID(viper.GetInt64(flagId)) // TODO make this default to pulling id from chain @@ -143,28 +137,35 @@ func GeneratePaymentCmd(cdc *wire.Codec) *cobra.Command { CryptoSignature: sig, }} - // Print out the update + // Write out the update jsonUpdate, err := wire.MarshalJSONIndent(cdc, update) if err != nil { return err } - fmt.Println(string(jsonUpdate)) + paymentFile := viper.GetString(flagPaymentFile) + err = ioutil.WriteFile(paymentFile, jsonUpdate, 0644) + if err != nil { + return err + } + fmt.Printf("Written payment out to %v.\n", paymentFile) return nil }, } cmd.Flags().Int(flagId, 0, "ID of the payment channel.") - cmd.Flags().String(flagSenderAmount, "", "") - cmd.Flags().String(flagReceiverAmount, "", "") + cmd.Flags().String(flagSenderAmount, "", "Total coins to payout to sender on channel close.") + cmd.Flags().String(flagReceiverAmount, "", "Total coins to payout to sender on channel close.") + cmd.Flags().String(flagPaymentFile, "payment.json", "File name to write the payment into.") return cmd } func VerifyPaymentCmd(cdc *wire.Codec, paychanStoreName string) *cobra.Command { + flagPaymentFile := "payment" cmd := &cobra.Command{ Use: "verify", - Short: "", // TODO - Long: "", + Short: "Verify a payment file.", + Long: "Verify that a received payment can be used to close a channel.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { @@ -172,7 +173,7 @@ func VerifyPaymentCmd(cdc *wire.Codec, paychanStoreName string) *cobra.Command { ctx := context.NewCoreContextFromViper() // read in update - bz, err := ioutil.ReadAll(os.Stdin) + bz, err := ioutil.ReadFile(viper.GetString(flagPaymentFile)) if err != nil { // TODO add nice message about how to feed in stdin return err @@ -190,24 +191,30 @@ func VerifyPaymentCmd(cdc *wire.Codec, paychanStoreName string) *cobra.Command { cdc.MustUnmarshalBinary(res, &channel) //verify - updateIsOK := paychan.VerifyUpdate(channel, update) + verificationError := paychan.VerifyUpdate(channel, update) // print result - fmt.Println(updateIsOK) - + if verificationError == nil { + fmt.Printf("Payment is valid for channel '%d'.\n", update.ChannelID) + } else { + fmt.Printf("Payment is NOT valid for channel '%d'.\n", update.ChannelID) + fmt.Println(verificationError) + } return nil }, } + cmd.Flags().String(flagPaymentFile, "payment.json", "File name to read the payment from.") return cmd } func SubmitPaymentCmd(cdc *wire.Codec) *cobra.Command { + flagPaymentFile := "payment" cmd := &cobra.Command{ Use: "submit", - Short: "", - Long: "", + Short: "Submit a payment to the blockchain to close the channel.", + Long: fmt.Sprintf("Submit a payment to the blockchain to either close a channel immediately (if you are the receiver) or after a dispute period of %d blocks (if you are the sender).", paychan.ChannelDisputeTime), Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { @@ -222,7 +229,7 @@ func SubmitPaymentCmd(cdc *wire.Codec) *cobra.Command { } // read in update - bz, err := ioutil.ReadAll(os.Stdin) + bz, err := ioutil.ReadFile(viper.GetString(flagPaymentFile)) if err != nil { return err } @@ -248,152 +255,44 @@ func SubmitPaymentCmd(cdc *wire.Codec) *cobra.Command { return nil }, } - + cmd.Flags().String(flagPaymentFile, "payment.json", "File to read the payment from.") return cmd } -/* -func ClosePaychanCmd(cdc *wire.Codec) *cobra.Command { - flagState := "state" - +func GetChannelCmd(cdc *wire.Codec, paychanStoreName string) *cobra.Command { + flagId := "chan-id" cmd := &cobra.Command{ - Use: "close", - Short: "Close a payment channel, given a state", - Long: "Close an existing payment channel with a state received from a sender. This signs it as the receiver before submitting to the blockchain.", + Use: "get", + Short: "Get info on a channel.", + Long: "Get information on a non closed channel.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - ctx := context.NewCoreContextFromViper().WithDecoder(authcmd.GetAccountDecoder(cdc)) - // Get the sender-signed close tx - state := viper.GetString(flagState) - txBytes, err := base64.StdEncoding.DecodeString(state) - if err != nil { - return err + // Create a "client context" stuct populated with info from common flags + ctx := context.NewCoreContextFromViper() + + // Get channel ID + id := paychan.ChannelID(viper.GetInt64(flagId)) + + // Get the channel from the node + res, err := ctx.QueryStore(paychan.GetChannelKey(id), paychanStoreName) + if len(res) == 0 || err != nil { + return errors.Errorf("channel with ID '%d' does not exist", id) } - stdTx := auth.StdTx{} - cdc.UnmarshalBinary(txBytes, &stdTx) + var channel paychan.Channel + cdc.MustUnmarshalBinary(res, &channel) - // Sign close tx - - // ensure contxt has up to date account and sequence numbers - ctx, err = Ensure(ctx) - if err != nil { - return err - } - // Sign message (asks user for password) - _, sig, err := UserSignMsg(ctx, ctx.FromAddressName, stdTx.Msg) + // Convert the channel to a json object for pretty printing + jsonChannel, err := wire.MarshalJSONIndent(cdc, channel) if err != nil { return err } - // Append signature to close tx - stdTx.Signatures = append(stdTx.Signatures, sig) - // encode close tx - txBytes, err = cdc.MarshalBinary(stdTx) - if err != nil { - return err - } - - // Broadcast close tx to the blockchain - - res, err := ctx.BroadcastTx(txBytes) - if err != nil { - return err - } - fmt.Printf("Committed at block %d. Hash: %s\n", res.Height, res.Hash.String()) + // print out json channel + fmt.Println(string(jsonChannel)) return nil }, } - cmd.Flags().String(flagState, "", "State received from sender.") + cmd.Flags().Int(flagId, 0, "ID of the payment channel.") return cmd } - -// HELPER FUNCTIONS -// This is a partial refactor of cosmos-sdk/client/context. -// Existing API was awkard to use for paychans. - -func Ensure(ctx context.CoreContext) (context.CoreContext, error) { - - ctx, err := context.EnsureAccountNumber(ctx) - if err != nil { - return ctx, err - } - // default to next sequence number if none provided - ctx, err = context.EnsureSequence(ctx) - if err != nil { - return ctx, err - } - return ctx, nil -} - -func UserSignMsg(ctx context.CoreContext, name string, msg sdk.Msg) (signMsg auth.StdSignMsg, stdSig auth.StdSignature, err error) { - - // TODO check how to handle non error return values on error. Returning empty versions doesn't seem right. - - passphrase, err := ctx.GetPassphraseFromStdin(name) - if err != nil { - return signMsg, stdSig, err - } - - // build the Sign Messsage from the Standard Message - chainID := ctx.ChainID - if chainID == "" { - return signMsg, stdSig, errors.Errorf("Chain ID required but not specified") - } - accnum := ctx.AccountNumber - sequence := ctx.Sequence - - signMsg = auth.StdSignMsg{ - ChainID: chainID, - AccountNumbers: []int64{accnum}, - Sequences: []int64{sequence}, - Msg: msg, - Fee: auth.NewStdFee(ctx.Gas, sdk.Coin{}), // TODO run simulate to estimate gas? - } - - keybase, err := keys.GetKeyBase() - if err != nil { - return signMsg, stdSig, err - } - - // sign and build - bz := signMsg.Bytes() - - sig, pubkey, err := keybase.Sign(name, passphrase, bz) - if err != nil { - return signMsg, stdSig, err - } - stdSig = auth.StdSignature{ - PubKey: pubkey, - Signature: sig, - AccountNumber: accnum, - Sequence: sequence, - } - - return signMsg, stdSig, nil -} - -func Build(cdc *wire.Codec, signMsg auth.StdSignMsg, sig auth.StdSignature) ([]byte, error) { - tx := auth.NewStdTx(signMsg.Msg, signMsg.Fee, []auth.StdSignature{sig}) - return cdc.MarshalBinary(tx) -} - -func EnsureSignBuild(ctx context.CoreContext, name string, msg sdk.Msg, cdc *wire.Codec) ([]byte, error) { - //Ensure context has up to date account and sequence numbers - ctx, err := Ensure(ctx) - if err != nil { - return nil, err - } - // Sign message (asks user for password) - signMsg, sig, err := UserSignMsg(ctx, name, msg) - if err != nil { - return nil, err - } - // Create tx and marshal - txBytes, err := Build(cdc, signMsg, sig) - if err != nil { - return nil, err - } - return txBytes, nil -} -*/ diff --git a/internal/x/paychan/wire.go b/internal/x/paychan/wire.go index 98c7817e..ee3da559 100644 --- a/internal/x/paychan/wire.go +++ b/internal/x/paychan/wire.go @@ -9,12 +9,10 @@ func RegisterWire(cdc *wire.Codec) { cdc.RegisterConcrete(MsgSubmitUpdate{}, "paychan/MsgSubmitUpdate", nil) } +// TODO move this to near the msg definitions var msgCdc = wire.NewCodec() -/* func init() { + wire.RegisterCrypto(msgCdc) RegisterWire(msgCdc) - // TODO is this needed? - //wire.RegisterCrypto(msgCdc) } -*/