From 16b039534d46099eeff9b1d0660502f74115783c Mon Sep 17 00:00:00 2001 From: rigel rozanski Date: Wed, 12 Jul 2017 05:02:16 -0400 Subject: [PATCH] working sequence number with errors --- app/app_test.go | 5 +- cmd/basecli/commands/cmds.go | 20 ++++++- context.go | 21 +++++++ .../cmd/countercli/commands/counter.go | 4 ++ docs/guide/counter/plugins/counter/counter.go | 9 ++- .../counter/plugins/counter/counter_test.go | 16 ++--- errors/common.go | 50 ++++++++++++---- modules/coin/errors.go | 7 --- modules/nonce/replaycheck.go | 44 ++++++++++---- modules/nonce/store.go | 30 ++++++++++ modules/nonce/tx.go | 60 ++++++++++++------- 11 files changed, 196 insertions(+), 70 deletions(-) create mode 100644 modules/nonce/store.go diff --git a/app/app_test.go b/app/app_test.go index cbb446bb05..9d182c7dd1 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -8,14 +8,16 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - abci "github.com/tendermint/abci/types" "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/modules/auth" "github.com/tendermint/basecoin/modules/base" "github.com/tendermint/basecoin/modules/coin" "github.com/tendermint/basecoin/modules/fee" + "github.com/tendermint/basecoin/modules/nonce" "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/state" + + abci "github.com/tendermint/abci/types" wire "github.com/tendermint/go-wire" eyes "github.com/tendermint/merkleeyes/client" "github.com/tendermint/tmlibs/log" @@ -58,6 +60,7 @@ func (at *appTest) signTx(tx basecoin.Tx) basecoin.Tx { func (at *appTest) getTx(coins coin.Coins) basecoin.Tx { tx := at.baseTx(coins) tx = base.NewChainTx(at.chainID, 0, tx) + tx = nonce.NewTx(tx, 0, []basecoin.Actor{at.acctIn.Actor()}) return at.signTx(tx) } diff --git a/cmd/basecli/commands/cmds.go b/cmd/basecli/commands/cmds.go index d069a838ab..c451d35283 100644 --- a/cmd/basecli/commands/cmds.go +++ b/cmd/basecli/commands/cmds.go @@ -2,6 +2,7 @@ package commands import ( "encoding/hex" + "fmt" "strings" "github.com/pkg/errors" @@ -50,7 +51,7 @@ func init() { flags.Int(FlagSequence, -1, "Sequence number for this transaction") } -// runDemo is an example of how to make a tx +// doSendTx is an example of how to make a tx func doSendTx(cmd *cobra.Command, args []string) error { // load data from json or flags var tx basecoin.Tx @@ -65,6 +66,15 @@ func doSendTx(cmd *cobra.Command, args []string) error { return err } + sendTx, ok := tx.Unwrap().(coin.SendTx) + if !ok { + return errors.New("tx not SendTx") + } + var nonceAccount []basecoin.Actor + for _, input := range sendTx.Inputs { + nonceAccount = append(nonceAccount, input.Address) + } + // TODO: make this more flexible for middleware tx, err = WrapFeeTx(tx) if err != nil { @@ -75,8 +85,12 @@ func doSendTx(cmd *cobra.Command, args []string) error { return err } - // XXX - what is the nonceAccount here!!! - tx = nonce.NewTx(tx, viper.GetInt(FlagSequence), nonceAccount) + //add the nonce tx layer to the tx + seq := viper.GetInt(FlagSequence) + if seq < 0 { + return fmt.Errorf("sequence must be greater than 0") + } + tx = nonce.NewTx(tx, uint32(seq), nonceAccount) // XXX - what is the nonceAccount here!!! // Note: this is single sig (no multi sig yet) stx := auth.NewSig(tx) diff --git a/context.go b/context.go index d67fa0d9ae..1308e797c3 100644 --- a/context.go +++ b/context.go @@ -2,6 +2,8 @@ package basecoin import ( "bytes" + "fmt" + "sort" wire "github.com/tendermint/go-wire" "github.com/tendermint/go-wire/data" @@ -19,6 +21,7 @@ type Actor struct { Address data.Bytes `json:"addr"` // arbitrary app-specific unique id } +// NewActor - create a new actor func NewActor(app string, addr []byte) Actor { return Actor{App: app, Address: addr} } @@ -48,3 +51,21 @@ type Context interface { ChainID() string BlockHeight() uint64 } + +//////////////////////////////// Sort Interface +// USAGE sort.Sort(ByAddress()) + +func (a Actor) String() string { + return fmt.Sprintf("%x", a.Address) +} + +// ByAddress implements sort.Interface for []Actor based on +// the Address field. +type ByAddress []Actor + +// Verify the sort interface at compile time +var _ sort.Interface = ByAddress{} + +func (a ByAddress) Len() int { return len(a) } +func (a ByAddress) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a ByAddress) Less(i, j int) bool { return bytes.Compare(a[i].Address, a[j].Address) == -1 } diff --git a/docs/guide/counter/cmd/countercli/commands/counter.go b/docs/guide/counter/cmd/countercli/commands/counter.go index 9e420b008c..0e340d01f0 100644 --- a/docs/guide/counter/cmd/countercli/commands/counter.go +++ b/docs/guide/counter/cmd/countercli/commands/counter.go @@ -83,6 +83,10 @@ func readCounterTxFlags() (tx basecoin.Tx, err error) { return tx, err } +<<<<<<< HEAD tx = counter.NewTx(viper.GetBool(FlagValid), feeCoins, viper.GetInt(bcmd.FlagSequence)) +======= + tx = counter.NewTx(viper.GetBool(FlagValid), feeCoins) +>>>>>>> working sequence number with errors return tx, nil } diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index b1a7b86088..7a4eb9e4cb 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -36,15 +36,14 @@ func init() { type Tx struct { Valid bool `json:"valid"` Fee coin.Coins `json:"fee"` - Sequence int `json:"sequence"` + Sequence int `json:""` } // NewTx - return a new counter transaction struct wrapped as a basecoin transaction -func NewTx(valid bool, fee coin.Coins, sequence int) basecoin.Tx { +func NewTx(valid bool, fee coin.Coins) basecoin.Tx { return Tx{ - Valid: valid, - Fee: fee, - Sequence: sequence, + Valid: valid, + Fee: fee, }.Wrap() } diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index 60c5101fd1..919c42dc75 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -40,8 +40,8 @@ func TestCounterPlugin(t *testing.T) { require.Equal(t, "Success", log) // Deliver a CounterTx - DeliverCounterTx := func(valid bool, counterFee coin.Coins, inputSequence int) abci.Result { - tx := NewTx(valid, counterFee, inputSequence) + DeliverCounterTx := func(valid bool, counterFee coin.Coins) abci.Result { + tx := NewTx(valid, counterFee) tx = base.NewChainTx(chainID, 0, tx) stx := auth.NewSig(tx) auth.Sign(stx, acct.Key) @@ -49,19 +49,19 @@ func TestCounterPlugin(t *testing.T) { return bcApp.DeliverTx(txBytes) } - // Test a basic send, no fee (doesn't update sequence as no money spent) - res := DeliverCounterTx(true, nil, 1) + // Test a basic send, no fee + res := DeliverCounterTx(true, nil) assert.True(res.IsOK(), res.String()) // Test an invalid send, no fee - res = DeliverCounterTx(false, nil, 1) + res = DeliverCounterTx(false, nil) assert.True(res.IsErr(), res.String()) - // Test the fee (increments sequence) - res = DeliverCounterTx(true, coin.Coins{{"gold", 100}}, 1) + // Test an invalid send, with supported fee + res = DeliverCounterTx(true, coin.Coins{{"gold", 100}}) assert.True(res.IsOK(), res.String()) // Test unsupported fee - res = DeliverCounterTx(true, coin.Coins{{"silver", 100}}, 2) + res = DeliverCounterTx(true, coin.Coins{{"silver", 100}}) assert.True(res.IsErr(), res.String()) } diff --git a/errors/common.go b/errors/common.go index 81d786beee..13aaf3d488 100644 --- a/errors/common.go +++ b/errors/common.go @@ -12,12 +12,16 @@ import ( var ( errDecoding = fmt.Errorf("Error decoding input") + errNoAccount = fmt.Errorf("No such account") errUnauthorized = fmt.Errorf("Unauthorized") errInvalidSignature = fmt.Errorf("Invalid Signature") errTooLarge = fmt.Errorf("Input size too large") errMissingSignature = fmt.Errorf("Signature missing") errTooManySignatures = fmt.Errorf("Too many signatures") errNoChain = fmt.Errorf("No chain id provided") + errNoNonce = fmt.Errorf("Tx doesn't contain nonce") + errNotMember = fmt.Errorf("nonce contains non-permissioned member") + errBadNonce = fmt.Errorf("Bad nonce seqence") errWrongChain = fmt.Errorf("Wrong chain for tx") errUnknownTxType = fmt.Errorf("Tx type unknown") errInvalidFormat = fmt.Errorf("Invalid format") @@ -26,6 +30,14 @@ var ( errUnknownKey = fmt.Errorf("Unknown key") ) +var ( + internalErr = abci.CodeType_InternalError + encodingErr = abci.CodeType_EncodingError + unauthorized = abci.CodeType_Unauthorized + unknownRequest = abci.CodeType_UnknownRequest + unknownAddress = abci.CodeType_BaseUnknownAddress +) + // some crazy reflection to unwrap any generated struct. func unwrap(i interface{}) interface{} { v := reflect.ValueOf(i) @@ -71,76 +83,90 @@ func IsUnknownKeyErr(err error) bool { } func ErrInternal(msg string) TMError { - return New(msg, abci.CodeType_InternalError) + return New(msg, internalErr) } // IsInternalErr matches any error that is not classified func IsInternalErr(err error) bool { - return HasErrorCode(err, abci.CodeType_InternalError) + return HasErrorCode(err, internalErr) } func ErrDecoding() TMError { - return WithCode(errDecoding, abci.CodeType_EncodingError) + return WithCode(errDecoding, encodingErr) } func IsDecodingErr(err error) bool { return IsSameError(errDecoding, err) } func ErrUnauthorized() TMError { - return WithCode(errUnauthorized, abci.CodeType_Unauthorized) + return WithCode(errUnauthorized, unauthorized) } // IsUnauthorizedErr is generic helper for any unauthorized errors, // also specific sub-types func IsUnauthorizedErr(err error) bool { - return HasErrorCode(err, abci.CodeType_Unauthorized) + return HasErrorCode(err, unauthorized) } func ErrMissingSignature() TMError { - return WithCode(errMissingSignature, abci.CodeType_Unauthorized) + return WithCode(errMissingSignature, unauthorized) } func IsMissingSignatureErr(err error) bool { return IsSameError(errMissingSignature, err) } func ErrTooManySignatures() TMError { - return WithCode(errTooManySignatures, abci.CodeType_Unauthorized) + return WithCode(errTooManySignatures, unauthorized) } func IsTooManySignaturesErr(err error) bool { return IsSameError(errTooManySignatures, err) } func ErrInvalidSignature() TMError { - return WithCode(errInvalidSignature, abci.CodeType_Unauthorized) + return WithCode(errInvalidSignature, unauthorized) } func IsInvalidSignatureErr(err error) bool { return IsSameError(errInvalidSignature, err) } func ErrNoChain() TMError { - return WithCode(errNoChain, abci.CodeType_Unauthorized) + return WithCode(errNoChain, unauthorized) } func IsNoChainErr(err error) bool { return IsSameError(errNoChain, err) } +func ErrNoNonce() TMError { + return WithCode(errNoNonce, unauthorized) +} +func ErrBadNonce() TMError { + return WithCode(errBadNonce, unauthorized) +} +func ErrNotMember() TMError { + return WithCode(errBadNonce, unauthorized) +} + +func ErrNoAccount() TMError { + return WithCode(errNoAccount, unknownAddress) +} + func ErrWrongChain(chain string) TMError { msg := errors.Wrap(errWrongChain, chain) - return WithCode(msg, abci.CodeType_Unauthorized) + return WithCode(msg, unauthorized) } func IsWrongChainErr(err error) bool { return IsSameError(errWrongChain, err) } func ErrTooLarge() TMError { - return WithCode(errTooLarge, abci.CodeType_EncodingError) + return WithCode(errTooLarge, encodingErr) } func IsTooLargeErr(err error) bool { return IsSameError(errTooLarge, err) } func ErrExpired() TMError { - return WithCode(errExpired, abci.CodeType_Unauthorized) + return WithCode(errExpired, unauthorized) } func IsExpiredErr(err error) bool { return IsSameError(errExpired, err) diff --git a/modules/coin/errors.go b/modules/coin/errors.go index bc3d7c5b78..d245354bff 100644 --- a/modules/coin/errors.go +++ b/modules/coin/errors.go @@ -60,13 +60,6 @@ func IsInvalidCoinsErr(err error) bool { return errors.IsSameError(errInvalidCoins, err) } -func ErrInvalidSequence() errors.TMError { - return errors.WithCode(errInvalidSequence, invalidInput) -} -func IsInvalidSequenceErr(err error) bool { - return errors.IsSameError(errInvalidSequence, err) -} - func ErrInsufficientFunds() errors.TMError { return errors.WithCode(errInsufficientFunds, invalidInput) } diff --git a/modules/nonce/replaycheck.go b/modules/nonce/replaycheck.go index 9e75f0b181..1412bfb165 100644 --- a/modules/nonce/replaycheck.go +++ b/modules/nonce/replaycheck.go @@ -2,6 +2,7 @@ package nonce import ( "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/state" ) @@ -11,8 +12,7 @@ const ( NameNonce = "nonce" ) -// ReplayCheck parses out go-crypto signatures and adds permissions to the -// context for use inside the application +// ReplayCheck uses the sequence to check for replay attacks type ReplayCheck struct { stack.PassOption } @@ -24,22 +24,42 @@ func (ReplayCheck) Name() string { var _ stack.Middleware = ReplayCheck{} -// CheckTx verifies the signatures are correct - fulfills Middlware interface -func (ReplayCheck) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { - sigs, tnext, err := getSigners(tx) +// CheckTx verifies tx is not being replayed - fulfills Middlware interface +func (r ReplayCheck) CheckTx(ctx basecoin.Context, store state.KVStore, + tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { + + stx, err := r.checkNonceTx(ctx, store, tx) if err != nil { return res, err } - ctx2 := addSigners(ctx, sigs) - return next.CheckTx(ctx2, store, tnext) + return next.CheckTx(ctx, store, stx) } -// DeliverTx verifies the signatures are correct - fulfills Middlware interface -func (ReplayCheck) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { - sigs, tnext, err := getSigners(tx) +// DeliverTx verifies tx is not being replayed - fulfills Middlware interface +func (r ReplayCheck) DeliverTx(ctx basecoin.Context, store state.KVStore, + tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { + + stx, err := r.checkNonceTx(ctx, store, tx) if err != nil { return res, err } - ctx2 := addSigners(ctx, sigs) - return next.DeliverTx(ctx2, store, tnext) + return next.DeliverTx(ctx, store, stx) +} + +// checkNonceTx varifies the nonce sequence +func (r ReplayCheck) checkNonceTx(ctx basecoin.Context, store state.KVStore, + tx basecoin.Tx) (basecoin.Tx, error) { + + // make sure it is a the nonce Tx (Tx from this package) + nonceTx, ok := tx.Unwrap().(Tx) + if !ok { + return tx, errors.ErrNoNonce() + } + + // check the nonce sequence number + err := nonceTx.CheckIncrementSeq(ctx, store) + if err != nil { + return tx, err + } + return nonceTx.Tx, nil } diff --git a/modules/nonce/store.go b/modules/nonce/store.go new file mode 100644 index 0000000000..51f6cdb1dd --- /dev/null +++ b/modules/nonce/store.go @@ -0,0 +1,30 @@ +package nonce + +import ( + "fmt" + + wire "github.com/tendermint/go-wire" + + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/state" +) + +func getSeq(store state.KVStore, key []byte) (seq uint32, err error) { + // fmt.Printf("load: %X\n", key) + data := store.Get(key) + if len(data) == 0 { + return seq, errors.ErrNoAccount() + } + err = wire.ReadBinaryBytes(data, &seq) + if err != nil { + msg := fmt.Sprintf("Error reading sequence for %X", key) + return seq, errors.ErrInternal(msg) + } + return seq, nil +} + +func setSeq(store state.KVStore, key []byte, seq uint32) error { + bin := wire.BinaryBytes(seq) + store.Set(key, bin) + return nil // real stores can return error... +} diff --git a/modules/nonce/tx.go b/modules/nonce/tx.go index 741340cfdf..95773a0895 100644 --- a/modules/nonce/tx.go +++ b/modules/nonce/tx.go @@ -4,66 +4,82 @@ Package nonce XXX package nonce import ( - "github.com/tendermint/basecoin/state" + "sort" "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/state" + + "github.com/tendermint/tmlibs/merkle" ) // nolint const ( - // for signatures - ByteSingleTx = 0x16 - ByteMultiSig = 0x17 + ByteNonce = 0x69 //TODO overhaul byte assign system don't make no sense! + TypeNonce = "nonce" ) -/**** Registration ****/ - -//func init() { -//basecoin.TxMapper.RegisterImplementation(&Tx{}, TypeSingleTx, ByteSingleTx) -//} +func init() { + basecoin.TxMapper.RegisterImplementation(&Tx{}, TypeNonce, ByteNonce) +} // Tx - XXX fill in type Tx struct { Tx basecoin.Tx `json:p"tx"` Sequence uint32 Signers []basecoin.Actor // or simple []data.Bytes (they are only pubkeys...) + seqKey []byte //key to store the sequence number } var _ basecoin.TxInner = &Tx{} // NewTx wraps the tx with a signable nonce -func NewTx(tx basecoin.Tx, sequence uint32, signers []basecoin.Actor) *Tx { - return &Tx{ +func NewTx(tx basecoin.Tx, sequence uint32, signers []basecoin.Actor) basecoin.Tx { + + //Generate the sequence key as the hash of the list of signers, sorted by address + sort.Sort(basecoin.ByAddress(signers)) + seqKey := merkle.SimpleHashFromBinary(signers) + + return (Tx{ Tx: tx, Sequence: sequence, Signers: signers, - } + seqKey: seqKey, + }).Wrap() } //nolint -func (n *Tx) Wrap() basecoin.Tx { - return basecoin.Tx{s} +func (n Tx) Wrap() basecoin.Tx { + return basecoin.Tx{n} } -func (n *Tx) ValidateBasic() error { - return s.Tx.ValidateBasic() +func (n Tx) ValidateBasic() error { + return n.Tx.ValidateBasic() } -// CheckSequence - XXX fill in -func (n *Tx) CheckSequence(ctx basecoin.Context, store state.KVStore) error { +// CheckIncrementSeq - XXX fill in +func (n Tx) CheckIncrementSeq(ctx basecoin.Context, store state.KVStore) error { // check the current state - h := hash(Sort(n.Signers)) - cur := loadSeq(store, h) + cur, err := getSeq(store, n.seqKey) + if err != nil { + return err + } if n.Sequence != cur+1 { - return ErrBadNonce() + return errors.ErrBadNonce() } // make sure they all signed for _, s := range n.Signers { if !ctx.HasPermission(s) { - return ErrNotMember() + return errors.ErrNotMember() } } + //finally increment the sequence by 1 + err = setSeq(store, n.seqKey, cur+1) + if err != nil { + return err + } + return nil }