Clean up error handling

errors/common.go

@@ -7,14 +7,17 @@ package errors
 import abci "github.com/tendermint/abci/types"
 
 const (
-	msgDecoding        = "Error decoding input"
-	msgUnauthorized    = "Unauthorized"
-	msgInvalidAddress  = "Invalid Address"
-	msgInvalidCoins    = "Invalid Coins"
-	msgInvalidSequence = "Invalid Sequence"
-	msgNoInputs        = "No Input Coins"
-	msgNoOutputs       = "No Output Coins"
-	msgTooLarge        = "Input size too large"
+	msgDecoding          = "Error decoding input"
+	msgUnauthorized      = "Unauthorized"
+	msgInvalidAddress    = "Invalid Address"
+	msgInvalidCoins      = "Invalid Coins"
+	msgInvalidSequence   = "Invalid Sequence"
+	msgInvalidSignature  = "Invalid Signature"
+	msgNoInputs          = "No Input Coins"
+	msgNoOutputs         = "No Output Coins"
+	msgTooLarge          = "Input size too large"
+	msgMissingSignature  = "Signature missing"
+	msgTooManySignatures = "Too many signatures"
 )
 
 func DecodingError() TMError {
@@ -25,6 +28,18 @@ func Unauthorized() TMError {
 	return New(msgUnauthorized, abci.CodeType_Unauthorized)
 }
 
+func MissingSignature() TMError {
+	return New(msgMissingSignature, abci.CodeType_Unauthorized)
+}
+
+func TooManySignatures() TMError {
+	return New(msgTooManySignatures, abci.CodeType_Unauthorized)
+}
+
+func InvalidSignature() TMError {
+	return New(msgInvalidSignature, abci.CodeType_Unauthorized)
+}
+
 func InvalidAddress() TMError {
 	return New(msgInvalidAddress, abci.CodeType_BaseInvalidInput)
 }

tx.go

@@ -1,12 +1,23 @@
 package basecoin
 
-// TODO: add some common functionality here...
+// TxInner is the interface all concrete transactions should implement.
+//
+// It adds bindings for clean un/marhsaling of the various implementations
+// both as json and binary, as well as some common functionality to move them.
+//
 // +gen wrapper:"Tx"
 type TxInner interface {
 	Wrap() Tx
+
+	// ValidateBasic should be a stateless check and just verify that the
+	// tx is properly formated (required strings not blank, signatures exist, etc.)
+	// this can also be run on the client-side for better debugging before posting a tx
 	ValidateBasic() error
 }
 
+// TODO: do we need this abstraction? TxLayer???
+// please review again after implementing "middleware"
+
 // TxLayer provides a standard way to deal with "middleware" tx,
 // That add context to an embedded tx.
 type TxLayer interface {

tx_test.go

@@ -11,13 +11,15 @@ type Demo struct{}
 
 var _ TxLayer = Demo{}
 
-func (d Demo) Next() Tx { return Tx{} }
-func (d Demo) Wrap() Tx { return Tx{d} }
+func (d Demo) Next() Tx             { return Tx{} }
+func (d Demo) Wrap() Tx             { return Tx{d} }
+func (d Demo) ValidateBasic() error { return nil }
 
 // define a Fake struct that doesn't implement TxLayer
 type Fake struct{}
 
-func (f Fake) Wrap() Tx { return Tx{f} }
+func (f Fake) Wrap() Tx             { return Tx{f} }
+func (f Fake) ValidateBasic() error { return nil }
 
 // Make sure the layer
 func TestLayer(t *testing.T) {

txs/sigs.go

@@ -14,15 +14,12 @@ a basecoin.Tx.
 package txs
 
 import (
-	// TODO: merge in usage of pkg/errors into basecoin/errors and remove this
-	"github.com/pkg/errors"
-
 	crypto "github.com/tendermint/go-crypto"
 	"github.com/tendermint/go-crypto/keys"
 	"github.com/tendermint/go-wire/data"
 
 	"github.com/tendermint/basecoin"
-	berrs "github.com/tendermint/basecoin/errors"
+	"github.com/tendermint/basecoin/errors"
 )
 
 // Signed holds one signature of the data
@@ -64,7 +61,7 @@ func (s *OneSig) Wrap() basecoin.Tx {
 func (s *OneSig) ValidateBasic() error {
 	// TODO: VerifyBytes here, we do it in Signers?
 	if s.Empty() || !s.Pubkey.VerifyBytes(s.SignBytes(), s.Sig) {
-		return berrs.Unauthorized()
+		return errors.Unauthorized()
 	}
 	return s.Tx.ValidateBasic()
 }
@@ -90,10 +87,10 @@ func (s *OneSig) SignBytes() []byte {
 func (s *OneSig) Sign(pubkey crypto.PubKey, sig crypto.Signature) error {
 	signed := Signed{sig, pubkey}
 	if signed.Empty() {
-		return errors.New("Signature or Key missing")
+		return errors.MissingSignature()
 	}
 	if !s.Empty() {
-		return errors.New("Transaction can only be signed once")
+		return errors.TooManySignatures()
 	}
 	// set the value once we are happy
 	s.Signed = signed
@@ -105,10 +102,10 @@ func (s *OneSig) Sign(pubkey crypto.PubKey, sig crypto.Signature) error {
 // including if there are no signatures
 func (s *OneSig) Signers() ([]crypto.PubKey, error) {
 	if s.Empty() {
-		return nil, errors.New("Never signed")
+		return nil, errors.MissingSignature()
 	}
 	if !s.Pubkey.VerifyBytes(s.SignBytes(), s.Sig) {
-		return nil, errors.New("Signature doesn't match")
+		return nil, errors.InvalidSignature()
 	}
 	return []crypto.PubKey{s.Pubkey}, nil
 }
@@ -135,8 +132,7 @@ func (s *MultiSig) ValidateBasic() error {
 	// TODO: more efficient
 	_, err := s.Signers()
 	if err != nil {
-		// TODO: better return value
-		return berrs.Unauthorized()
+		return err
 	}
 	return s.Tx.ValidateBasic()
 }
@@ -162,7 +158,7 @@ func (s *MultiSig) SignBytes() []byte {
 func (s *MultiSig) Sign(pubkey crypto.PubKey, sig crypto.Signature) error {
 	signed := Signed{sig, pubkey}
 	if signed.Empty() {
-		return errors.New("Signature or Key missing")
+		return errors.MissingSignature()
 	}
 	// set the value once we are happy
 	s.Sigs = append(s.Sigs, signed)
@@ -174,7 +170,7 @@ func (s *MultiSig) Sign(pubkey crypto.PubKey, sig crypto.Signature) error {
 // including if there are no signatures
 func (s *MultiSig) Signers() ([]crypto.PubKey, error) {
 	if len(s.Sigs) == 0 {
-		return nil, errors.New("Never signed")
+		return nil, errors.MissingSignature()
 	}
 	// verify all the signatures before returning them
 	keys := make([]crypto.PubKey, len(s.Sigs))
@@ -182,7 +178,7 @@ func (s *MultiSig) Signers() ([]crypto.PubKey, error) {
 	for i := range s.Sigs {
 		ms := s.Sigs[i]
 		if !ms.Pubkey.VerifyBytes(data, ms.Sig) {
-			return nil, errors.Errorf("Signature %d doesn't match (key: %X)", i, ms.Pubkey.Bytes())
+			return nil, errors.InvalidSignature()
 		}
 		keys[i] = ms.Pubkey
 	}