From 918e217e1f52daa6f30a19468a04f9b1893eee82 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Wed, 20 Jun 2018 21:27:36 +0200 Subject: [PATCH] Merge PR #1280: Implement simple transaction memos * AltBytes -> Memo, memo CLI support & thread-through * Check memo size, update changelog * Update existing testcases * Nuke CircleCI caches * Charge gas proportional to memo size * Fix gas allocations in ante handler testcases * Add testcases * Update changelog * Fix tiny CLI bug & add to CLI tests * Add '--memo' to gaiacli * Add testcase for large memos * Address PR comments --- CHANGELOG.md | 4 ++ baseapp/baseapp_test.go | 2 + client/context/helpers.go | 4 +- client/context/types.go | 7 +++ client/context/viper.go | 1 + client/flags.go | 2 + cmd/gaia/cli_test/cli_test.go | 9 ++++ examples/kvstore/tx.go | 4 ++ server/mock/tx.go | 4 ++ types/errors.go | 6 +++ types/tx_msg.go | 3 ++ x/auth/ante.go | 25 ++++++++--- x/auth/ante_test.go | 81 ++++++++++++++++++++++++++++++----- x/auth/mock/simulate_block.go | 5 ++- x/auth/stdtx.go | 30 +++++++------ x/auth/stdtx_test.go | 2 +- 16 files changed, 157 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d557899372..5edcaedcf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ BREAKING CHANGES * Change default ports from 466xx to 266xx +* AltBytes renamed to Memo, now a string, max 100 characters, costs a bit of gas + +FEATURES +* [gaiacli] You can now attach a simple text-only memo to any transaction, with the `--memo` flag FIXES * \#1259 - fix bug where certain tests that could have a nil pointer in defer diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 532d39f1e6..7ee9f418ec 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -263,6 +263,7 @@ const msgType2 = "testTx" func (tx testTx) Type() string { return msgType2 } func (tx testTx) GetMsg() sdk.Msg { return tx } +func (tx testTx) GetMemo() string { return "" } func (tx testTx) GetSignBytes() []byte { return nil } func (tx testTx) GetSigners() []sdk.Address { return nil } func (tx testTx) GetSignatures() []auth.StdSignature { return nil } @@ -547,6 +548,7 @@ const msgType = "testUpdatePowerTx" func (tx testUpdatePowerTx) Type() string { return msgType } func (tx testUpdatePowerTx) GetMsg() sdk.Msg { return tx } +func (tx testUpdatePowerTx) GetMemo() string { return "" } func (tx testUpdatePowerTx) GetSignBytes() []byte { return nil } func (tx testUpdatePowerTx) ValidateBasic() sdk.Error { return nil } func (tx testUpdatePowerTx) GetSigners() []sdk.Address { return nil } diff --git a/client/context/helpers.go b/client/context/helpers.go index 5449afa97e..5ce81d0d35 100644 --- a/client/context/helpers.go +++ b/client/context/helpers.go @@ -123,12 +123,14 @@ func (ctx CoreContext) SignAndBuild(name, passphrase string, msg sdk.Msg, cdc *w } accnum := ctx.AccountNumber sequence := ctx.Sequence + memo := ctx.Memo signMsg := auth.StdSignMsg{ ChainID: chainID, AccountNumbers: []int64{accnum}, Sequences: []int64{sequence}, Msg: msg, + Memo: memo, Fee: auth.NewStdFee(ctx.Gas, sdk.Coin{}), // TODO run simulate to estimate gas? } @@ -152,7 +154,7 @@ func (ctx CoreContext) SignAndBuild(name, passphrase string, msg sdk.Msg, cdc *w }} // marshal bytes - tx := auth.NewStdTx(signMsg.Msg, signMsg.Fee, sigs) + tx := auth.NewStdTx(signMsg.Msg, signMsg.Fee, sigs, memo) return cdc.MarshalBinary(tx) } diff --git a/client/context/types.go b/client/context/types.go index 791ffb23a2..6b0c074f2c 100644 --- a/client/context/types.go +++ b/client/context/types.go @@ -16,6 +16,7 @@ type CoreContext struct { FromAddressName string AccountNumber int64 Sequence int64 + Memo string Client rpcclient.Client Decoder auth.AccountDecoder AccountStore string @@ -70,6 +71,12 @@ func (c CoreContext) WithSequence(sequence int64) CoreContext { return c } +// WithMemo - return a copy of the context with an updated memo +func (c CoreContext) WithMemo(memo string) CoreContext { + c.Memo = memo + return c +} + // WithClient - return a copy of the context with an updated RPC client instance func (c CoreContext) WithClient(client rpcclient.Client) CoreContext { c.Client = client diff --git a/client/context/viper.go b/client/context/viper.go index 5f262d56f6..d3896fe580 100644 --- a/client/context/viper.go +++ b/client/context/viper.go @@ -36,6 +36,7 @@ func NewCoreContextFromViper() CoreContext { NodeURI: nodeURI, AccountNumber: viper.GetInt64(client.FlagAccountNumber), Sequence: viper.GetInt64(client.FlagSequence), + Memo: viper.GetString(client.FlagMemo), Client: rpc, Decoder: nil, AccountStore: "acc", diff --git a/client/flags.go b/client/flags.go index 5af588bd44..eb390e8a04 100644 --- a/client/flags.go +++ b/client/flags.go @@ -12,6 +12,7 @@ const ( FlagName = "name" FlagAccountNumber = "account-number" FlagSequence = "sequence" + FlagMemo = "memo" FlagFee = "fee" ) @@ -37,6 +38,7 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { c.Flags().String(FlagName, "", "Name of private key with which to sign") c.Flags().Int64(FlagAccountNumber, 0, "AccountNumber number to sign the tx") c.Flags().Int64(FlagSequence, 0, "Sequence number to sign the tx") + c.Flags().String(FlagMemo, "", "Memo to send along with transaction") c.Flags().String(FlagFee, "", "Fee to pay along with transaction") c.Flags().String(FlagChainID, "", "Chain ID of tendermint node") c.Flags().String(FlagNode, "tcp://localhost:26657", ": to tendermint rpc interface for this chain") diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index af62c3e992..919a4c5b66 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -64,6 +64,15 @@ func TestGaiaCLISend(t *testing.T) { assert.Equal(t, int64(20), barAcc.GetCoins().AmountOf("steak").Int64()) fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %v %v", fooCech, flags)) assert.Equal(t, int64(30), fooAcc.GetCoins().AmountOf("steak").Int64()) + + // test memo + executeWrite(t, fmt.Sprintf("gaiacli send %v --amount=10steak --to=%v --name=foo --memo 'testmemo'", flags, barCech), pass) + tests.WaitForNextHeightTM(port) + + barAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %v %v", barCech, flags)) + assert.Equal(t, int64(30), barAcc.GetCoins().AmountOf("steak").Int64()) + fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %v %v", fooCech, flags)) + assert.Equal(t, int64(20), fooAcc.GetCoins().AmountOf("steak").Int64()) } func TestGaiaCLICreateValidator(t *testing.T) { diff --git a/examples/kvstore/tx.go b/examples/kvstore/tx.go index fa32d93bfb..3cedf35bb8 100644 --- a/examples/kvstore/tx.go +++ b/examples/kvstore/tx.go @@ -22,6 +22,10 @@ func (tx kvstoreTx) GetMsg() sdk.Msg { return tx } +func (tx kvstoreTx) GetMemo() string { + return "" +} + func (tx kvstoreTx) GetSignBytes() []byte { return tx.bytes } diff --git a/server/mock/tx.go b/server/mock/tx.go index bd437f2d08..5630b46d86 100644 --- a/server/mock/tx.go +++ b/server/mock/tx.go @@ -35,6 +35,10 @@ func (tx kvstoreTx) GetMsg() sdk.Msg { return tx } +func (tx kvstoreTx) GetMemo() string { + return "" +} + func (tx kvstoreTx) GetSignBytes() []byte { return tx.bytes } diff --git a/types/errors.go b/types/errors.go index f979ee118c..f76b171afb 100644 --- a/types/errors.go +++ b/types/errors.go @@ -53,6 +53,7 @@ const ( CodeInsufficientCoins CodeType = 10 CodeInvalidCoins CodeType = 11 CodeOutOfGas CodeType = 12 + CodeMemoTooLarge CodeType = 13 // CodespaceRoot is a codespace for error codes in this file only. // Notice that 0 is an "unset" codespace, which can be overridden with @@ -91,6 +92,8 @@ func CodeToDefaultMsg(code CodeType) string { return "invalid coins" case CodeOutOfGas: return "out of gas" + case CodeMemoTooLarge: + return "memo too large" default: return fmt.Sprintf("unknown code %d", code) } @@ -137,6 +140,9 @@ func ErrInvalidCoins(msg string) Error { func ErrOutOfGas(msg string) Error { return newErrorWithRootCodespace(CodeOutOfGas, msg) } +func ErrMemoTooLarge(msg string) Error { + return newErrorWithRootCodespace(CodeMemoTooLarge, msg) +} //---------------------------------------- // Error & sdkError diff --git a/types/tx_msg.go b/types/tx_msg.go index 240db31060..510e0a3838 100644 --- a/types/tx_msg.go +++ b/types/tx_msg.go @@ -31,6 +31,9 @@ type Tx interface { // Gets the Msg. GetMsg() Msg + + // Gets the memo. + GetMemo() string } //__________________________________________________________ diff --git a/x/auth/ante.go b/x/auth/ante.go index c50da0c32a..cedc94dc47 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -9,8 +9,10 @@ import ( ) const ( - deductFeesCost sdk.Gas = 10 - verifyCost = 100 + deductFeesCost sdk.Gas = 10 + memoCostPerByte sdk.Gas = 1 + verifyCost = 100 + maxMemoCharacters = 100 ) // NewAnteHandler returns an AnteHandler that checks @@ -36,6 +38,20 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { true } + memo := tx.GetMemo() + + if len(memo) > maxMemoCharacters { + return ctx, + sdk.ErrMemoTooLarge(fmt.Sprintf("maximum number of characters is %d but received %d characters", maxMemoCharacters, len(memo))).Result(), + true + } + + // set the gas meter + ctx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) + + // charge gas for the memo + ctx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(memo)), "memo") + msg := tx.GetMsg() // Assert that number of signatures is correct. @@ -62,7 +78,7 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { if chainID == "" { chainID = viper.GetString("chain-id") } - signBytes := StdSignBytes(ctx.ChainID(), accNums, sequences, fee, msg) + signBytes := StdSignBytes(ctx.ChainID(), accNums, sequences, fee, msg, memo) // Check sig and nonce and collect signer accounts. var signerAccs = make([]Account, len(signerAddrs)) @@ -99,9 +115,6 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { // cache the signer accounts in the context ctx = WithSigners(ctx, signerAccs) - // set the gas meter - ctx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) - // TODO: tx tags (?) return ctx, sdk.Result{}, false // continue... diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index 3935334a8f..453502ddf0 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -1,6 +1,7 @@ package auth import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -18,7 +19,7 @@ func newTestMsg(addrs ...sdk.Address) *sdk.TestMsg { } func newStdFee() StdFee { - return NewStdFee(100, + return NewStdFee(5000, sdk.NewCoin("atom", 150), ) } @@ -47,22 +48,39 @@ func checkValidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx // run the tx through the anteHandler and ensure it fails with the given code func checkInvalidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx sdk.Tx, code sdk.CodeType) { + defer func() { + if r := recover(); r != nil { + switch r.(type) { + case sdk.ErrorOutOfGas: + assert.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, code), sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOutOfGas), + fmt.Sprintf("Expected ErrorOutOfGas, got %v", r)) + default: + panic(r) + } + } + }() _, result, abort := anteHandler(ctx, tx) assert.True(t, abort) - assert.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, code), result.Code) + assert.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, code), result.Code, + fmt.Sprintf("Expected %v, got %v", sdk.ToABCICode(sdk.CodespaceRoot, code), result)) } func newTestTx(ctx sdk.Context, msg sdk.Msg, privs []crypto.PrivKey, accNums []int64, seqs []int64, fee StdFee) sdk.Tx { - signBytes := StdSignBytes(ctx.ChainID(), accNums, seqs, fee, msg) - return newTestTxWithSignBytes(msg, privs, accNums, seqs, fee, signBytes) + signBytes := StdSignBytes(ctx.ChainID(), accNums, seqs, fee, msg, "") + return newTestTxWithSignBytes(msg, privs, accNums, seqs, fee, signBytes, "") } -func newTestTxWithSignBytes(msg sdk.Msg, privs []crypto.PrivKey, accNums []int64, seqs []int64, fee StdFee, signBytes []byte) sdk.Tx { +func newTestTxWithMemo(ctx sdk.Context, msg sdk.Msg, privs []crypto.PrivKey, accNums []int64, seqs []int64, fee StdFee, memo string) sdk.Tx { + signBytes := StdSignBytes(ctx.ChainID(), accNums, seqs, fee, msg, memo) + return newTestTxWithSignBytes(msg, privs, accNums, seqs, fee, signBytes, memo) +} + +func newTestTxWithSignBytes(msg sdk.Msg, privs []crypto.PrivKey, accNums []int64, seqs []int64, fee StdFee, signBytes []byte, memo string) sdk.Tx { sigs := make([]StdSignature, len(privs)) for i, priv := range privs { sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: priv.Sign(signBytes), AccountNumber: accNums[i], Sequence: seqs[i]} } - tx := NewStdTx(msg, fee, sigs) + tx := NewStdTx(msg, fee, sigs, memo) return tx } @@ -252,9 +270,7 @@ func TestAnteHandlerFees(t *testing.T) { var tx sdk.Tx msg := newTestMsg(addr1) privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} - fee := NewStdFee(100, - sdk.NewCoin("atom", 150), - ) + fee := newStdFee() // signer does not have enough funds to pay the fee tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) @@ -273,6 +289,50 @@ func TestAnteHandlerFees(t *testing.T) { assert.True(t, feeCollector.GetCollectedFees(ctx).IsEqual(sdk.Coins{sdk.NewCoin("atom", 150)})) } +// Test logic around memo gas consumption. +func TestAnteHandlerMemoGas(t *testing.T) { + // setup + ms, capKey, capKey2 := setupMultiStore() + cdc := wire.NewCodec() + RegisterBaseAccount(cdc) + mapper := NewAccountMapper(cdc, capKey, &BaseAccount{}) + feeCollector := NewFeeCollectionKeeper(cdc, capKey2) + anteHandler := NewAnteHandler(mapper, feeCollector) + ctx := sdk.NewContext(ms, abci.Header{ChainID: "mychainid"}, false, nil, log.NewNopLogger()) + + // keys and addresses + priv1, addr1 := privAndAddr() + + // set the accounts + acc1 := mapper.NewAccountWithAddress(ctx, addr1) + mapper.SetAccount(ctx, acc1) + + // msg and signatures + var tx sdk.Tx + msg := newTestMsg(addr1) + privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} + fee := NewStdFee(0, sdk.NewCoin("atom", 0)) + + // tx does not have enough gas + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeOutOfGas) + + // tx with memo doesn't have enough gas + fee = NewStdFee(1001, sdk.NewCoin("atom", 0)) + tx = newTestTxWithMemo(ctx, msg, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsd") + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeOutOfGas) + + // memo too large + fee = NewStdFee(2001, sdk.NewCoin("atom", 0)) + tx = newTestTxWithMemo(ctx, msg, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsdabcininasidniandsinasindiansdiansdinaisndiasndiadninsdabcininasidniandsinasindiansdiansdinaisndiasndiadninsd") + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeMemoTooLarge) + + // tx with memo has enough gas + fee = NewStdFee(1100, sdk.NewCoin("atom", 0)) + tx = newTestTxWithMemo(ctx, msg, privs, accnums, seqs, fee, "abcininasidniandsinasindiansdiansdinaisndiasndiadninsd") + checkValidTx(t, anteHandler, ctx, tx) +} + func TestAnteHandlerBadSignBytes(t *testing.T) { // setup ms, capKey, capKey2 := setupMultiStore() @@ -333,7 +393,8 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { for _, cs := range cases { tx := newTestTxWithSignBytes( msg, privs, accnums, seqs, fee, - StdSignBytes(cs.chainID, cs.accnums, cs.seqs, cs.fee, cs.msg), + StdSignBytes(cs.chainID, cs.accnums, cs.seqs, cs.fee, cs.msg, ""), + "", ) checkInvalidTx(t, anteHandler, ctx, tx, cs.code) } diff --git a/x/auth/mock/simulate_block.go b/x/auth/mock/simulate_block.go index 7a4cb4aef3..ff578c2b36 100644 --- a/x/auth/mock/simulate_block.go +++ b/x/auth/mock/simulate_block.go @@ -42,15 +42,16 @@ func GenTx(msg sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKeyEd25 } sigs := make([]auth.StdSignature, len(priv)) + memo := "testmemotestmemo" for i, p := range priv { sigs[i] = auth.StdSignature{ PubKey: p.PubKey(), - Signature: p.Sign(auth.StdSignBytes(chainID, accnums, seq, fee, msg)), + Signature: p.Sign(auth.StdSignBytes(chainID, accnums, seq, fee, msg, memo)), AccountNumber: accnums[i], Sequence: seq[i], } } - return auth.NewStdTx(msg, fee, sigs) + return auth.NewStdTx(msg, fee, sigs, memo) } // check a transaction result diff --git a/x/auth/stdtx.go b/x/auth/stdtx.go index 5c43a37177..e3a69a0391 100644 --- a/x/auth/stdtx.go +++ b/x/auth/stdtx.go @@ -15,19 +15,24 @@ type StdTx struct { Msg sdk.Msg `json:"msg"` Fee StdFee `json:"fee"` Signatures []StdSignature `json:"signatures"` + Memo string `json:"memo"` } -func NewStdTx(msg sdk.Msg, fee StdFee, sigs []StdSignature) StdTx { +func NewStdTx(msg sdk.Msg, fee StdFee, sigs []StdSignature, memo string) StdTx { return StdTx{ Msg: msg, Fee: fee, Signatures: sigs, + Memo: memo, } } //nolint func (tx StdTx) GetMsg() sdk.Msg { return tx.Msg } +//nolint +func (tx StdTx) GetMemo() string { return tx.Memo } + // Signatures returns the signature of signers who signed the Msg. // CONTRACT: Length returned is same as length of // pubkeys returned from MsgKeySigners, and the order @@ -85,23 +90,24 @@ func (fee StdFee) Bytes() []byte { // and the Sequence numbers for each signature (prevent // inchain replay and enforce tx ordering per account). type StdSignDoc struct { - ChainID string `json:"chain_id"` - AccountNumbers []int64 `json:"account_numbers"` - Sequences []int64 `json:"sequences"` - FeeBytes []byte `json:"fee_bytes"` - MsgBytes []byte `json:"msg_bytes"` - AltBytes []byte `json:"alt_bytes"` + ChainID string `json:"chain_id"` + AccountNumbers []int64 `json:"account_numbers"` + Sequences []int64 `json:"sequences"` + Fee json.RawMessage `json:"fee"` + Msg json.RawMessage `json:"msg"` + Memo string `json:"memo"` } // StdSignBytes returns the bytes to sign for a transaction. // TODO: change the API to just take a chainID and StdTx ? -func StdSignBytes(chainID string, accnums []int64, sequences []int64, fee StdFee, msg sdk.Msg) []byte { +func StdSignBytes(chainID string, accnums []int64, sequences []int64, fee StdFee, msg sdk.Msg, memo string) []byte { bz, err := json.Marshal(StdSignDoc{ ChainID: chainID, AccountNumbers: accnums, Sequences: sequences, - FeeBytes: fee.Bytes(), - MsgBytes: msg.GetSignBytes(), + Fee: json.RawMessage(fee.Bytes()), + Msg: json.RawMessage(msg.GetSignBytes()), + Memo: memo, }) if err != nil { panic(err) @@ -118,12 +124,12 @@ type StdSignMsg struct { Sequences []int64 Fee StdFee Msg sdk.Msg - // XXX: Alt + Memo string } // get message bytes func (msg StdSignMsg) Bytes() []byte { - return StdSignBytes(msg.ChainID, msg.AccountNumbers, msg.Sequences, msg.Fee, msg.Msg) + return StdSignBytes(msg.ChainID, msg.AccountNumbers, msg.Sequences, msg.Fee, msg.Msg, msg.Memo) } // Standard Signature diff --git a/x/auth/stdtx_test.go b/x/auth/stdtx_test.go index 412b37c20d..6b84b3f6e5 100644 --- a/x/auth/stdtx_test.go +++ b/x/auth/stdtx_test.go @@ -23,7 +23,7 @@ func TestStdTx(t *testing.T) { fee := newStdFee() sigs := []StdSignature{} - tx := NewStdTx(msg, fee, sigs) + tx := NewStdTx(msg, fee, sigs, "") assert.Equal(t, msg, tx.GetMsg()) assert.Equal(t, sigs, tx.GetSignatures())