Remove redundant validation, allow 0 amount inputs,

code payload errors and prevent SendTx overpayment
Signed-off-by: Silas Davis <silas@monax.io>

execution/errors/errors.go

@@ -32,6 +32,12 @@ const (
 	ErrorCodeEventPublish
 	ErrorCodeInvalidString
 	ErrorCodeEventMapping
+	ErrorCodeInvalidAddress
+	ErrorCodeDuplicateAddress
+	ErrorCodeInsufficientFunds
+	ErrorCodeOverpayment
+	ErrorCodeZeroPayment
+	ErrorCodeInvalidSequence
 )
 
 func (c Code) ErrorCode() Code {
@@ -39,6 +45,10 @@ func (c Code) ErrorCode() Code {
 }
 
 func (c Code) Error() string {
+	return fmt.Sprintf("Error %d: %s", c, c.String())
+}
+
+func (c Code) String() string {
 	switch c {
 	case ErrorCodeUnknownAddress:
 		return "Unknown address"
@@ -82,6 +92,18 @@ func (c Code) Error() string {
 		return "Event mapping error"
 	case ErrorCodeGeneric:
 		return "Generic error"
+	case ErrorCodeInvalidAddress:
+		return "Invalid address"
+	case ErrorCodeDuplicateAddress:
+		return "Duplicate address"
+	case ErrorCodeInsufficientFunds:
+		return "Insufficient funds"
+	case ErrorCodeOverpayment:
+		return "Overpayment"
+	case ErrorCodeZeroPayment:
+		return "Zero payment error"
+	case ErrorCodeInvalidSequence:
+		return "Invalid sequence number"
 	default:
 		return "Unknown error"
 	}

execution/errors/errors_test.go

@@ -2,6 +2,7 @@ package errors
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -9,7 +10,7 @@ import (
 )
 
 func TestErrorCode_MarshalJSON(t *testing.T) {
-	ec := NewCodedError(ErrorCodeDataStackOverflow, "arrg")
+	ec := NewCodedError(ErrorCodeDataStackOverflow, "arrgh")
 	bs, err := json.Marshal(ec)
 	require.NoError(t, err)
 
@@ -19,3 +20,8 @@ func TestErrorCode_MarshalJSON(t *testing.T) {
 
 	assert.Equal(t, ec, ecOut)
 }
+
+func TestCode_String(t *testing.T) {
+	err := ErrorCodeCodeOutOfBounds
+	fmt.Println(err.Error())
+}

execution/executors/call_context.go

@@ -58,7 +58,7 @@ func (ctx *CallContext) Precheck() (*acm.MutableAccount, acm.Account, error) {
 	if inAcc == nil {
 		ctx.Logger.InfoMsg("Cannot find input account",
 			"tx_input", ctx.tx.Input)
-		return nil, nil, payload.ErrTxInvalidAddress
+		return nil, nil, errors.ErrorCodeInvalidAddress
 	}
 
 	err = validateInput(inAcc, ctx.tx.Input)
@@ -70,7 +70,7 @@ func (ctx *CallContext) Precheck() (*acm.MutableAccount, acm.Account, error) {
 	if ctx.tx.Input.Amount < ctx.tx.Fee {
 		ctx.Logger.InfoMsg("Sender did not send enough to cover the fee",
 			"tx_input", ctx.tx.Input)
-		return nil, nil, payload.ErrTxInsufficientFunds
+		return nil, nil, errors.ErrorCodeInsufficientFunds
 	}
 
 	ctx.Logger.TraceMsg("Incrementing sequence number for CallTx",
@@ -191,7 +191,7 @@ func (ctx *CallContext) Deliver(inAcc, outAcc acm.Account, value uint64) error {
 					"caller_address", inAcc.Address(),
 					"callee_address", ctx.tx.Address)
 			}
-			ctx.CallEvents(payload.ErrTxInvalidAddress)
+			ctx.CallEvents(errors.ErrorCodeInvalidAddress)
 			return nil
 		}
 		callee = acm.AsMutableAccount(outAcc)

execution/executors/name_context.go

@@ -42,7 +42,7 @@ func (ctx *NameContext) Execute(txe *exec.TxExecution) error {
 	if inAcc == nil {
 		ctx.Logger.InfoMsg("Cannot find input account",
 			"tx_input", ctx.tx.Input)
-		return payload.ErrTxInvalidAddress
+		return errors.ErrorCodeInvalidAddress
 	}
 	// check permission
 	if !hasNamePermission(ctx.StateWriter, inAcc, ctx.Logger) {
@@ -57,7 +57,7 @@ func (ctx *NameContext) Execute(txe *exec.TxExecution) error {
 	if ctx.tx.Input.Amount < ctx.tx.Fee {
 		ctx.Logger.InfoMsg("Sender did not send enough to cover the fee",
 			"tx_input", ctx.tx.Input)
-		return payload.ErrTxInsufficientFunds
+		return errors.ErrorCodeInsufficientFunds
 	}
 
 	// validate the input strings

execution/executors/permissions_context.go

@@ -7,6 +7,7 @@ import (
 	"github.com/hyperledger/burrow/acm/state"
 	"github.com/hyperledger/burrow/blockchain"
 	"github.com/hyperledger/burrow/crypto"
+	"github.com/hyperledger/burrow/execution/errors"
 	"github.com/hyperledger/burrow/execution/exec"
 	"github.com/hyperledger/burrow/logging"
 	"github.com/hyperledger/burrow/logging/structure"
@@ -35,7 +36,7 @@ func (ctx *PermissionsContext) Execute(txe *exec.TxExecution) error {
 	if inAcc == nil {
 		ctx.Logger.InfoMsg("Cannot find input account",
 			"tx_input", ctx.tx.Input)
-		return payload.ErrTxInvalidAddress
+		return errors.ErrorCodeInvalidAddress
 	}
 
 	err = ctx.tx.PermArgs.EnsureValid()

execution/executors/send_context.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/hyperledger/burrow/acm/state"
 	"github.com/hyperledger/burrow/blockchain"
+	"github.com/hyperledger/burrow/execution/errors"
 	"github.com/hyperledger/burrow/execution/exec"
 	"github.com/hyperledger/burrow/logging"
 	"github.com/hyperledger/burrow/txs/payload"
@@ -49,7 +50,13 @@ func (ctx *SendContext) Execute(txe *exec.TxExecution) error {
 		return err
 	}
 	if outTotal > inTotal {
-		return payload.ErrTxInsufficientFunds
+		return errors.ErrorCodeInsufficientFunds
+	}
+	if outTotal < inTotal {
+		return errors.ErrorCodeOverpayment
+	}
+	if outTotal == 0 {
+		return errors.ErrorCodeZeroPayment
 	}
 
 	// Good! Adjust accounts

execution/executors/shared.go

@@ -6,6 +6,7 @@ import (
 	"github.com/hyperledger/burrow/acm"
 	"github.com/hyperledger/burrow/acm/state"
 	"github.com/hyperledger/burrow/crypto"
+	"github.com/hyperledger/burrow/execution/errors"
 	"github.com/hyperledger/burrow/logging"
 	"github.com/hyperledger/burrow/logging/structure"
 	"github.com/hyperledger/burrow/permission"
@@ -23,14 +24,14 @@ func getInputs(accountGetter state.AccountGetter,
 	for _, in := range ins {
 		// Account shouldn't be duplicated
 		if _, ok := accounts[in.Address]; ok {
-			return nil, payload.ErrTxDuplicateAddress
+			return nil, errors.ErrorCodeDuplicateAddress
 		}
 		acc, err := state.GetMutableAccount(accountGetter, in.Address)
 		if err != nil {
 			return nil, err
 		}
 		if acc == nil {
-			return nil, payload.ErrTxInvalidAddress
+			return nil, errors.ErrorCodeInvalidAddress
 		}
 		accounts[in.Address] = acc
 	}
@@ -48,7 +49,7 @@ func getOrMakeOutputs(accountGetter state.AccountGetter, accs map[crypto.Address
 	for _, out := range outs {
 		// Account shouldn't be duplicated
 		if _, ok := accs[out.Address]; ok {
-			return nil, payload.ErrTxDuplicateAddress
+			return nil, errors.ErrorCodeDuplicateAddress
 		}
 		acc, err := state.GetMutableAccount(accountGetter, out.Address)
 		if err != nil {
@@ -92,20 +93,16 @@ func validateInputs(accs map[crypto.Address]*acm.MutableAccount, ins []*payload.
 }
 
 func validateInput(acc *acm.MutableAccount, in *payload.TxInput) error {
-	// Check TxInput basic
-	if err := in.ValidateBasic(); err != nil {
-		return err
-	}
 	// Check sequences
 	if acc.Sequence()+1 != uint64(in.Sequence) {
 		return payload.ErrTxInvalidSequence{
-			Got:      in.Sequence,
-			Expected: acc.Sequence() + uint64(1),
+			Input:   in,
+			Account: acc,
 		}
 	}
 	// Check amount
 	if acc.Balance() < uint64(in.Amount) {
-		return payload.ErrTxInsufficientFunds
+		return errors.ErrorCodeInsufficientFunds
 	}
 	return nil
 }
@@ -113,10 +110,6 @@ func validateInput(acc *acm.MutableAccount, in *payload.TxInput) error {
 func validateOutputs(outs []*payload.TxOutput) (uint64, error) {
 	total := uint64(0)
 	for _, out := range outs {
-		// Check TxOutput basic
-		if err := out.ValidateBasic(); err != nil {
-			return 0, err
-		}
 		// Good. Add amount to total
 		total += out.Amount
 	}

txs/payload/errors.go

 package payload
 
 import (
-	"errors"
 	"fmt"
-)
 
-var (
-	ErrTxInvalidAddress    = errors.New("error invalid address")
-	ErrTxDuplicateAddress  = errors.New("error duplicate address")
-	ErrTxInvalidAmount     = errors.New("error invalid amount")
-	ErrTxInsufficientFunds = errors.New("error insufficient funds")
-	ErrTxUnknownPubKey     = errors.New("error unknown pubkey")
-	ErrTxInvalidPubKey     = errors.New("error invalid pubkey")
-	ErrTxInvalidSignature  = errors.New("error invalid signature")
+	"github.com/hyperledger/burrow/acm"
+	"github.com/hyperledger/burrow/execution/errors"
 )
 
 type ErrTxInvalidSequence struct {
-	Got      uint64
-	Expected uint64
+	Input   *TxInput
+	Account acm.Account
 }
 
 func (e ErrTxInvalidSequence) Error() string {
-	return fmt.Sprintf("Error invalid sequence. Got %d, expected %d", e.Got, e.Expected)
+	return fmt.Sprintf("Error invalid sequence in input %v: input has sequence %d, but account has expected "+
+		"%d, so expected input to have sequence %d", e.Input, e.Input.Sequence, e.Account.Sequence(), e.Account.Sequence()+1)
+}
+
+func (e ErrTxInvalidSequence) ErrorCode() errors.Code {
+	return errors.ErrorCodeInvalidSequence
 }

txs/payload/tx_input.go

@@ -2,20 +2,8 @@ package payload
 
 import (
 	"fmt"
-
-	"github.com/hyperledger/burrow/crypto"
 )
 
-func (txIn *TxInput) ValidateBasic() error {
-	if txIn.Address == crypto.ZeroAddress {
-		return ErrTxInvalidAddress
-	}
-	if txIn.Amount == 0 {
-		return ErrTxInvalidAmount
-	}
-	return nil
-}
-
 func (txIn *TxInput) String() string {
 	return fmt.Sprintf("TxInput{%s, Amount: %v, Sequence:%v}", txIn.Address, txIn.Amount, txIn.Sequence)
 }

txs/payload/tx_output.go

@@ -4,16 +4,6 @@ import (
 	"fmt"
 )
 
-func (txOut *TxOutput) ValidateBasic() error {
-	if len(txOut.Address) != 20 {
-		return ErrTxInvalidAddress
-	}
-	if txOut.Amount == 0 {
-		return ErrTxInvalidAmount
-	}
-	return nil
-}
-
 func (txOut *TxOutput) String() string {
 	return fmt.Sprintf("TxOutput{%s, Amount: %v}", txOut.Address, txOut.Amount)
 }