Feature/8 add cli commands for parce related data #56
Conversation
- restructure tx-related functions's folder structure - add signing option for future use (user can choose which key to use for signing the tx)
- add register, request, cancel operations - add grant, revoke, discard operations - decrypt key when making message(rpc) - add linebreak between CLI operations
|
After parcel-related operations' descriptions on rpc.md are fully set, each of operation's description on CLI is planned to get improved for better understanding. |
client/keys/util.go
Outdated
| @@ -51,3 +60,72 @@ func SaveKeyList(path string, keyList map[string]Key) error { | |||
|
|
|||
| return nil | |||
| } | |||
|
|
|||
| func GetKeyToSign(path string) (Key, error) { | |||
There was a problem hiding this comment.
why don''t you change the name to 'SelectKey...'?
client/keys/util.go
Outdated
| return key, errors.New("The key doesn't exist") | ||
| } | ||
|
|
||
| key = keyList[nickname] |
There was a problem hiding this comment.
recommend refactoring. not very clear distinction between getkey...() and check().
client/keys/util.go
Outdated
| return key, err | ||
| } | ||
|
|
||
| nickname = strings.Replace(nickname, "\r\n", "", -1) |
There was a problem hiding this comment.
is it necessary? what about using Trim()?
client/keys/util.go
Outdated
| keyStatus := Check(nickname, path) | ||
| switch keyStatus { | ||
| case NoExists: | ||
| return key, errors.New("The key doesn't exist") |
There was a problem hiding this comment.
purpose of this function is to select one of the existing keys, then this error message is confusing. what? doesn't exist?
client/keys/util.go
Outdated
| return key, nil | ||
| } | ||
|
|
||
| func GetDecryptedKey(key *Key) error { |
There was a problem hiding this comment.
this function "transforms" the key that we already have. what about renaming it to "decrypt key".
client/rpc/broadcast.go
Outdated
|
|
||
| "github.com/amolabs/amoabci/amo/operation" | ||
| atypes "github.com/amolabs/amoabci/amo/types" | ||
| ) | ||
|
|
||
| // Transfer handles transfer transaction | ||
| func Transfer(to types.Address, amount *atypes.Currency) (*ctypes.ResultBroadcastTxCommit, error) { | ||
| return RPCBroadcastTxCommit(MakeMessage(operation.TxTransfer, operation.Transfer{ | ||
| func Transfer(to crypto.Address, amount *atypes.Currency, sign bool) (*ctypes.ResultBroadcastTxCommit, error) { |
There was a problem hiding this comment.
after we start to support signed transaction, signing a transaction should be mandatory. an unsigned transaction will be not an option. and this comment applies to other transactions: register, request, ...
client/rpc/rpc.go
Outdated
| @@ -16,20 +22,62 @@ var ( | |||
| ) | |||
|
|
|||
| // MakeMessage handles making tx message | |||
| func MakeMessage(t string, payload interface{}) types.Tx { | |||
| func MakeMessage(t string, nonce uint32, payload interface{}, sign bool) (types.Tx, error) { | |||
There was a problem hiding this comment.
refer to the comment to the Transfer() function.
client/rpc/rpc.go
Outdated
| @@ -16,20 +22,62 @@ var ( | |||
| ) | |||
|
|
|||
| // MakeMessage handles making tx message | |||
| func MakeMessage(t string, payload interface{}) types.Tx { | |||
| func MakeMessage(t string, nonce uint32, payload interface{}, sign bool) (types.Tx, error) { | |||
There was a problem hiding this comment.
Think about this situation: we got a key to sign a transaction, but we don't care how. we just got a key. then, we just need a function to make a formed or formatted message. one unit of a code does one thing: make something from the prepared ingredients. think about layers or hierarchy of functions.
- change functions' name to make their feature clear - set 'sign' to default - reconsider and restructure hierarchy of functions (remove dependecy between 'client' and 'cmd' functions)
…d_cli_commands_for_parce-related_data
MakeMessage -> Sign Payload -> Signature, SigningPubKey, Signer(Address), Payload, etc..
(unfortunately verification on amoabci server's
checkTx()is not supported yet)resolves #8