From 5950ff34e3ea805a06d6dd27c3b5c7f0dcf6c2cc Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 12 Jul 2017 18:54:07 +0200 Subject: [PATCH] remove sequence number from coins --- app/app_test.go | 14 ++--- cmd/basecli/commands/cmds.go | 5 +- docs/guide/counter/plugins/counter/counter.go | 4 +- errors/common.go | 4 +- modules/coin/bench_test.go | 6 +-- modules/coin/coin.go | 13 +++++ modules/coin/handler.go | 9 ++-- modules/coin/handler_test.go | 20 +++---- modules/coin/store.go | 18 ++----- modules/coin/tx.go | 27 +++++----- modules/coin/tx_test.go | 54 +++++++------------ 11 files changed, 82 insertions(+), 92 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index 9656190c8..ff9ed2221 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -41,8 +41,8 @@ func newAppTest(t *testing.T) *appTest { } // make a tx sending 5mycoin from each acctIn to acctOut -func (at *appTest) getTx(seq int, coins coin.Coins) basecoin.Tx { - in := []coin.TxInput{{Address: at.acctIn.Actor(), Coins: coins, Sequence: seq}} +func (at *appTest) getTx(coins coin.Coins) basecoin.Tx { + in := []coin.TxInput{{Address: at.acctIn.Actor(), Coins: coins}} out := []coin.TxOutput{{Address: at.acctOut.Actor(), Coins: coins}} tx := coin.NewSendTx(in, out) tx = base.NewChainTx(at.chainID, 0, tx) @@ -193,22 +193,22 @@ func TestTx(t *testing.T) { //Bad Balance at.acctIn.Coins = coin.Coins{{"mycoin", 2}} at.initAccount(at.acctIn) - res, _, _ := at.exec(t, at.getTx(1, coin.Coins{{"mycoin", 5}}), true) + res, _, _ := at.exec(t, at.getTx(coin.Coins{{"mycoin", 5}}), true) assert.True(res.IsErr(), "ExecTx/Bad CheckTx: Expected error return from ExecTx, returned: %v", res) - res, diffIn, diffOut := at.exec(t, at.getTx(1, coin.Coins{{"mycoin", 5}}), false) + res, diffIn, diffOut := at.exec(t, at.getTx(coin.Coins{{"mycoin", 5}}), false) assert.True(res.IsErr(), "ExecTx/Bad DeliverTx: Expected error return from ExecTx, returned: %v", res) assert.True(diffIn.IsZero()) assert.True(diffOut.IsZero()) //Regular CheckTx at.reset() - res, _, _ = at.exec(t, at.getTx(1, coin.Coins{{"mycoin", 5}}), true) + res, _, _ = at.exec(t, at.getTx(coin.Coins{{"mycoin", 5}}), true) assert.True(res.IsOK(), "ExecTx/Good CheckTx: Expected OK return from ExecTx, Error: %v", res) //Regular DeliverTx at.reset() amt := coin.Coins{{"mycoin", 3}} - res, diffIn, diffOut = at.exec(t, at.getTx(1, amt), false) + res, diffIn, diffOut = at.exec(t, at.getTx(amt), false) assert.True(res.IsOK(), "ExecTx/Good DeliverTx: Expected OK return from ExecTx, Error: %v", res) assert.Equal(amt.Negative(), diffIn) assert.Equal(amt, diffOut) @@ -218,7 +218,7 @@ func TestQuery(t *testing.T) { assert := assert.New(t) at := newAppTest(t) - res, _, _ := at.exec(t, at.getTx(1, coin.Coins{{"mycoin", 5}}), false) + res, _, _ := at.exec(t, at.getTx(coin.Coins{{"mycoin", 5}}), false) assert.True(res.IsOK(), "Commit, DeliverTx: Expected OK return from DeliverTx, Error: %v", res) resQueryPreCommit := at.app.Query(abci.RequestQuery{ diff --git a/cmd/basecli/commands/cmds.go b/cmd/basecli/commands/cmds.go index 4c5a90640..1f477e988 100644 --- a/cmd/basecli/commands/cmds.go +++ b/cmd/basecli/commands/cmds.go @@ -125,9 +125,8 @@ func readSendTxFlags() (tx basecoin.Tx, err error) { // craft the inputs and outputs ins := []coin.TxInput{{ - Address: fromAddr, - Coins: amountCoins, - Sequence: viper.GetInt(FlagSequence), + Address: fromAddr, + Coins: amountCoins, }} outs := []coin.TxOutput{{ Address: toAddr, diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 168886160..dd432822b 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -145,7 +145,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi if len(senders) == 0 { return res, errors.ErrMissingSignature() } - in := []coin.TxInput{{Address: senders[0], Coins: ctr.Fee, Sequence: ctr.Sequence}} + in := []coin.TxInput{{Address: senders[0], Coins: ctr.Fee}} out := []coin.TxOutput{{Address: StoreActor(), Coins: ctr.Fee}} send := coin.NewSendTx(in, out) // if the deduction fails (too high), abort the command @@ -170,7 +170,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi func checkTx(ctx basecoin.Context, tx basecoin.Tx) (ctr Tx, err error) { ctr, ok := tx.Unwrap().(Tx) if !ok { - return ctr, errors.ErrInvalidFormat(tx) + return ctr, errors.ErrInvalidFormat(TypeTx, tx) } err = ctr.ValidateBasic() if err != nil { diff --git a/errors/common.go b/errors/common.go index c793a6953..5eb4b1ee3 100644 --- a/errors/common.go +++ b/errors/common.go @@ -47,8 +47,8 @@ func IsUnknownTxTypeErr(err error) bool { return IsSameError(errUnknownTxType, err) } -func ErrInvalidFormat(tx interface{}) TMError { - msg := fmt.Sprintf("%T", unwrap(tx)) +func ErrInvalidFormat(expected string, tx interface{}) TMError { + msg := fmt.Sprintf("%T not %s", unwrap(tx), expected) w := errors.Wrap(errInvalidFormat, msg) return WithCode(w, abci.CodeType_UnknownRequest) } diff --git a/modules/coin/bench_test.go b/modules/coin/bench_test.go index 83d3da6b7..34bf39bd1 100644 --- a/modules/coin/bench_test.go +++ b/modules/coin/bench_test.go @@ -15,8 +15,8 @@ func makeHandler() basecoin.Handler { return NewHandler() } -func makeSimpleTx(from, to basecoin.Actor, amount Coins, seq int) basecoin.Tx { - in := []TxInput{{Address: from, Coins: amount, Sequence: seq}} +func makeSimpleTx(from, to basecoin.Actor, amount Coins) basecoin.Tx { + in := []TxInput{{Address: from, Coins: amount}} out := []TxOutput{{Address: to, Coins: amount}} return NewSendTx(in, out) } @@ -35,7 +35,7 @@ func BenchmarkSimpleTransfer(b *testing.B) { // now, loop... for i := 1; i <= b.N; i++ { ctx := stack.MockContext("foo", 100).WithPermissions(sender) - tx := makeSimpleTx(sender, receiver, Coins{{"mycoin", 2}}, i) + tx := makeSimpleTx(sender, receiver, Coins{{"mycoin", 2}}) _, err := h.DeliverTx(ctx, store, tx) // never should error if err != nil { diff --git a/modules/coin/coin.go b/modules/coin/coin.go index 6696288ab..168b06925 100644 --- a/modules/coin/coin.go +++ b/modules/coin/coin.go @@ -16,10 +16,23 @@ type Coin struct { Amount int64 `json:"amount"` } +// String provides a human-readable representation of a coin func (coin Coin) String() string { return fmt.Sprintf("%v%v", coin.Amount, coin.Denom) } +// IsZero returns if this represents no money +func (coin Coin) IsZero() bool { + return coin.Amount == 0 +} + +// IsGTE returns true if they are the same type and the receiver is +// an equal or greater value +func (coin Coin) IsGTE(other Coin) bool { + return (coin.Denom == other.Denom) && + (coin.Amount >= other.Amount) +} + //regex codes for extracting coins from string var reDenom = regexp.MustCompile("") var reAmt = regexp.MustCompile("(\\d+)") diff --git a/modules/coin/handler.go b/modules/coin/handler.go index bd047f8eb..0e3378db7 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -37,7 +37,7 @@ func (h Handler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin. // now make sure there is money for _, in := range send.Inputs { - _, err = CheckCoins(store, in.Address, in.Coins.Negative(), in.Sequence) + _, err = CheckCoins(store, in.Address, in.Coins.Negative()) if err != nil { return res, err } @@ -56,7 +56,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi // deduct from all input accounts for _, in := range send.Inputs { - _, err = ChangeCoins(store, in.Address, in.Coins.Negative(), in.Sequence) + _, err = ChangeCoins(store, in.Address, in.Coins.Negative()) if err != nil { return res, err } @@ -64,8 +64,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi // add to all output accounts for _, out := range send.Outputs { - // note: sequence number is ignored when adding coins, only checked for subtracting - _, err = ChangeCoins(store, out.Address, out.Coins, 0) + _, err = ChangeCoins(store, out.Address, out.Coins) if err != nil { return res, err } @@ -107,7 +106,7 @@ func checkTx(ctx basecoin.Context, tx basecoin.Tx) (send SendTx, err error) { // check if the tx is proper type and valid send, ok := tx.Unwrap().(SendTx) if !ok { - return send, errors.ErrInvalidFormat(tx) + return send, errors.ErrInvalidFormat(TypeSend, tx) } err = send.ValidateBasic() if err != nil { diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index 716ad7ed7..11959eccf 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -35,40 +35,40 @@ func TestHandlerValidation(t *testing.T) { // auth works with different apps {true, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{addr1}}, {true, NewSendTx( - []TxInput{NewTxInput(addr2, someCoins, 2)}, + []TxInput{NewTxInput(addr2, someCoins)}, []TxOutput{NewTxOutput(addr1, someCoins)}), []basecoin.Actor{addr1, addr2}}, // check multi-input with both sigs {true, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, someCoins, 3)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, someCoins)}, []TxOutput{NewTxOutput(addr1, doubleCoins)}), []basecoin.Actor{addr1, addr2}}, // wrong permissions fail {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{}}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{addr2}}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, someCoins, 3)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, someCoins)}, []TxOutput{NewTxOutput(addr1, doubleCoins)}), []basecoin.Actor{addr1}}, // invalid input fails {false, NewSendTx( - []TxInput{NewTxInput(addr1, minusCoins, 2)}, + []TxInput{NewTxInput(addr1, minusCoins)}, []TxOutput{NewTxOutput(addr2, minusCoins)}), []basecoin.Actor{addr2}}, } @@ -113,7 +113,7 @@ func TestDeliverTx(t *testing.T) { { []money{{addr1, moreCoins}}, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 1)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{addr1}, []money{{addr1, diffCoins}, {addr2, someCoins}}, @@ -122,7 +122,7 @@ func TestDeliverTx(t *testing.T) { { []money{{addr1, mixedCoins}, {addr2, moreCoins}}, NewSendTx( - []TxInput{NewTxInput(addr1, otherCoins, 1), NewTxInput(addr2, someCoins, 1)}, + []TxInput{NewTxInput(addr1, otherCoins), NewTxInput(addr2, someCoins)}, []TxOutput{NewTxOutput(addr3, mixedCoins)}), []basecoin.Actor{addr1, addr2}, []money{{addr1, someCoins}, {addr2, diffCoins}, {addr3, mixedCoins}}, @@ -131,7 +131,7 @@ func TestDeliverTx(t *testing.T) { { []money{{addr1, moreCoins.Plus(otherCoins)}}, NewSendTx( - []TxInput{NewTxInput(addr1, otherCoins, 1), NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, otherCoins), NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, mixedCoins)}), []basecoin.Actor{addr1}, []money{{addr1, diffCoins}, {addr2, mixedCoins}}, diff --git a/modules/coin/store.go b/modules/coin/store.go index 94c086f1d..56e99f281 100644 --- a/modules/coin/store.go +++ b/modules/coin/store.go @@ -22,14 +22,14 @@ func GetAccount(store state.KVStore, addr basecoin.Actor) (Account, error) { } // CheckCoins makes sure there are funds, but doesn't change anything -func CheckCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (Coins, error) { - acct, err := updateCoins(store, addr, coins, seq) +func CheckCoins(store state.KVStore, addr basecoin.Actor, coins Coins) (Coins, error) { + acct, err := updateCoins(store, addr, coins) return acct.Coins, err } // ChangeCoins changes the money, returns error if it would be negative -func ChangeCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (Coins, error) { - acct, err := updateCoins(store, addr, coins, seq) +func ChangeCoins(store state.KVStore, addr basecoin.Actor, coins Coins) (Coins, error) { + acct, err := updateCoins(store, addr, coins) if err != nil { return acct.Coins, err } @@ -41,7 +41,7 @@ func ChangeCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) // updateCoins will load the account, make all checks, and return the updated account. // // it doesn't save anything, that is up to you to decide (Check/Change Coins) -func updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (acct Account, err error) { +func updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins) (acct Account, err error) { acct, err = loadAccount(store, addr.Bytes()) // we can increase an empty account... if IsNoAccountErr(err) && coins.IsPositive() { @@ -51,14 +51,6 @@ func updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) return acct, err } - // check sequence if we are deducting... ugh, need a cleaner replay protection - if !coins.IsPositive() { - if seq != acct.Sequence+1 { - return acct, ErrInvalidSequence() - } - acct.Sequence++ - } - // check amount final := acct.Coins.Plus(coins) if !final.IsNonnegative() { diff --git a/modules/coin/tx.go b/modules/coin/tx.go index 147f0ec84..b3598970e 100644 --- a/modules/coin/tx.go +++ b/modules/coin/tx.go @@ -20,9 +20,8 @@ const ( // TxInput - expected coin movement outputs, used with SendTx type TxInput struct { - Address basecoin.Actor `json:"address"` - Coins Coins `json:"coins"` - Sequence int `json:"sequence"` // Nonce: Must be 1 greater than the last committed TxInput + Address basecoin.Actor `json:"address"` + Coins Coins `json:"coins"` } // ValidateBasic - validate transaction input @@ -40,22 +39,18 @@ func (txIn TxInput) ValidateBasic() error { if !txIn.Coins.IsPositive() { return ErrInvalidCoins() } - if txIn.Sequence <= 0 { - return ErrInvalidSequence() - } return nil } func (txIn TxInput) String() string { - return fmt.Sprintf("TxInput{%v,%v,%v}", txIn.Address, txIn.Coins, txIn.Sequence) + return fmt.Sprintf("TxInput{%v,%v}", txIn.Address, txIn.Coins) } // NewTxInput - create a transaction input, used with SendTx -func NewTxInput(addr basecoin.Actor, coins Coins, sequence int) TxInput { +func NewTxInput(addr basecoin.Actor, coins Coins) TxInput { input := TxInput{ - Address: addr, - Coins: coins, - Sequence: sequence, + Address: addr, + Coins: coins, } return input } @@ -110,11 +105,19 @@ type SendTx struct { var _ basecoin.Tx = NewSendTx(nil, nil) -// NewSendTx - new SendTx +// NewSendTx - construct arbitrary multi-in, multi-out sendtx func NewSendTx(in []TxInput, out []TxOutput) basecoin.Tx { return SendTx{Inputs: in, Outputs: out}.Wrap() } +// NewSendOneTx is a helper for the standard (?) case where there is exactly +// one sender and one recipient +func NewSendOneTx(sender, recipient basecoin.Actor, amount Coins) basecoin.Tx { + in := []TxInput{{Address: sender, Coins: amount}} + out := []TxOutput{{Address: recipient, Coins: amount}} + return SendTx{Inputs: in, Outputs: out}.Wrap() +} + // ValidateBasic - validate the send transaction func (tx SendTx) ValidateBasic() error { // this just makes sure all the inputs and outputs are properly formatted, diff --git a/modules/coin/tx_test.go b/modules/coin/tx_test.go index 9aa0699ed..1b292ae28 100644 --- a/modules/coin/tx_test.go +++ b/modules/coin/tx_test.go @@ -46,26 +46,14 @@ var coins = []struct { func TestTxValidateInput(t *testing.T) { assert := assert.New(t) - seqs := []struct { - seq int - valid bool - }{ - {-3, false}, - {0, false}, - {1, true}, - {6571265735, true}, - } - for i, act := range actors { for j, coin := range coins { - for k, seq := range seqs { - input := NewTxInput(act.actor, coin.coins, seq.seq) - err := input.ValidateBasic() - if act.valid && coin.valid && seq.valid { - assert.Nil(err, "%d,%d,%d: %+v", i, j, k, err) - } else { - assert.NotNil(err, "%d,%d,%d", i, j, k) - } + input := NewTxInput(act.actor, coin.coins) + err := input.ValidateBasic() + if act.valid && coin.valid { + assert.Nil(err, "%d,%d: %+v", i, j, err) + } else { + assert.NotNil(err, "%d,%d", i, j) } } } @@ -111,15 +99,15 @@ func TestTxValidateTx(t *testing.T) { }{ // 0-2. valid cases {true, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}, )}, {true, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, otherCoins, 5)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, otherCoins)}, []TxOutput{NewTxOutput(addr3, bothCoins)}, )}, {true, NewSendTx( - []TxInput{NewTxInput(addr1, bothCoins, 42)}, + []TxInput{NewTxInput(addr1, bothCoins)}, []TxOutput{NewTxOutput(addr2, someCoins), NewTxOutput(addr3, otherCoins)}, )}, @@ -129,39 +117,35 @@ func TestTxValidateTx(t *testing.T) { []TxOutput{NewTxOutput(addr2, someCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, nil, )}, - // 5-8. invalid inputs + // 5-7. invalid inputs {false, NewSendTx( - []TxInput{NewTxInput(noAddr, someCoins, 2)}, + []TxInput{NewTxInput(noAddr, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, -1)}, - []TxOutput{NewTxOutput(addr2, someCoins)}, - )}, - {false, NewSendTx( - []TxInput{NewTxInput(addr1, noCoins, 2)}, + []TxInput{NewTxInput(addr1, noCoins)}, []TxOutput{NewTxOutput(addr2, noCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, minusCoins, 2)}, + []TxInput{NewTxInput(addr1, minusCoins)}, []TxOutput{NewTxOutput(addr2, minusCoins)}, )}, - // 9-11. totals don't match + // 8-10. totals don't match {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 7)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, moreCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, minusCoins, 5)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, minusCoins)}, []TxOutput{NewTxOutput(addr3, someCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, moreCoins, 5)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, moreCoins)}, []TxOutput{NewTxOutput(addr3, bothCoins)}, )}, } @@ -185,7 +169,7 @@ func TestTxSerializeTx(t *testing.T) { someCoins := Coins{{"atom", 123}} send := NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}, )