diff --git a/app/app.go b/app/app.go index e053408a48..836e8c66fa 100644 --- a/app/app.go +++ b/app/app.go @@ -14,6 +14,7 @@ import ( "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/stack" sm "github.com/tendermint/basecoin/state" "github.com/tendermint/basecoin/version" @@ -52,7 +53,7 @@ func NewBasecoin(handler basecoin.Handler, eyesCli *eyes.Client, logger log.Logg } // DefaultHandler - placeholder to just handle sendtx -func DefaultHandler() basecoin.Handler { +func DefaultHandler(feeDenom string) basecoin.Handler { // use the default stack h := coin.NewHandler() d := stack.NewDispatcher(stack.WrapHandler(h)) @@ -61,6 +62,7 @@ func DefaultHandler() basecoin.Handler { stack.Recovery{}, auth.Signatures{}, base.Chain{}, + fee.NewSimpleFeeMiddleware(coin.Coin{feeDenom, 0}, fee.Bank), ).Use(d) } diff --git a/app/app_test.go b/app/app_test.go index 9656190c8d..cbb446bb05 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -13,6 +13,7 @@ import ( "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/stack" "github.com/tendermint/basecoin/state" wire "github.com/tendermint/go-wire" @@ -40,17 +41,33 @@ func newAppTest(t *testing.T) *appTest { return at } -// make a tx sending 5mycoin from each acctIn to acctOut -func (at *appTest) getTx(seq int, coins coin.Coins) basecoin.Tx { - in := []coin.TxInput{{Address: at.acctIn.Actor(), Coins: coins, Sequence: seq}} +// baseTx is the +func (at *appTest) baseTx(coins coin.Coins) basecoin.Tx { + in := []coin.TxInput{{Address: at.acctIn.Actor(), Coins: coins}} out := []coin.TxOutput{{Address: at.acctOut.Actor(), Coins: coins}} tx := coin.NewSendTx(in, out) - tx = base.NewChainTx(at.chainID, 0, tx) + return tx +} + +func (at *appTest) signTx(tx basecoin.Tx) basecoin.Tx { stx := auth.NewMulti(tx) auth.Sign(stx, at.acctIn.Key) return stx.Wrap() } +func (at *appTest) getTx(coins coin.Coins) basecoin.Tx { + tx := at.baseTx(coins) + tx = base.NewChainTx(at.chainID, 0, tx) + return at.signTx(tx) +} + +func (at *appTest) feeTx(coins coin.Coins, toll coin.Coin) basecoin.Tx { + tx := at.baseTx(coins) + tx = fee.NewFee(tx, toll, at.acctIn.Actor()) + tx = base.NewChainTx(at.chainID, 0, tx) + return at.signTx(tx) +} + // set the account on the app through SetOption func (at *appTest) initAccount(acct *coin.AccountWithKey) { res := at.app.SetOption("coin/account", acct.MakeOption()) @@ -67,7 +84,7 @@ func (at *appTest) reset() { logger := log.NewTMLogger(os.Stdout).With("module", "app") logger = log.NewTracingLogger(logger) at.app = NewBasecoin( - DefaultHandler(), + DefaultHandler("mycoin"), eyesCli, logger, ) @@ -124,7 +141,7 @@ func TestSetOption(t *testing.T) { eyesCli := eyes.NewLocalClient("", 0) app := NewBasecoin( - DefaultHandler(), + DefaultHandler("atom"), eyesCli, log.TestingLogger().With("module", "app"), ) @@ -193,32 +210,43 @@ func TestTx(t *testing.T) { //Bad Balance at.acctIn.Coins = coin.Coins{{"mycoin", 2}} at.initAccount(at.acctIn) - res, _, _ := at.exec(t, at.getTx(1, coin.Coins{{"mycoin", 5}}), true) + res, _, _ := at.exec(t, at.getTx(coin.Coins{{"mycoin", 5}}), true) assert.True(res.IsErr(), "ExecTx/Bad CheckTx: Expected error return from ExecTx, returned: %v", res) - res, diffIn, diffOut := at.exec(t, at.getTx(1, coin.Coins{{"mycoin", 5}}), false) + res, diffIn, diffOut := at.exec(t, at.getTx(coin.Coins{{"mycoin", 5}}), false) assert.True(res.IsErr(), "ExecTx/Bad DeliverTx: Expected error return from ExecTx, returned: %v", res) assert.True(diffIn.IsZero()) assert.True(diffOut.IsZero()) //Regular CheckTx at.reset() - res, _, _ = at.exec(t, at.getTx(1, coin.Coins{{"mycoin", 5}}), true) + res, _, _ = at.exec(t, at.getTx(coin.Coins{{"mycoin", 5}}), true) assert.True(res.IsOK(), "ExecTx/Good CheckTx: Expected OK return from ExecTx, Error: %v", res) //Regular DeliverTx at.reset() amt := coin.Coins{{"mycoin", 3}} - res, diffIn, diffOut = at.exec(t, at.getTx(1, amt), false) + res, diffIn, diffOut = at.exec(t, at.getTx(amt), false) assert.True(res.IsOK(), "ExecTx/Good DeliverTx: Expected OK return from ExecTx, Error: %v", res) assert.Equal(amt.Negative(), diffIn) assert.Equal(amt, diffOut) + + //DeliverTx with fee.... 4 get to recipient, 1 extra taxed + at.reset() + amt = coin.Coins{{"mycoin", 4}} + toll := coin.Coin{"mycoin", 1} + res, diffIn, diffOut = at.exec(t, at.feeTx(amt, toll), false) + assert.True(res.IsOK(), "ExecTx/Good DeliverTx: Expected OK return from ExecTx, Error: %v", res) + payment := amt.Plus(coin.Coins{toll}).Negative() + assert.Equal(payment, diffIn) + assert.Equal(amt, diffOut) + } func TestQuery(t *testing.T) { assert := assert.New(t) at := newAppTest(t) - res, _, _ := at.exec(t, at.getTx(1, coin.Coins{{"mycoin", 5}}), false) + res, _, _ := at.exec(t, at.getTx(coin.Coins{{"mycoin", 5}}), false) assert.True(res.IsOK(), "Commit, DeliverTx: Expected OK return from DeliverTx, Error: %v", res) resQueryPreCommit := at.app.Query(abci.RequestQuery{ diff --git a/app/genesis_test.go b/app/genesis_test.go index 1d99b38eb4..95e1d788e3 100644 --- a/app/genesis_test.go +++ b/app/genesis_test.go @@ -18,7 +18,7 @@ const genesisAcctFilepath = "./testdata/genesis2.json" func TestLoadGenesisDoNotFailIfAppOptionsAreMissing(t *testing.T) { eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(DefaultHandler(), eyesCli, log.TestingLogger()) + app := NewBasecoin(DefaultHandler("mycoin"), eyesCli, log.TestingLogger()) err := app.LoadGenesis("./testdata/genesis3.json") require.Nil(t, err, "%+v", err) } @@ -27,7 +27,7 @@ func TestLoadGenesis(t *testing.T) { assert, require := assert.New(t), require.New(t) eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(DefaultHandler(), eyesCli, log.TestingLogger()) + app := NewBasecoin(DefaultHandler("mycoin"), eyesCli, log.TestingLogger()) err := app.LoadGenesis(genesisFilepath) require.Nil(err, "%+v", err) @@ -56,7 +56,7 @@ func TestLoadGenesisAccountAddress(t *testing.T) { assert, require := assert.New(t), require.New(t) eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(DefaultHandler(), eyesCli, log.TestingLogger()) + app := NewBasecoin(DefaultHandler("mycoin"), eyesCli, log.TestingLogger()) err := app.LoadGenesis(genesisAcctFilepath) require.Nil(err, "%+v", err) diff --git a/cmd/basecli/commands/cmds.go b/cmd/basecli/commands/cmds.go index 4c5a90640e..631c371d6a 100644 --- a/cmd/basecli/commands/cmds.go +++ b/cmd/basecli/commands/cmds.go @@ -16,6 +16,7 @@ import ( "github.com/tendermint/basecoin/modules/auth" "github.com/tendermint/basecoin/modules/base" "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/modules/fee" ) //------------------------- @@ -64,7 +65,10 @@ func doSendTx(cmd *cobra.Command, args []string) error { } // TODO: make this more flexible for middleware - // add the chain info + tx, err = WrapFeeTx(tx) + if err != nil { + return err + } tx, err = WrapChainTx(tx) if err != nil { return err @@ -83,6 +87,21 @@ func doSendTx(cmd *cobra.Command, args []string) error { return txcmd.OutputTx(bres) } +// WrapFeeTx checks for FlagFee and if present wraps the tx with a +// FeeTx of the given amount, paid by the signer +func WrapFeeTx(tx basecoin.Tx) (res basecoin.Tx, err error) { + //parse the fee and amounts into coin types + toll, err := coin.ParseCoin(viper.GetString(FlagFee)) + if err != nil { + return res, err + } + // if no fee, do nothing, otherwise wrap it + if toll.IsZero() { + return tx, nil + } + return fee.NewFee(tx, toll, getSignerAddr()), nil +} + // WrapChainTx will wrap the tx with a ChainTx from the standard flags func WrapChainTx(tx basecoin.Tx) (res basecoin.Tx, err error) { expires := viper.GetInt64(FlagExpires) @@ -94,6 +113,15 @@ func WrapChainTx(tx basecoin.Tx) (res basecoin.Tx, err error) { return res, nil } +func getSignerAddr() (res basecoin.Actor) { + // this could be much cooler with multisig... + signer := txcmd.GetSigner() + if !signer.Empty() { + res = auth.SigPerm(signer.Address()) + } + return res +} + func readSendTxFlags() (tx basecoin.Tx, err error) { // parse to address chain, to, err := parseChainAddress(viper.GetString(FlagTo)) @@ -103,31 +131,15 @@ func readSendTxFlags() (tx basecoin.Tx, err error) { toAddr := auth.SigPerm(to) toAddr.ChainID = chain - // //parse the fee and amounts into coin types - // tx.Fee, err = btypes.ParseCoin(viper.GetString(FlagFee)) - // if err != nil { - // return err - // } - // // set the gas - // tx.Gas = viper.GetInt64(FlagGas) - amountCoins, err := coin.ParseCoins(viper.GetString(FlagAmount)) if err != nil { return tx, err } - // this could be much cooler with multisig... - var fromAddr basecoin.Actor - signer := txcmd.GetSigner() - if !signer.Empty() { - fromAddr = auth.SigPerm(signer.Address()) - } - // craft the inputs and outputs ins := []coin.TxInput{{ - Address: fromAddr, - Coins: amountCoins, - Sequence: viper.GetInt(FlagSequence), + Address: getSignerAddr(), + Coins: amountCoins, }} outs := []coin.TxOutput{{ Address: toAddr, diff --git a/cmd/basecoin/main.go b/cmd/basecoin/main.go index 2185262b73..8b4f5764cd 100644 --- a/cmd/basecoin/main.go +++ b/cmd/basecoin/main.go @@ -11,7 +11,8 @@ import ( func main() { rt := commands.RootCmd - commands.Handler = app.DefaultHandler() + // require all fees in mycoin - change this in your app! + commands.Handler = app.DefaultHandler("mycoin") rt.AddCommand( commands.InitCmd, diff --git a/docs/guide/counter/cmd/counter/main.go b/docs/guide/counter/cmd/counter/main.go index b7ba4c48ff..37816b0421 100644 --- a/docs/guide/counter/cmd/counter/main.go +++ b/docs/guide/counter/cmd/counter/main.go @@ -18,7 +18,7 @@ func main() { } // TODO: register the counter here - commands.Handler = counter.NewHandler() + commands.Handler = counter.NewHandler("mycoin") RootCmd.AddCommand( commands.InitCmd, diff --git a/docs/guide/counter/cmd/countercli/commands/counter.go b/docs/guide/counter/cmd/countercli/commands/counter.go index 326b62b19c..9e420b008c 100644 --- a/docs/guide/counter/cmd/countercli/commands/counter.go +++ b/docs/guide/counter/cmd/countercli/commands/counter.go @@ -28,14 +28,15 @@ You must pass --valid for it to count and the countfee will be added to the coun const ( FlagCountFee = "countfee" FlagValid = "valid" - FlagSequence = "sequence" // FIXME: currently not supported... ) func init() { fs := CounterTxCmd.Flags() fs.String(FlagCountFee, "", "Coins to send in the format ,...") fs.Bool(FlagValid, false, "Is count valid?") - fs.Int(FlagSequence, -1, "Sequence number for this transaction") + + fs.String(bcmd.FlagFee, "0mycoin", "Coins for the transaction fee of the format ") + fs.Int(bcmd.FlagSequence, -1, "Sequence number for this transaction") } // TODO: counterTx is very similar to the sendtx one, @@ -55,11 +56,15 @@ func counterTx(cmd *cobra.Command, args []string) error { } // TODO: make this more flexible for middleware - // add the chain info + tx, err = bcmd.WrapFeeTx(tx) + if err != nil { + return err + } tx, err = bcmd.WrapChainTx(tx) if err != nil { return err } + stx := auth.NewSig(tx) // Sign if needed and post. This it the work-horse @@ -78,6 +83,6 @@ func readCounterTxFlags() (tx basecoin.Tx, err error) { return tx, err } - tx = counter.NewTx(viper.GetBool(FlagValid), feeCoins, viper.GetInt(FlagSequence)) + tx = counter.NewTx(viper.GetBool(FlagValid), feeCoins, viper.GetInt(bcmd.FlagSequence)) return tx, nil } diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 168886160b..b1a7b86088 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -11,6 +11,7 @@ import ( "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/stack" "github.com/tendermint/basecoin/state" ) @@ -23,7 +24,7 @@ import ( // so it gets routed properly const ( NameCounter = "cntr" - ByteTx = 0x21 //TODO What does this byte represent should use typebytes probably + ByteTx = 0x2F //TODO What does this byte represent should use typebytes probably TypeTx = NameCounter + "/count" ) @@ -89,12 +90,12 @@ func ErrDecoding() error { //-------------------------------------------------------------------------------- // NewHandler returns a new counter transaction processing handler -func NewHandler() basecoin.Handler { +func NewHandler(feeDenom string) basecoin.Handler { // use the default stack - coin := coin.NewHandler() + ch := coin.NewHandler() counter := Handler{} dispatcher := stack.NewDispatcher( - stack.WrapHandler(coin), + stack.WrapHandler(ch), counter, ) return stack.New( @@ -102,6 +103,7 @@ func NewHandler() basecoin.Handler { stack.Recovery{}, auth.Signatures{}, base.Chain{}, + fee.NewSimpleFeeMiddleware(coin.Coin{feeDenom, 0}, fee.Bank), ).Use(dispatcher) } @@ -145,7 +147,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi if len(senders) == 0 { return res, errors.ErrMissingSignature() } - in := []coin.TxInput{{Address: senders[0], Coins: ctr.Fee, Sequence: ctr.Sequence}} + in := []coin.TxInput{{Address: senders[0], Coins: ctr.Fee}} out := []coin.TxOutput{{Address: StoreActor(), Coins: ctr.Fee}} send := coin.NewSendTx(in, out) // if the deduction fails (too high), abort the command @@ -170,7 +172,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi func checkTx(ctx basecoin.Context, tx basecoin.Tx) (ctr Tx, err error) { ctr, ok := tx.Unwrap().(Tx) if !ok { - return ctr, errors.ErrInvalidFormat(tx) + return ctr, errors.ErrInvalidFormat(TypeTx, tx) } err = ctr.ValidateBasic() if err != nil { diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index 2de91d62a7..60c5101fd1 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -27,7 +27,7 @@ func TestCounterPlugin(t *testing.T) { logger := log.NewTMLogger(os.Stdout).With("module", "app") // logger = log.NewTracingLogger(logger) bcApp := app.NewBasecoin( - NewHandler(), + NewHandler("gold"), eyesCli, logger, ) diff --git a/errors/common.go b/errors/common.go index c793a6953e..81d786beee 100644 --- a/errors/common.go +++ b/errors/common.go @@ -6,6 +6,7 @@ import ( "reflect" "github.com/pkg/errors" + abci "github.com/tendermint/abci/types" ) @@ -40,25 +41,22 @@ func unwrap(i interface{}) interface{} { func ErrUnknownTxType(tx interface{}) TMError { msg := fmt.Sprintf("%T", unwrap(tx)) - w := errors.Wrap(errUnknownTxType, msg) - return WithCode(w, abci.CodeType_UnknownRequest) + return WithMessage(msg, errUnknownTxType, abci.CodeType_UnknownRequest) } func IsUnknownTxTypeErr(err error) bool { return IsSameError(errUnknownTxType, err) } -func ErrInvalidFormat(tx interface{}) TMError { - msg := fmt.Sprintf("%T", unwrap(tx)) - w := errors.Wrap(errInvalidFormat, msg) - return WithCode(w, abci.CodeType_UnknownRequest) +func ErrInvalidFormat(expected string, tx interface{}) TMError { + msg := fmt.Sprintf("%T not %s", unwrap(tx), expected) + return WithMessage(msg, errInvalidFormat, abci.CodeType_UnknownRequest) } func IsInvalidFormatErr(err error) bool { return IsSameError(errInvalidFormat, err) } func ErrUnknownModule(mod string) TMError { - w := errors.Wrap(errUnknownModule, mod) - return WithCode(w, abci.CodeType_UnknownRequest) + return WithMessage(mod, errUnknownModule, abci.CodeType_UnknownRequest) } func IsUnknownModuleErr(err error) bool { return IsSameError(errUnknownModule, err) diff --git a/errors/main.go b/errors/main.go index 21f261148a..a75a523c03 100644 --- a/errors/main.go +++ b/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 { diff --git a/modules/coin/bench_test.go b/modules/coin/bench_test.go index 83d3da6b78..34bf39bd1e 100644 --- a/modules/coin/bench_test.go +++ b/modules/coin/bench_test.go @@ -15,8 +15,8 @@ func makeHandler() basecoin.Handler { return NewHandler() } -func makeSimpleTx(from, to basecoin.Actor, amount Coins, seq int) basecoin.Tx { - in := []TxInput{{Address: from, Coins: amount, Sequence: seq}} +func makeSimpleTx(from, to basecoin.Actor, amount Coins) basecoin.Tx { + in := []TxInput{{Address: from, Coins: amount}} out := []TxOutput{{Address: to, Coins: amount}} return NewSendTx(in, out) } @@ -35,7 +35,7 @@ func BenchmarkSimpleTransfer(b *testing.B) { // now, loop... for i := 1; i <= b.N; i++ { ctx := stack.MockContext("foo", 100).WithPermissions(sender) - tx := makeSimpleTx(sender, receiver, Coins{{"mycoin", 2}}, i) + tx := makeSimpleTx(sender, receiver, Coins{{"mycoin", 2}}) _, err := h.DeliverTx(ctx, store, tx) // never should error if err != nil { diff --git a/modules/coin/coin.go b/modules/coin/coin.go index 6696288abf..168b069259 100644 --- a/modules/coin/coin.go +++ b/modules/coin/coin.go @@ -16,10 +16,23 @@ type Coin struct { Amount int64 `json:"amount"` } +// String provides a human-readable representation of a coin func (coin Coin) String() string { return fmt.Sprintf("%v%v", coin.Amount, coin.Denom) } +// IsZero returns if this represents no money +func (coin Coin) IsZero() bool { + return coin.Amount == 0 +} + +// IsGTE returns true if they are the same type and the receiver is +// an equal or greater value +func (coin Coin) IsGTE(other Coin) bool { + return (coin.Denom == other.Denom) && + (coin.Amount >= other.Amount) +} + //regex codes for extracting coins from string var reDenom = regexp.MustCompile("") var reAmt = regexp.MustCompile("(\\d+)") diff --git a/modules/coin/genesis.go b/modules/coin/genesis.go index 06bcd9c1c6..b174c8c11a 100644 --- a/modules/coin/genesis.go +++ b/modules/coin/genesis.go @@ -15,16 +15,14 @@ import ( type GenesisAccount struct { Address data.Bytes `json:"address"` // this from types.Account (don't know how to embed this properly) - PubKey crypto.PubKey `json:"pub_key"` // May be nil, if not known. - Sequence int `json:"sequence"` - Balance Coins `json:"coins"` + PubKey crypto.PubKey `json:"pub_key"` // May be nil, if not known. + Balance Coins `json:"coins"` } // ToAccount - GenesisAccount struct to a basecoin Account func (g GenesisAccount) ToAccount() Account { return Account{ - Sequence: g.Sequence, - Coins: g.Balance, + Coins: g.Balance, } } diff --git a/modules/coin/handler.go b/modules/coin/handler.go index bd047f8eb4..0e3378db70 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -37,7 +37,7 @@ func (h Handler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin. // now make sure there is money for _, in := range send.Inputs { - _, err = CheckCoins(store, in.Address, in.Coins.Negative(), in.Sequence) + _, err = CheckCoins(store, in.Address, in.Coins.Negative()) if err != nil { return res, err } @@ -56,7 +56,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi // deduct from all input accounts for _, in := range send.Inputs { - _, err = ChangeCoins(store, in.Address, in.Coins.Negative(), in.Sequence) + _, err = ChangeCoins(store, in.Address, in.Coins.Negative()) if err != nil { return res, err } @@ -64,8 +64,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi // add to all output accounts for _, out := range send.Outputs { - // note: sequence number is ignored when adding coins, only checked for subtracting - _, err = ChangeCoins(store, out.Address, out.Coins, 0) + _, err = ChangeCoins(store, out.Address, out.Coins) if err != nil { return res, err } @@ -107,7 +106,7 @@ func checkTx(ctx basecoin.Context, tx basecoin.Tx) (send SendTx, err error) { // check if the tx is proper type and valid send, ok := tx.Unwrap().(SendTx) if !ok { - return send, errors.ErrInvalidFormat(tx) + return send, errors.ErrInvalidFormat(TypeSend, tx) } err = send.ValidateBasic() if err != nil { diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index 716ad7ed7f..11959eccfc 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -35,40 +35,40 @@ func TestHandlerValidation(t *testing.T) { // auth works with different apps {true, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{addr1}}, {true, NewSendTx( - []TxInput{NewTxInput(addr2, someCoins, 2)}, + []TxInput{NewTxInput(addr2, someCoins)}, []TxOutput{NewTxOutput(addr1, someCoins)}), []basecoin.Actor{addr1, addr2}}, // check multi-input with both sigs {true, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, someCoins, 3)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, someCoins)}, []TxOutput{NewTxOutput(addr1, doubleCoins)}), []basecoin.Actor{addr1, addr2}}, // wrong permissions fail {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{}}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{addr2}}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, someCoins, 3)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, someCoins)}, []TxOutput{NewTxOutput(addr1, doubleCoins)}), []basecoin.Actor{addr1}}, // invalid input fails {false, NewSendTx( - []TxInput{NewTxInput(addr1, minusCoins, 2)}, + []TxInput{NewTxInput(addr1, minusCoins)}, []TxOutput{NewTxOutput(addr2, minusCoins)}), []basecoin.Actor{addr2}}, } @@ -113,7 +113,7 @@ func TestDeliverTx(t *testing.T) { { []money{{addr1, moreCoins}}, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 1)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}), []basecoin.Actor{addr1}, []money{{addr1, diffCoins}, {addr2, someCoins}}, @@ -122,7 +122,7 @@ func TestDeliverTx(t *testing.T) { { []money{{addr1, mixedCoins}, {addr2, moreCoins}}, NewSendTx( - []TxInput{NewTxInput(addr1, otherCoins, 1), NewTxInput(addr2, someCoins, 1)}, + []TxInput{NewTxInput(addr1, otherCoins), NewTxInput(addr2, someCoins)}, []TxOutput{NewTxOutput(addr3, mixedCoins)}), []basecoin.Actor{addr1, addr2}, []money{{addr1, someCoins}, {addr2, diffCoins}, {addr3, mixedCoins}}, @@ -131,7 +131,7 @@ func TestDeliverTx(t *testing.T) { { []money{{addr1, moreCoins.Plus(otherCoins)}}, NewSendTx( - []TxInput{NewTxInput(addr1, otherCoins, 1), NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, otherCoins), NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, mixedCoins)}), []basecoin.Actor{addr1}, []money{{addr1, diffCoins}, {addr2, mixedCoins}}, diff --git a/modules/coin/helper.go b/modules/coin/helper.go index 4c66ec6a82..21c9ea7825 100644 --- a/modules/coin/helper.go +++ b/modules/coin/helper.go @@ -39,9 +39,8 @@ func (a *AccountWithKey) Actor() basecoin.Actor { // This is intended for use in test cases func (a *AccountWithKey) MakeOption() string { info := GenesisAccount{ - Address: a.Address(), - Sequence: a.Sequence, - Balance: a.Coins, + Address: a.Address(), + Balance: a.Coins, } js, err := data.ToJSON(info) if err != nil { diff --git a/modules/coin/store.go b/modules/coin/store.go index 94c086f1de..516e7acea9 100644 --- a/modules/coin/store.go +++ b/modules/coin/store.go @@ -22,14 +22,14 @@ func GetAccount(store state.KVStore, addr basecoin.Actor) (Account, error) { } // CheckCoins makes sure there are funds, but doesn't change anything -func CheckCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (Coins, error) { - acct, err := updateCoins(store, addr, coins, seq) +func CheckCoins(store state.KVStore, addr basecoin.Actor, coins Coins) (Coins, error) { + acct, err := updateCoins(store, addr, coins) return acct.Coins, err } // ChangeCoins changes the money, returns error if it would be negative -func ChangeCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (Coins, error) { - acct, err := updateCoins(store, addr, coins, seq) +func ChangeCoins(store state.KVStore, addr basecoin.Actor, coins Coins) (Coins, error) { + acct, err := updateCoins(store, addr, coins) if err != nil { return acct.Coins, err } @@ -41,7 +41,7 @@ func ChangeCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) // updateCoins will load the account, make all checks, and return the updated account. // // it doesn't save anything, that is up to you to decide (Check/Change Coins) -func updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) (acct Account, err error) { +func updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins) (acct Account, err error) { acct, err = loadAccount(store, addr.Bytes()) // we can increase an empty account... if IsNoAccountErr(err) && coins.IsPositive() { @@ -51,14 +51,6 @@ func updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) return acct, err } - // check sequence if we are deducting... ugh, need a cleaner replay protection - if !coins.IsPositive() { - if seq != acct.Sequence+1 { - return acct, ErrInvalidSequence() - } - acct.Sequence++ - } - // check amount final := acct.Coins.Plus(coins) if !final.IsNonnegative() { @@ -71,8 +63,7 @@ func updateCoins(store state.KVStore, addr basecoin.Actor, coins Coins, seq int) // Account - coin account structure type Account struct { - Coins Coins `json:"coins"` - Sequence int `json:"sequence"` + Coins Coins `json:"coins"` } func loadAccount(store state.KVStore, key []byte) (acct Account, err error) { diff --git a/modules/coin/tx.go b/modules/coin/tx.go index 147f0ec848..b3598970ed 100644 --- a/modules/coin/tx.go +++ b/modules/coin/tx.go @@ -20,9 +20,8 @@ const ( // TxInput - expected coin movement outputs, used with SendTx type TxInput struct { - Address basecoin.Actor `json:"address"` - Coins Coins `json:"coins"` - Sequence int `json:"sequence"` // Nonce: Must be 1 greater than the last committed TxInput + Address basecoin.Actor `json:"address"` + Coins Coins `json:"coins"` } // ValidateBasic - validate transaction input @@ -40,22 +39,18 @@ func (txIn TxInput) ValidateBasic() error { if !txIn.Coins.IsPositive() { return ErrInvalidCoins() } - if txIn.Sequence <= 0 { - return ErrInvalidSequence() - } return nil } func (txIn TxInput) String() string { - return fmt.Sprintf("TxInput{%v,%v,%v}", txIn.Address, txIn.Coins, txIn.Sequence) + return fmt.Sprintf("TxInput{%v,%v}", txIn.Address, txIn.Coins) } // NewTxInput - create a transaction input, used with SendTx -func NewTxInput(addr basecoin.Actor, coins Coins, sequence int) TxInput { +func NewTxInput(addr basecoin.Actor, coins Coins) TxInput { input := TxInput{ - Address: addr, - Coins: coins, - Sequence: sequence, + Address: addr, + Coins: coins, } return input } @@ -110,11 +105,19 @@ type SendTx struct { var _ basecoin.Tx = NewSendTx(nil, nil) -// NewSendTx - new SendTx +// NewSendTx - construct arbitrary multi-in, multi-out sendtx func NewSendTx(in []TxInput, out []TxOutput) basecoin.Tx { return SendTx{Inputs: in, Outputs: out}.Wrap() } +// NewSendOneTx is a helper for the standard (?) case where there is exactly +// one sender and one recipient +func NewSendOneTx(sender, recipient basecoin.Actor, amount Coins) basecoin.Tx { + in := []TxInput{{Address: sender, Coins: amount}} + out := []TxOutput{{Address: recipient, Coins: amount}} + return SendTx{Inputs: in, Outputs: out}.Wrap() +} + // ValidateBasic - validate the send transaction func (tx SendTx) ValidateBasic() error { // this just makes sure all the inputs and outputs are properly formatted, diff --git a/modules/coin/tx_test.go b/modules/coin/tx_test.go index 9aa0699ed9..1b292ae289 100644 --- a/modules/coin/tx_test.go +++ b/modules/coin/tx_test.go @@ -46,26 +46,14 @@ var coins = []struct { func TestTxValidateInput(t *testing.T) { assert := assert.New(t) - seqs := []struct { - seq int - valid bool - }{ - {-3, false}, - {0, false}, - {1, true}, - {6571265735, true}, - } - for i, act := range actors { for j, coin := range coins { - for k, seq := range seqs { - input := NewTxInput(act.actor, coin.coins, seq.seq) - err := input.ValidateBasic() - if act.valid && coin.valid && seq.valid { - assert.Nil(err, "%d,%d,%d: %+v", i, j, k, err) - } else { - assert.NotNil(err, "%d,%d,%d", i, j, k) - } + input := NewTxInput(act.actor, coin.coins) + err := input.ValidateBasic() + if act.valid && coin.valid { + assert.Nil(err, "%d,%d: %+v", i, j, err) + } else { + assert.NotNil(err, "%d,%d", i, j) } } } @@ -111,15 +99,15 @@ func TestTxValidateTx(t *testing.T) { }{ // 0-2. valid cases {true, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}, )}, {true, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, otherCoins, 5)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, otherCoins)}, []TxOutput{NewTxOutput(addr3, bothCoins)}, )}, {true, NewSendTx( - []TxInput{NewTxInput(addr1, bothCoins, 42)}, + []TxInput{NewTxInput(addr1, bothCoins)}, []TxOutput{NewTxOutput(addr2, someCoins), NewTxOutput(addr3, otherCoins)}, )}, @@ -129,39 +117,35 @@ func TestTxValidateTx(t *testing.T) { []TxOutput{NewTxOutput(addr2, someCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, nil, )}, - // 5-8. invalid inputs + // 5-7. invalid inputs {false, NewSendTx( - []TxInput{NewTxInput(noAddr, someCoins, 2)}, + []TxInput{NewTxInput(noAddr, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, -1)}, - []TxOutput{NewTxOutput(addr2, someCoins)}, - )}, - {false, NewSendTx( - []TxInput{NewTxInput(addr1, noCoins, 2)}, + []TxInput{NewTxInput(addr1, noCoins)}, []TxOutput{NewTxOutput(addr2, noCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, minusCoins, 2)}, + []TxInput{NewTxInput(addr1, minusCoins)}, []TxOutput{NewTxOutput(addr2, minusCoins)}, )}, - // 9-11. totals don't match + // 8-10. totals don't match {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 7)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, moreCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, minusCoins, 5)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, minusCoins)}, []TxOutput{NewTxOutput(addr3, someCoins)}, )}, {false, NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2), NewTxInput(addr2, moreCoins, 5)}, + []TxInput{NewTxInput(addr1, someCoins), NewTxInput(addr2, moreCoins)}, []TxOutput{NewTxOutput(addr3, bothCoins)}, )}, } @@ -185,7 +169,7 @@ func TestTxSerializeTx(t *testing.T) { someCoins := Coins{{"atom", 123}} send := NewSendTx( - []TxInput{NewTxInput(addr1, someCoins, 2)}, + []TxInput{NewTxInput(addr1, someCoins)}, []TxOutput{NewTxOutput(addr2, someCoins)}, ) diff --git a/modules/fee/error_test.go b/modules/fee/error_test.go new file mode 100644 index 0000000000..4d255b8804 --- /dev/null +++ b/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)) +} diff --git a/modules/fee/errors.go b/modules/fee/errors.go index f29d3cbe16..06cdd10c5c 100644 --- a/modules/fee/errors.go +++ b/modules/fee/errors.go @@ -2,14 +2,16 @@ package fee import ( - rawerr "errors" + "fmt" abci "github.com/tendermint/abci/types" + "github.com/tendermint/basecoin/errors" ) var ( - errInsufficientFees = rawerr.New("Insufficient Fees") + 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) +} diff --git a/modules/fee/handler.go b/modules/fee/handler.go index d202ebab41..4e12a15a65 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -11,76 +11,74 @@ import ( // NameFee - namespace for the fee module const NameFee = "fee" -// AccountChecker - interface used by SimpleFeeHandler -type AccountChecker interface { - // Get amount checks the current amount - GetAmount(store state.KVStore, addr basecoin.Actor) (coin.Coins, error) +// Bank is a default location for the fees, but pass anything into +// the middleware constructor +var Bank = basecoin.Actor{App: NameFee, Address: []byte("bank")} - // ChangeAmount modifies the balance by the given amount and returns the new balance - // always returns an error if leading to negative balance - ChangeAmount(store state.KVStore, addr basecoin.Actor, coins coin.Coins) (coin.Coins, error) -} - -// SimpleFeeHandler - checker object for fee checking -type SimpleFeeHandler struct { - AccountChecker - MinFee coin.Coins +// SimpleFeeMiddleware - middleware for fee checking, constant amount +// It used modules.coin to move the money +type SimpleFeeMiddleware struct { + // 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 } +var _ stack.Middleware = SimpleFeeMiddleware{} + +// NewSimpleFeeMiddleware returns a fee handler with a fixed minimum fee. +// +// If minFee is 0, then the FeeTx is optional +func NewSimpleFeeMiddleware(minFee coin.Coin, collector basecoin.Actor) SimpleFeeMiddleware { + return SimpleFeeMiddleware{ + MinFee: minFee, + Collector: collector, + } +} + // Name - return the namespace for the fee module -func (SimpleFeeHandler) Name() string { +func (SimpleFeeMiddleware) Name() string { return NameFee } -var _ stack.Middleware = SimpleFeeHandler{} - -// Yes, I know refactor a bit... really too late already - // CheckTx - check the transaction -func (h SimpleFeeHandler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { - feeTx, ok := tx.Unwrap().(*Fee) - if !ok { - return res, errors.ErrInvalidFormat(tx) - } - - fees := coin.Coins{feeTx.Fee} - if !fees.IsGTE(h.MinFee) { - return res, ErrInsufficientFees() - } - - if !ctx.HasPermission(feeTx.Payer) { - return res, errors.ErrUnauthorized() - } - - _, err = h.ChangeAmount(store, feeTx.Payer, fees.Negative()) - if err != nil { - return res, err - } - - return basecoin.Result{Log: "Valid tx"}, nil +func (h SimpleFeeMiddleware) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { + return h.doTx(ctx, store, tx, next.CheckTx) } // DeliverTx - send the fee handler transaction -func (h SimpleFeeHandler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { - feeTx, ok := tx.Unwrap().(*Fee) +func (h SimpleFeeMiddleware) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { + return h.doTx(ctx, store, tx, next.DeliverTx) +} + +func (h SimpleFeeMiddleware) doTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.CheckerFunc) (res basecoin.Result, err error) { + feeTx, ok := tx.Unwrap().(Fee) if !ok { - return res, errors.ErrInvalidFormat(tx) + // the fee wrapper is not required if there is no minimum + if h.MinFee.IsZero() { + return next(ctx, store, tx) + } + return res, errors.ErrInvalidFormat(TypeFees, tx) } - fees := coin.Coins{feeTx.Fee} - if !fees.IsGTE(h.MinFee) { + // 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() } - if !ctx.HasPermission(feeTx.Payer) { - return res, errors.ErrUnauthorized() - } - - _, err = h.ChangeAmount(store, feeTx.Payer, fees.Negative()) + // now, try to make a IPC call to coins... + send := coin.NewSendOneTx(feeTx.Payer, h.Collector, coin.Coins{fee}) + _, err = next(ctx, store, send) if err != nil { return res, err } - return next.DeliverTx(ctx, store, feeTx.Next()) + return next(ctx, store, feeTx.Tx) } diff --git a/modules/fee/handler_test.go b/modules/fee/handler_test.go new file mode 100644 index 0000000000..b775c2d565 --- /dev/null +++ b/modules/fee/handler_test.go @@ -0,0 +1,135 @@ +package fee_test + +import ( + "testing" + + "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" +) + +func TestFeeChecks(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + atom := func(i int64) coin.Coin { return coin.Coin{"atom", i} } + eth := func(i int64) coin.Coin { return coin.Coin{"eth", i} } + atoms := func(i int64) coin.Coins { return coin.Coins{{"atom", i}} } + wallet := func(i, j int64) coin.Coins { return coin.Coins{atom(i), eth(j)} } + + // some coin amounts... + zero := coin.Coin{} + mixed := wallet(1200, 55) + pure := atoms(46657) + + // these are some accounts + collector := basecoin.Actor{App: fee.NameFee, Address: []byte("mine")} + key1 := coin.NewAccountWithKey(mixed) + key2 := coin.NewAccountWithKey(pure) + act1, act2 := key1.Actor(), key2.Actor() + + // set up the apps.... + disp := stack.NewDispatcher( + // OKHandler will just return success to a RawTx + stack.WrapHandler(stack.OKHandler{}), + // coin is needed to handle the IPC call from Fee middleware + stack.WrapHandler(coin.NewHandler()), + ) + // app1 requires no fees + app1 := stack.New(fee.NewSimpleFeeMiddleware(atom(0), collector)).Use(disp) + // app2 requires 2 atom + app2 := stack.New(fee.NewSimpleFeeMiddleware(atom(2), collector)).Use(disp) + + // set up the store and init the accounts + store := state.NewMemKVStore() + l := log.NewNopLogger() + _, err := app1.SetOption(l, store, "coin", "account", key1.MakeOption()) + require.Nil(err, "%+v", err) + _, err = app2.SetOption(l, store, "coin", "account", key2.MakeOption()) + require.Nil(err, "%+v", err) + + cases := []struct { + valid bool + // this is the middleware stack to test + app basecoin.Handler + // they sign the tx + signer basecoin.Actor + // wrap with the given fee if hasFee is true + hasFee bool + payer basecoin.Actor + fee coin.Coin + // expected balance after the tx + left coin.Coins + collected coin.Coins + }{ + // make sure it works with no fee (control group) + {true, app1, act1, false, act1, zero, mixed, nil}, + {true, app1, act2, false, act2, zero, pure, nil}, + + // no fee or too low is rejected + {false, app2, act1, false, act1, zero, mixed, nil}, + {false, app2, act2, false, act2, zero, pure, nil}, + {false, app2, act1, true, act1, zero, mixed, nil}, + {false, app2, act2, true, act2, atom(1), pure, nil}, + + // proper fees are transfered in both cases + {true, app1, act1, true, act1, atom(1), wallet(1199, 55), atoms(1)}, + {true, app2, act2, true, act2, atom(57), atoms(46600), atoms(58)}, + + // // fee must be the proper type... + {false, app1, act1, true, act1, eth(5), wallet(1199, 55), atoms(58)}, + + // signature must match + {false, app2, act1, true, act2, atom(5), atoms(46600), atoms(58)}, + + // send only works within limits + {true, app2, act1, true, act1, atom(1100), wallet(99, 55), atoms(1158)}, + {false, app2, act1, true, act1, atom(500), wallet(99, 55), atoms(1158)}, + } + + for i, tc := range cases { + // build the tx + tx := stack.NewRawTx([]byte{7, 8, 9}) + if tc.hasFee { + tx = fee.NewFee(tx, tc.fee, tc.payer) + } + + // set up the permissions + ctx := stack.MockContext("x", 1).WithPermissions(tc.signer) + + // call checktx... + _, err := tc.app.CheckTx(ctx, store, tx) + if tc.valid { + assert.Nil(err, "%d: %+v", i, err) + } else { + assert.NotNil(err, "%d", i) + } + + // call delivertx... + _, err = tc.app.DeliverTx(ctx, store, tx) + if tc.valid { + assert.Nil(err, "%d: %+v", i, err) + } else { + assert.NotNil(err, "%d", i) + } + + // check the account balance afterwards.... + cspace := stack.PrefixedStore(coin.NameCoin, store) + acct, err := coin.GetAccount(cspace, tc.payer) + require.Nil(err, "%d: %+v", i, err) + assert.Equal(tc.left, acct.Coins, "%d", i) + + // check the collected balance afterwards.... + acct, err = coin.GetAccount(cspace, collector) + require.Nil(err, "%d: %+v", i, err) + assert.Equal(tc.collected, acct.Coins, "%d", i) + } + +} diff --git a/modules/fee/tx.go b/modules/fee/tx.go index c17de796c4..d2f61f7327 100644 --- a/modules/fee/tx.go +++ b/modules/fee/tx.go @@ -7,38 +7,38 @@ import ( // nolint const ( - ByteFees = 0x20 - TypeFees = "fee" + ByteFees = 0x21 + TypeFees = NameFee + "/tx" ) func init() { basecoin.TxMapper. - RegisterImplementation(&Fee{}, TypeFees, ByteFees) + RegisterImplementation(Fee{}, TypeFees, ByteFees) } /**** Fee ****/ -var _ basecoin.TxLayer = &Fee{} - // Fee attaches a fee payment to the embedded tx type Fee struct { - Tx basecoin.Tx `json:"tx"` + // Gas coin.Coin `json:"gas"` // ????? Fee coin.Coin `json:"fee"` Payer basecoin.Actor `json:"payer"` // the address who pays the fee - // Gas coin.Coin `json:"gas"` // ????? + Tx basecoin.Tx `json:"tx"` +} + +// NewFee wraps a tx with a promised fee from this actor +func NewFee(tx basecoin.Tx, fee coin.Coin, payer basecoin.Actor) basecoin.Tx { + return Fee{Tx: tx, Fee: fee, Payer: payer}.Wrap() } // nolint - TxInner Functions -func NewFee(tx basecoin.Tx, fee coin.Coin, payer basecoin.Actor) basecoin.Tx { - return (&Fee{Tx: tx, Fee: fee, Payer: payer}).Wrap() -} -func (f *Fee) ValidateBasic() error { +func (f Fee) ValidateBasic() error { // TODO: more checks return f.Tx.ValidateBasic() } -func (f *Fee) Wrap() basecoin.Tx { +func (f Fee) Wrap() basecoin.Tx { return basecoin.Tx{f} } -func (f *Fee) Next() basecoin.Tx { +func (f Fee) Next() basecoin.Tx { return f.Tx } diff --git a/modules/roles/handler.go b/modules/roles/handler.go index 2ade96ee1c..0a31eb6eba 100644 --- a/modules/roles/handler.go +++ b/modules/roles/handler.go @@ -56,7 +56,7 @@ func checkTx(ctx basecoin.Context, tx basecoin.Tx) (create CreateRoleTx, err err // check if the tx is proper type and valid create, ok := tx.Unwrap().(CreateRoleTx) if !ok { - return create, errors.ErrInvalidFormat(tx) + return create, errors.ErrInvalidFormat(TypeCreateRoleTx, tx) } err = create.ValidateBasic() return create, err diff --git a/stack/helpers.go b/stack/helpers.go index 5dce066075..7405c16dda 100644 --- a/stack/helpers.go +++ b/stack/helpers.go @@ -21,7 +21,7 @@ const ( ByteRawTx = 0xF0 ByteCheckTx = 0xF1 - TypeRawTx = "raw" + TypeRawTx = NameOK + "/raw" // this will just say a-ok to RawTx TypeCheckTx = NameCheck + "/tx" rawMaxSize = 2000 * 1000 diff --git a/tests/cli/basictx.sh b/tests/cli/basictx.sh index f3db1f2f6d..0ecc4d9711 100755 --- a/tests/cli/basictx.sh +++ b/tests/cli/basictx.sh @@ -23,7 +23,7 @@ test00GetAccount() { assertFalse "requires arg" "${CLIENT_EXE} query account" - checkAccount $SENDER "0" "9007199254740992" + checkAccount $SENDER "9007199254740992" ACCT2=$(${CLIENT_EXE} query account $RECV 2>/dev/null) assertFalse "has no genesis account" $? @@ -40,15 +40,33 @@ test01SendTx() { HASH=$(echo $TX | jq .hash | tr -d \") TX_HEIGHT=$(echo $TX | jq .height) - checkAccount $SENDER "1" "9007199254740000" + checkAccount $SENDER "9007199254740000" # make sure 0x prefix also works - checkAccount "0x$SENDER" "1" "9007199254740000" - checkAccount $RECV "0" "992" + checkAccount "0x$SENDER" "9007199254740000" + checkAccount $RECV "992" # Make sure tx is indexed checkSendTx $HASH $TX_HEIGHT $SENDER "992" } +test02SendTxWithFee() { + SENDER=$(getAddr $RICH) + RECV=$(getAddr $POOR) + + TX=$(echo qwertyuiop | ${CLIENT_EXE} tx send --amount=90mycoin --fee=10mycoin --sequence=2 --to=$RECV --name=$RICH) + txSucceeded $? "$TX" "$RECV" + HASH=$(echo $TX | jq .hash | tr -d \") + TX_HEIGHT=$(echo $TX | jq .height) + + # deduct 100 from sender, add 90 to receiver... fees "vanish" + checkAccount $SENDER "9007199254739900" + checkAccount $RECV "1082" + + # Make sure tx is indexed + checkSendFeeTx $HASH $TX_HEIGHT $SENDER "90" "10" +} + + # Load common then run these tests with shunit2! DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" #get this files directory . $DIR/common.sh diff --git a/tests/cli/common.sh b/tests/cli/common.sh index 94219d6108..c361c1df84 100644 --- a/tests/cli/common.sh +++ b/tests/cli/common.sh @@ -124,6 +124,7 @@ getAddr() { echo $RAW | cut -d' ' -f2 } +# XXX Ex Usage: checkAccount $ADDR $AMOUNT # Desc: Assumes just one coin, checks the balance of first coin in any case checkAccount() { # make sure sender goes down @@ -133,8 +134,7 @@ checkAccount() { fi if [ -n "$DEBUG" ]; then echo $ACCT; echo; fi - assertEquals "proper sequence" "$2" $(echo $ACCT | jq .data.sequence) - assertEquals "proper money" "$3" $(echo $ACCT | jq .data.coins[0].amount) + assertEquals "proper money" "$2" $(echo $ACCT | jq .data.coins[0].amount) return $? } @@ -169,6 +169,30 @@ checkSendTx() { return $? } +# XXX Ex Usage: checkSendFeeTx $HASH $HEIGHT $SENDER $AMOUNT $FEE +# Desc: This is like checkSendTx, but asserts a feetx wrapper with $FEE value. +# This looks up the tx by hash, and makes sure the height and type match +# and that the first input was from this sender for this amount +checkSendFeeTx() { + TX=$(${CLIENT_EXE} query tx $1) + assertTrue "found tx" $? + if [ -n "$DEBUG" ]; then echo $TX; echo; fi + + 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=chain/tx" '"chain/tx"' $(echo $CTX | jq .type) + FTX=$(echo $CTX | jq .data.tx) + assertEquals "type=fee/tx" '"fee/tx"' $(echo $FTX | jq .type) + assertEquals "proper fee" "$5" $(echo $FTX | jq .data.fee.amount) + STX=$(echo $FTX | jq .data.tx) + assertEquals "type=coin/send" '"coin/send"' $(echo $STX | jq .type) + assertEquals "proper sender" "\"$3\"" $(echo $STX | jq .data.inputs[0].address.addr) + assertEquals "proper out amount" "$4" $(echo $STX | jq .data.outputs[0].coins[0].amount) + return $? +} + + # XXX Ex Usage: waitForBlock $port # Desc: Waits until the block height on that node increases by one waitForBlock() { diff --git a/tests/cli/counter.sh b/tests/cli/counter.sh index 378348e57a..30fb29ab40 100755 --- a/tests/cli/counter.sh +++ b/tests/cli/counter.sh @@ -23,7 +23,7 @@ test00GetAccount() { assertFalse "requires arg" "${CLIENT_EXE} query account" - checkAccount $SENDER "0" "9007199254740992" + checkAccount $SENDER "9007199254740992" ACCT2=$(${CLIENT_EXE} query account $RECV 2>/dev/null) assertFalse "has no genesis account" $? @@ -40,8 +40,8 @@ test01SendTx() { HASH=$(echo $TX | jq .hash | tr -d \") TX_HEIGHT=$(echo $TX | jq .height) - checkAccount $SENDER "1" "9007199254740000" - checkAccount $RECV "0" "992" + checkAccount $SENDER "9007199254740000" + checkAccount $RECV "992" # make sure tx is indexed checkSendTx $HASH $TX_HEIGHT $SENDER "992" @@ -76,7 +76,7 @@ test03AddCount() { checkCounter "1" "10" # make sure the account was debited - checkAccount $SENDER "2" "9007199254739990" + checkAccount $SENDER "9007199254739990" # make sure tx is indexed TX=$(${CLIENT_EXE} query tx $HASH --trace) @@ -89,6 +89,16 @@ test03AddCount() { assertEquals "type=cntr/count" '"cntr/count"' $(echo $CNTX | jq .type) assertEquals "proper fee" "10" $(echo $CNTX | jq .data.fee[0].amount) fi + + # test again with fees... + TX=$(echo qwertyuiop | ${CLIENT_EXE} tx counter --countfee=7mycoin --fee=4mycoin --sequence=3 --name=${RICH} --valid) + txSucceeded $? "$TX" "counter" + + # make sure the counter was updated, added 7 + checkCounter "2" "17" + + # make sure the account was debited 11 + checkAccount $SENDER "9007199254739979" } # Load common then run these tests with shunit2! diff --git a/tests/cli/restart.sh b/tests/cli/restart.sh index 993db2eb77..c6c6a500ba 100755 --- a/tests/cli/restart.sh +++ b/tests/cli/restart.sh @@ -26,8 +26,8 @@ test00PreRestart() { HASH=$(echo $TX | jq .hash | tr -d \") TX_HEIGHT=$(echo $TX | jq .height) - checkAccount $SENDER "1" "9007199254740000" - checkAccount $RECV "0" "992" + checkAccount $SENDER "9007199254740000" + checkAccount $RECV "992" # make sure tx is indexed checkSendTx $HASH $TX_HEIGHT $SENDER "992" @@ -63,8 +63,8 @@ test01OnRestart() { # make sure queries still work properly, with all 3 tx now executed echo "Checking state after restart..." - checkAccount $SENDER "3" "9007199254710000" - checkAccount $RECV "0" "30992" + checkAccount $SENDER "9007199254710000" + checkAccount $RECV "30992" # make sure tx is indexed checkSendTx $HASH $TX_HEIGHT $SENDER "10000"