frey changes

errors/common.go

@@ -15,12 +15,15 @@ var (
 	errUnauthorized      = fmt.Errorf("Unauthorized")
 	errInvalidSignature  = fmt.Errorf("Invalid Signature")
 	errTooLarge          = fmt.Errorf("Input size too large")
+	errNoSigners         = fmt.Errorf("There are no signers")
 	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")
+	errBadNonce          = fmt.Errorf("Bad nonce sequence")
+	errTxEmpty           = fmt.Errorf("The provided Tx is empty")
+	errZeroSequence      = fmt.Errorf("Sequence number cannot be zero")
 	errWrongChain        = fmt.Errorf("Wrong chain for tx")
 	errUnknownTxType     = fmt.Errorf("Tx type unknown")
 	errInvalidFormat     = fmt.Errorf("Invalid format")
@@ -107,6 +110,10 @@ func IsUnauthorizedErr(err error) bool {
 	return HasErrorCode(err, unauthorized)
 }
 
+func ErrNoSigners() TMError {
+	return WithCode(errNoSigners, unauthorized)
+}
+
 func ErrMissingSignature() TMError {
 	return WithCode(errMissingSignature, unauthorized)
 }
@@ -139,12 +146,20 @@ func ErrNoNonce() TMError {
 	return WithCode(errNoNonce, unauthorized)
 }
 func ErrBadNonce(got, expected uint32) TMError {
-	return WithCode(fmt.Errorf("Bad nonce seqence, got %d, expected %d", got, expected), unauthorized)
+	return WithCode(fmt.Errorf("Bad nonce sequence, got %d, expected %d", got, expected), unauthorized)
 }
 func ErrNotMember() TMError {
 	return WithCode(errBadNonce, unauthorized)
 }
 
+func ErrZeroSequence() TMError {
+	return WithCode(errZeroSequence, unauthorized)
+}
+
+func ErrTxEmpty() TMError {
+	return WithCode(errTxEmpty, unauthorized)
+}
+
 func ErrWrongChain(chain string) TMError {
 	msg := errors.Wrap(errWrongChain, chain)
 	return WithCode(msg, unauthorized)

modules/nonce/replaycheck.go

@@ -32,7 +32,17 @@ func (r ReplayCheck) CheckTx(ctx basecoin.Context, store state.KVStore,
 	if err != nil {
 		return res, err
 	}
-	return next.CheckTx(ctx, store, stx)
+
+	res, err = next.CheckTx(ctx, store, stx)
+	if err != nil {
+		return res, err
+	}
+
+	err = r.incrementNonceTx(ctx, store, tx)
+	if err != nil {
+		return res, err
+	}
+	return
 }
 
 // DeliverTx verifies tx is not being replayed - fulfills Middlware interface
@@ -43,7 +53,17 @@ func (r ReplayCheck) DeliverTx(ctx basecoin.Context, store state.KVStore,
 	if err != nil {
 		return res, err
 	}
-	return next.DeliverTx(ctx, store, stx)
+
+	res, err = next.DeliverTx(ctx, store, stx)
+	if err != nil {
+		return res, err
+	}
+
+	err = r.incrementNonceTx(ctx, store, tx)
+	if err != nil {
+		return res, err
+	}
+	return
 }
 
 // checkNonceTx varifies the nonce sequence
@@ -57,9 +77,27 @@ func (r ReplayCheck) checkNonceTx(ctx basecoin.Context, store state.KVStore,
 	}
 
 	// check the nonce sequence number
-	err := nonceTx.CheckIncrementSeq(ctx, store)
+	err := nonceTx.CheckSeq(ctx, store)
 	if err != nil {
 		return tx, err
 	}
 	return nonceTx.Tx, nil
 }
+
+// incrementNonceTx increases the nonce sequence number
+func (r ReplayCheck) incrementNonceTx(ctx basecoin.Context, store state.KVStore,
+	tx basecoin.Tx) error {
+
+	// make sure it is a the nonce Tx (Tx from this package)
+	nonceTx, ok := tx.Unwrap().(Tx)
+	if !ok {
+		return errors.ErrNoNonce()
+	}
+
+	// check the nonce sequence number
+	err := nonceTx.IncrementSeq(ctx, store)
+	if err != nil {
+		return err
+	}
+	return nil
+}

modules/nonce/tx.go

@@ -1,5 +1,9 @@
 /*
-Package nonce XXX
+Package nonce - This module allows replay protection to be added to process stack.
+This is achieved through the use of a sequence number for each unique set of signers.
+Note that the sequence number for the single signing account "foo" will be unique
+from the sequence number for a multi-sig account {"foo", "bar"} which would also be
+unique from a different multi-sig account {"foo", "soup"}
 */
 package nonce
 
@@ -9,8 +13,6 @@ import (
 	"github.com/tendermint/basecoin"
 	"github.com/tendermint/basecoin/errors"
 	"github.com/tendermint/basecoin/state"
-
-	"github.com/tendermint/tmlibs/merkle"
 )
 
 // nolint
@@ -23,7 +25,7 @@ func init() {
 	basecoin.TxMapper.RegisterImplementation(Tx{}, TypeNonce, ByteNonce)
 }
 
-// Tx - XXX fill in
+// Tx - Nonce transaction structure, contains list of signers and current sequence number
 type Tx struct {
 	Sequence uint32           `json:"sequence"`
 	Signers  []basecoin.Actor `json:"signers"`
@@ -46,23 +48,22 @@ func (n Tx) Wrap() basecoin.Tx {
 	return basecoin.Tx{n}
 }
 func (n Tx) ValidateBasic() error {
-	// rigel: check if Sequence ==  0, len(Signers) == 0, or Tx.Empty()
+	switch {
-	// these are all invalid, regardless of the state
+	case n.Tx.Empty():
-	// (also add max sequence number to prevent overflow?)
+		return errors.ErrTxEmpty()
+	case n.Sequence == 0:
+		return errors.ErrZeroSequence()
+	case len(n.Signers) == 0:
+		return errors.ErrNoSigners()
+	}
 	return n.Tx.ValidateBasic()
 }
 
-// CheckIncrementSeq - XXX fill in
+// CheckSeq - Check that the sequence number is one more than the state sequence number
-func (n Tx) CheckIncrementSeq(ctx basecoin.Context, store state.KVStore) error {
+// and further increment the sequence number
+func (n Tx) CheckSeq(ctx basecoin.Context, store state.KVStore) error {
 
-	// rigel: nice with the sort, problem is this modifies the TX in place...
+	seqKey := n.getSeqKey()
-	// if we reserialize the tx after this function, it will be a different
-	// representations... copy n.Signers before sorting them please
-
-	//Generate the sequence key as the hash of the list of signers, sorted by address
-	sort.Sort(basecoin.ByAddress(n.Signers))
-	// rigel: nice sort, no need for a merkle hash... something simpler also works
-	seqKey := merkle.SimpleHashFromBinary(n.Signers)
 
 	// check the current state
 	cur, err := getSeq(store, seqKey)
@@ -79,12 +80,21 @@ func (n Tx) CheckIncrementSeq(ctx basecoin.Context, store state.KVStore) error {
 			return errors.ErrNotMember()
 		}
 	}
+	return nil
+}
 
-	// rigel: this should be separate.  we check the sequence on CheckTx and DeliverTx
+// IncrementSeq - increment the sequence for a group of actors
-	// BEFORE we execute the wrapped tx.
+func (n Tx) IncrementSeq(ctx basecoin.Context, store state.KVStore) error {
-	// we increment the sequence in DeliverTx AFTER it returns success (not on error)
 
-	//finally increment the sequence by 1
+	seqKey := n.getSeqKey()
+
+	// check the current state
+	cur, err := getSeq(store, seqKey)
+	if err != nil {
+		return err
+	}
+
+	// increment the sequence by 1
 	err = setSeq(store, seqKey, cur+1)
 	if err != nil {
 		return err
@@ -92,3 +102,18 @@ func (n Tx) CheckIncrementSeq(ctx basecoin.Context, store state.KVStore) error {
 
 	return nil
 }
+
+// Generate the sequence key as the concatenated list of signers, sorted by address.
+func (n Tx) getSeqKey() (seqKey []byte) {
+
+	// First copy the list of signers to sort as sort is done in place
+	signers2sort := make([]basecoin.Actor, len(n.Signers))
+	copy(signers2sort, n.Signers)
+	sort.Sort(basecoin.ByAddress(n.Signers))
+
+	for _, signer := range n.Signers {
+		seqKey = append(seqKey, signer.Address...)
+	}
+	//seqKey = merkle.SimpleHashFromBinary(n.Signers)
+	return
+}

modules/nonce/tx_test.go

@@ -18,14 +18,12 @@ func TestNonce(t *testing.T) {
 	// generic args here...
 	chainID := "my-chain"
 	height := uint64(100)
-	// rigel: use MockContext, so we can add permissions
 	ctx := stack.MockContext(chainID, height)
 	store := state.NewMemKVStore()
 
-	// rigel: you can leave chainID blank for the actors, note the comment on Actor struct
+	act1 := basecoin.Actor{App: "fooz", Address: []byte{1, 2, 3, 4}}
-	act1 := basecoin.Actor{ChainID: chainID, App: "fooz", Address: []byte{1, 2, 3, 4}}
+	act2 := basecoin.Actor{App: "fooz", Address: []byte{1, 1, 1, 1}}
-	act2 := basecoin.Actor{ChainID: chainID, App: "fooz", Address: []byte{1, 1, 1, 1}}
+	act3 := basecoin.Actor{App: "fooz", Address: []byte{3, 3, 3, 3}}
-	act3 := basecoin.Actor{ChainID: chainID, App: "fooz", Address: []byte{3, 3, 3, 3}}
 
 	// let's construct some tests to make the table a bit less verbose
 	set0 := []basecoin.Actor{}
@@ -36,12 +34,10 @@ func TestNonce(t *testing.T) {
 	set123 := []basecoin.Actor{act1, act2, act3}
 	set321 := []basecoin.Actor{act3, act2, act1}
 
-	// rigel: test cases look good, but also add reordering
 	testList := []struct {
-		valid  bool
+		valid   bool
-		seq    uint32
+		seq     uint32
-		actors []basecoin.Actor
+		actors  []basecoin.Actor
-		// rigel: you forgot to sign the tx, of course the get rejected...
 		signers []basecoin.Actor
 	}{
 		// one signer
@@ -72,17 +68,17 @@ func TestNonce(t *testing.T) {
 		{false, 2, set321, set321}, // no repetition
 	}
 
-	// rigel: don't wrap nil, it's bad, wrap a raw byte thing
 	raw := stack.NewRawTx([]byte{42})
 	for i, test := range testList {
-		// rigel: set the permissions
+
+		// set the permissions
 		myCtx := ctx.WithPermissions(test.signers...)
 
 		tx := NewTx(test.seq, test.actors, raw)
 		nonceTx, ok := tx.Unwrap().(Tx)
 		require.True(ok)
 
-		err := nonceTx.CheckIncrementSeq(myCtx, store)
+		err := nonceTx.CheckSeq(myCtx, store)
 		if test.valid {
 			assert.Nil(err, "%d: %+v", i, err)
 		} else {

tests/cli/common.sh

@@ -161,6 +161,7 @@ checkSendTx() {
     assertEquals "proper height" $2 $(echo $TX | jq .height)
     assertEquals "type=sigs/one" '"sigs/one"' $(echo $TX | jq .data.type)
     CTX=$(echo $TX | jq .data.data.tx)
+    assertEquals "type=nonce" '"nonce"' $(echo $CTX | jq .type)
     assertEquals "type=chain/tx" '"chain/tx"' $(echo $CTX | jq .type)
     STX=$(echo $CTX | jq .data.tx)
     assertEquals "type=coin/send" '"coin/send"' $(echo $STX | jq .type)