Skip to content

Commit

Permalink
Merge PR cosmos#1280: Implement simple transaction memos
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
cwgoes authored Jun 20, 2018
1 parent 2a9bc21 commit 918e217
Show file tree
Hide file tree
Showing 16 changed files with 157 additions and 32 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand Down
4 changes: 3 additions & 1 deletion client/context/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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?
}

Expand All @@ -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)
}
Expand Down
7 changes: 7 additions & 0 deletions client/context/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type CoreContext struct {
FromAddressName string
AccountNumber int64
Sequence int64
Memo string
Client rpcclient.Client
Decoder auth.AccountDecoder
AccountStore string
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions client/context/viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions client/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const (
FlagName = "name"
FlagAccountNumber = "account-number"
FlagSequence = "sequence"
FlagMemo = "memo"
FlagFee = "fee"
)

Expand All @@ -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", "<host>:<port> to tendermint rpc interface for this chain")
Expand Down
9 changes: 9 additions & 0 deletions cmd/gaia/cli_test/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions examples/kvstore/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions server/mock/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 6 additions & 0 deletions types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions types/tx_msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ type Tx interface {

// Gets the Msg.
GetMsg() Msg

// Gets the memo.
GetMemo() string
}

//__________________________________________________________
Expand Down
25 changes: 19 additions & 6 deletions x/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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))
Expand Down Expand Up @@ -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...
Expand Down
81 changes: 71 additions & 10 deletions x/auth/ante_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package auth

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -18,7 +19,7 @@ func newTestMsg(addrs ...sdk.Address) *sdk.TestMsg {
}

func newStdFee() StdFee {
return NewStdFee(100,
return NewStdFee(5000,
sdk.NewCoin("atom", 150),
)
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand Down Expand Up @@ -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)
}
Expand Down
5 changes: 3 additions & 2 deletions x/auth/mock/simulate_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 918e217

Please sign in to comment.