Cleaned up fees and errors a bit from feedback

errors/common.go

@@ -6,6 +6,7 @@ import (
 	"reflect"
 
 	"github.com/pkg/errors"
+
 	abci "github.com/tendermint/abci/types"
 )
 
@@ -40,8 +41,7 @@ func unwrap(i interface{}) interface{} {
 
 func ErrUnknownTxType(tx interface{}) TMError {
 	msg := fmt.Sprintf("%T", unwrap(tx))
-	w := errors.Wrap(errUnknownTxType, msg)
+	return WithMessage(msg, errUnknownTxType, abci.CodeType_UnknownRequest)
-	return WithCode(w, abci.CodeType_UnknownRequest)
 }
 func IsUnknownTxTypeErr(err error) bool {
 	return IsSameError(errUnknownTxType, err)
@@ -49,16 +49,14 @@ func IsUnknownTxTypeErr(err error) bool {
 
 func ErrInvalidFormat(expected string, tx interface{}) TMError {
 	msg := fmt.Sprintf("%T not %s", unwrap(tx), expected)
-	w := errors.Wrap(errInvalidFormat, msg)
+	return WithMessage(msg, errInvalidFormat, abci.CodeType_UnknownRequest)
-	return WithCode(w, abci.CodeType_UnknownRequest)
 }
 func IsInvalidFormatErr(err error) bool {
 	return IsSameError(errInvalidFormat, err)
 }
 
 func ErrUnknownModule(mod string) TMError {
-	w := errors.Wrap(errUnknownModule, mod)
+	return WithMessage(mod, errUnknownModule, abci.CodeType_UnknownRequest)
-	return WithCode(w, abci.CodeType_UnknownRequest)
 }
 func IsUnknownModuleErr(err error) bool {
 	return IsSameError(errUnknownModule, err)

errors/main.go

@@ -104,6 +104,13 @@ func WithCode(err error, code abci.CodeType) TMError {
 	}
 }
 
+// WithMessage prepends some text to the error, then calls WithCode
+// It wraps the original error, so IsSameError will still match on err
+func WithMessage(prefix string, err error, code abci.CodeType) TMError {
+	e2 := errors.WithMessage(err, prefix)
+	return WithCode(e2, code)
+}
+
 // New adds a stacktrace if necessary and sets the code and msg,
 // overriding the state if err was already TMError
 func New(msg string, code abci.CodeType) TMError {

modules/fee/error_test.go

@@ -0,0 +1,19 @@
+package fee
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestErrors(t *testing.T) {
+	assert := assert.New(t)
+
+	e := ErrInsufficientFees()
+	assert.True(IsInsufficientFeesErr(e))
+	assert.False(IsWrongFeeDenomErr(e))
+
+	e2 := ErrWrongFeeDenom("atom")
+	assert.False(IsInsufficientFeesErr(e2))
+	assert.True(IsWrongFeeDenomErr(e2))
+}

modules/fee/errors.go

@@ -5,11 +5,13 @@ import (
 	"fmt"
 
 	abci "github.com/tendermint/abci/types"
+
 	"github.com/tendermint/basecoin/errors"
 )
 
 var (
 	errInsufficientFees = fmt.Errorf("Insufficient Fees")
+	errWrongFeeDenom    = fmt.Errorf("Required fee denomination")
 )
 
 func ErrInsufficientFees() errors.TMError {
@@ -18,3 +20,10 @@ func ErrInsufficientFees() errors.TMError {
 func IsInsufficientFeesErr(err error) bool {
 	return errors.IsSameError(errInsufficientFees, err)
 }
+
+func ErrWrongFeeDenom(denom string) errors.TMError {
+	return errors.WithMessage(denom, errWrongFeeDenom, abci.CodeType_BaseInvalidInput)
+}
+func IsWrongFeeDenomErr(err error) bool {
+	return errors.IsSameError(errWrongFeeDenom, err)
+}

modules/fee/handler.go

@@ -18,7 +18,11 @@ var Bank = basecoin.Actor{App: NameFee, Address: []byte("bank")}
 // SimpleFeeMiddleware - middleware for fee checking, constant amount
 // It used modules.coin to move the money
 type SimpleFeeMiddleware struct {
-	MinFee    coin.Coin //
+	// the fee must be the same denomination and >= this amount
+	// if the amount is 0, then the fee tx wrapper is optional
+	MinFee coin.Coin
+	// all fees go here, which could be a dump (Bank) or something reachable
+	// by other app logic
 	Collector basecoin.Actor
 	stack.PassOption
 }
@@ -60,8 +64,11 @@ func (h SimpleFeeMiddleware) doTx(ctx basecoin.Context, store state.KVStore, tx
 		return res, errors.ErrInvalidFormat(TypeFees, tx)
 	}
 
-	// see if it is big enough...
+	// see if it is the proper denom and big enough
 	fee := feeTx.Fee
+	if fee.Denom != h.MinFee.Denom {
+		return res, ErrWrongFeeDenom(h.MinFee.Denom)
+	}
 	if !fee.IsGTE(h.MinFee) {
 		return res, ErrInsufficientFees()
 	}

modules/fee/handler_test.go

@@ -6,12 +6,13 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/tendermint/tmlibs/log"
+
 	"github.com/tendermint/basecoin"
 	"github.com/tendermint/basecoin/modules/coin"
 	"github.com/tendermint/basecoin/modules/fee"
 	"github.com/tendermint/basecoin/stack"
 	"github.com/tendermint/basecoin/state"
-	"github.com/tendermint/tmlibs/log"
 )
 
 func TestFeeChecks(t *testing.T) {