From d0a2041c89d28aa3baa6e585646e225946e0c090 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 16:47:03 +0200 Subject: [PATCH 01/24] Remove references to IBC from binaries --- Makefile | 4 +- cmd/basecoin/commands/ibc.go | 10 +- cmd/basecoin/commands/relay.go | 492 ++++++++++++++++----------------- cmd/basecoin/commands/start.go | 2 +- cmd/basecoin/main.go | 2 +- 5 files changed, 255 insertions(+), 255 deletions(-) diff --git a/Makefile b/Makefile index b13954ce4f..bd7d851fd7 100644 --- a/Makefile +++ b/Makefile @@ -25,9 +25,9 @@ test_unit: test_cli: tests/cli/shunit2 # sudo apt-get install jq @./tests/cli/basictx.sh - @./tests/cli/counter.sh + # @./tests/cli/counter.sh @./tests/cli/restart.sh - @./tests/cli/ibc.sh + # @./tests/cli/ibc.sh test_tutorial: docs/guide/shunit2 shelldown ${TUTORIALS} diff --git a/cmd/basecoin/commands/ibc.go b/cmd/basecoin/commands/ibc.go index b76eae9023..3cb98a9fb9 100644 --- a/cmd/basecoin/commands/ibc.go +++ b/cmd/basecoin/commands/ibc.go @@ -1,8 +1,8 @@ package commands -import "github.com/tendermint/basecoin/plugins/ibc" +// import "github.com/tendermint/basecoin/plugins/ibc" -// returns a new IBC plugin to be registered with Basecoin -func NewIBCPlugin() *ibc.IBCPlugin { - return ibc.New() -} +// // returns a new IBC plugin to be registered with Basecoin +// func NewIBCPlugin() *ibc.IBCPlugin { +// return ibc.New() +// } diff --git a/cmd/basecoin/commands/relay.go b/cmd/basecoin/commands/relay.go index b9ae47afa4..13bdeb9dcc 100644 --- a/cmd/basecoin/commands/relay.go +++ b/cmd/basecoin/commands/relay.go @@ -1,298 +1,298 @@ package commands -import ( - "fmt" - "io/ioutil" - "strconv" - "time" +// import ( +// "fmt" +// "io/ioutil" +// "strconv" +// "time" - "github.com/pkg/errors" - "github.com/spf13/cobra" +// "github.com/pkg/errors" +// "github.com/spf13/cobra" - // "github.com/spf13/viper" - // "github.com/tendermint/tmlibs/cli" - // "github.com/tendermint/tmlibs/log" +// // "github.com/spf13/viper" +// // "github.com/tendermint/tmlibs/cli" +// // "github.com/tendermint/tmlibs/log" - "github.com/tendermint/go-wire" - "github.com/tendermint/merkleeyes/iavl" - cmn "github.com/tendermint/tmlibs/common" +// "github.com/tendermint/go-wire" +// "github.com/tendermint/merkleeyes/iavl" +// cmn "github.com/tendermint/tmlibs/common" - "github.com/tendermint/basecoin/plugins/ibc" - "github.com/tendermint/basecoin/types" - "github.com/tendermint/tendermint/rpc/client" - tmtypes "github.com/tendermint/tendermint/types" -) +// "github.com/tendermint/basecoin/plugins/ibc" +// "github.com/tendermint/basecoin/types" +// "github.com/tendermint/tendermint/rpc/client" +// tmtypes "github.com/tendermint/tendermint/types" +// ) -var RelayCmd = &cobra.Command{ - Use: "relay", - Short: "Relay ibc packets between two chains", -} +// var RelayCmd = &cobra.Command{ +// Use: "relay", +// Short: "Relay ibc packets between two chains", +// } -var RelayStartCmd = &cobra.Command{ - Use: "start", - Short: "Start basecoin relayer to relay IBC packets between chains", - RunE: relayStartCmd, -} +// var RelayStartCmd = &cobra.Command{ +// Use: "start", +// Short: "Start basecoin relayer to relay IBC packets between chains", +// RunE: relayStartCmd, +// } -var RelayInitCmd = &cobra.Command{ - Use: "init", - Short: "Register both chains with each other, to prepare the relayer to run", - RunE: relayInitCmd, -} +// var RelayInitCmd = &cobra.Command{ +// Use: "init", +// Short: "Register both chains with each other, to prepare the relayer to run", +// RunE: relayInitCmd, +// } -//flags -var ( - chain1AddrFlag string - chain2AddrFlag string +// //flags +// var ( +// chain1AddrFlag string +// chain2AddrFlag string - chain1IDFlag string - chain2IDFlag string +// chain1IDFlag string +// chain2IDFlag string - fromFileFlag string +// fromFileFlag string - genesisFile1Flag string - genesisFile2Flag string -) +// genesisFile1Flag string +// genesisFile2Flag string +// ) -func init() { - flags := []Flag2Register{ - {&chain1AddrFlag, "chain1-addr", "tcp://localhost:46657", "Node address for chain1"}, - {&chain2AddrFlag, "chain2-addr", "tcp://localhost:36657", "Node address for chain2"}, - {&chain1IDFlag, "chain1-id", "test_chain_1", "ChainID for chain1"}, - {&chain2IDFlag, "chain2-id", "test_chain_2", "ChainID for chain2"}, - {&fromFileFlag, "from", "key.json", "Path to a private key to sign the transaction"}, - } - RegisterPersistentFlags(RelayCmd, flags) +// func init() { +// flags := []Flag2Register{ +// {&chain1AddrFlag, "chain1-addr", "tcp://localhost:46657", "Node address for chain1"}, +// {&chain2AddrFlag, "chain2-addr", "tcp://localhost:36657", "Node address for chain2"}, +// {&chain1IDFlag, "chain1-id", "test_chain_1", "ChainID for chain1"}, +// {&chain2IDFlag, "chain2-id", "test_chain_2", "ChainID for chain2"}, +// {&fromFileFlag, "from", "key.json", "Path to a private key to sign the transaction"}, +// } +// RegisterPersistentFlags(RelayCmd, flags) - initFlags := []Flag2Register{ - {&genesisFile1Flag, "genesis1", "", "Path to genesis file for chain1"}, - {&genesisFile2Flag, "genesis2", "", "Path to genesis file for chain2"}, - } - RegisterFlags(RelayInitCmd, initFlags) +// initFlags := []Flag2Register{ +// {&genesisFile1Flag, "genesis1", "", "Path to genesis file for chain1"}, +// {&genesisFile2Flag, "genesis2", "", "Path to genesis file for chain2"}, +// } +// RegisterFlags(RelayInitCmd, initFlags) - RelayCmd.AddCommand(RelayStartCmd) - RelayCmd.AddCommand(RelayInitCmd) -} +// RelayCmd.AddCommand(RelayStartCmd) +// RelayCmd.AddCommand(RelayInitCmd) +// } -func relayStartCmd(cmd *cobra.Command, args []string) error { - go loop(chain1AddrFlag, chain2AddrFlag, chain1IDFlag, chain2IDFlag) - go loop(chain2AddrFlag, chain1AddrFlag, chain2IDFlag, chain1IDFlag) +// func relayStartCmd(cmd *cobra.Command, args []string) error { +// go loop(chain1AddrFlag, chain2AddrFlag, chain1IDFlag, chain2IDFlag) +// go loop(chain2AddrFlag, chain1AddrFlag, chain2IDFlag, chain1IDFlag) - cmn.TrapSignal(func() { - // TODO: Cleanup - }) - return nil -} +// cmn.TrapSignal(func() { +// // TODO: Cleanup +// }) +// return nil +// } -func relayInitCmd(cmd *cobra.Command, args []string) error { - err := registerChain(chain1IDFlag, chain1AddrFlag, chain2IDFlag, genesisFile2Flag, fromFileFlag) - if err != nil { - return err - } - err = registerChain(chain2IDFlag, chain2AddrFlag, chain1IDFlag, genesisFile1Flag, fromFileFlag) - return err -} +// func relayInitCmd(cmd *cobra.Command, args []string) error { +// err := registerChain(chain1IDFlag, chain1AddrFlag, chain2IDFlag, genesisFile2Flag, fromFileFlag) +// if err != nil { +// return err +// } +// err = registerChain(chain2IDFlag, chain2AddrFlag, chain1IDFlag, genesisFile1Flag, fromFileFlag) +// return err +// } -func registerChain(chainID, node, registerChainID, registerGenesis, keyFile string) error { - genesisBytes, err := ioutil.ReadFile(registerGenesis) - if err != nil { - return errors.Errorf("Error reading genesis file %v: %v\n", registerGenesis, err) - } +// func registerChain(chainID, node, registerChainID, registerGenesis, keyFile string) error { +// genesisBytes, err := ioutil.ReadFile(registerGenesis) +// if err != nil { +// return errors.Errorf("Error reading genesis file %v: %v\n", registerGenesis, err) +// } - ibcTx := ibc.IBCRegisterChainTx{ - ibc.BlockchainGenesis{ - ChainID: registerChainID, - Genesis: string(genesisBytes), - }, - } +// ibcTx := ibc.IBCRegisterChainTx{ +// ibc.BlockchainGenesis{ +// ChainID: registerChainID, +// Genesis: string(genesisBytes), +// }, +// } - privKey, err := LoadKey(keyFile) - if err != nil { - return err - } - relay := newRelayer(privKey, chainID, node) - return relay.appTx(ibcTx) -} +// privKey, err := LoadKey(keyFile) +// if err != nil { +// return err +// } +// relay := newRelayer(privKey, chainID, node) +// return relay.appTx(ibcTx) +// } -func loop(addr1, addr2, id1, id2 string) { - nextSeq := 0 +// func loop(addr1, addr2, id1, id2 string) { +// nextSeq := 0 - // load the priv key - privKey, err := LoadKey(fromFileFlag) - if err != nil { - logger.Error(err.Error()) - cmn.PanicCrisis(err.Error()) - } +// // load the priv key +// privKey, err := LoadKey(fromFileFlag) +// if err != nil { +// logger.Error(err.Error()) +// cmn.PanicCrisis(err.Error()) +// } - // relay from chain1 to chain2 - thisRelayer := newRelayer(privKey, id2, addr2) +// // relay from chain1 to chain2 +// thisRelayer := newRelayer(privKey, id2, addr2) - logger.Info(fmt.Sprintf("Relaying from chain %v on %v to chain %v on %v", id1, addr1, id2, addr2)) +// logger.Info(fmt.Sprintf("Relaying from chain %v on %v to chain %v on %v", id1, addr1, id2, addr2)) - httpClient := client.NewHTTP(addr1, "/websocket") +// httpClient := client.NewHTTP(addr1, "/websocket") -OUTER: - for { +// OUTER: +// for { - time.Sleep(time.Second) +// time.Sleep(time.Second) - // get the latest ibc packet sequence number - key := fmt.Sprintf("ibc,egress,%v,%v", id1, id2) - query, err := queryWithClient(httpClient, []byte(key)) - if err != nil { - logger.Error("Error querying for latest sequence", "key", key, "error", err.Error()) - continue OUTER - } - if len(query.Value) == 0 { - // nothing yet - continue OUTER - } +// // get the latest ibc packet sequence number +// key := fmt.Sprintf("ibc,egress,%v,%v", id1, id2) +// query, err := queryWithClient(httpClient, []byte(key)) +// if err != nil { +// logger.Error("Error querying for latest sequence", "key", key, "error", err.Error()) +// continue OUTER +// } +// if len(query.Value) == 0 { +// // nothing yet +// continue OUTER +// } - seq, err := strconv.ParseUint(string(query.Value), 10, 64) - if err != nil { - logger.Error("Error parsing sequence number from query", "query.Value", query.Value, "error", err.Error()) - continue OUTER - } - seq -= 1 // seq is the packet count. -1 because 0-indexed +// seq, err := strconv.ParseUint(string(query.Value), 10, 64) +// if err != nil { +// logger.Error("Error parsing sequence number from query", "query.Value", query.Value, "error", err.Error()) +// continue OUTER +// } +// seq -= 1 // seq is the packet count. -1 because 0-indexed - if nextSeq <= int(seq) { - logger.Info("Got new packets", "last-sequence", nextSeq-1, "new-sequence", seq) - } +// if nextSeq <= int(seq) { +// logger.Info("Got new packets", "last-sequence", nextSeq-1, "new-sequence", seq) +// } - // get all packets since the last one we relayed - for ; nextSeq <= int(seq); nextSeq++ { - key := fmt.Sprintf("ibc,egress,%v,%v,%d", id1, id2, nextSeq) - query, err := queryWithClient(httpClient, []byte(key)) - if err != nil { - logger.Error("Error querying for packet", "seqeuence", nextSeq, "key", key, "error", err.Error()) - continue OUTER - } +// // get all packets since the last one we relayed +// for ; nextSeq <= int(seq); nextSeq++ { +// key := fmt.Sprintf("ibc,egress,%v,%v,%d", id1, id2, nextSeq) +// query, err := queryWithClient(httpClient, []byte(key)) +// if err != nil { +// logger.Error("Error querying for packet", "seqeuence", nextSeq, "key", key, "error", err.Error()) +// continue OUTER +// } - var packet ibc.Packet - err = wire.ReadBinaryBytes(query.Value, &packet) - if err != nil { - logger.Error("Error unmarshalling packet", "key", key, "query.Value", query.Value, "error", err.Error()) - continue OUTER - } +// var packet ibc.Packet +// err = wire.ReadBinaryBytes(query.Value, &packet) +// if err != nil { +// logger.Error("Error unmarshalling packet", "key", key, "query.Value", query.Value, "error", err.Error()) +// continue OUTER +// } - proof := new(iavl.IAVLProof) - err = wire.ReadBinaryBytes(query.Proof, &proof) - if err != nil { - logger.Error("Error unmarshalling proof", "query.Proof", query.Proof, "error", err.Error()) - continue OUTER - } +// proof := new(iavl.IAVLProof) +// err = wire.ReadBinaryBytes(query.Proof, &proof) +// if err != nil { +// logger.Error("Error unmarshalling proof", "query.Proof", query.Proof, "error", err.Error()) +// continue OUTER +// } - // query.Height is actually for the next block, - // so wait a block before we fetch the header & commit - if err := waitForBlock(httpClient); err != nil { - logger.Error("Error waiting for a block", "addr", addr1, "error", err.Error()) - continue OUTER - } +// // query.Height is actually for the next block, +// // so wait a block before we fetch the header & commit +// if err := waitForBlock(httpClient); err != nil { +// logger.Error("Error waiting for a block", "addr", addr1, "error", err.Error()) +// continue OUTER +// } - // get the header and commit from the height the query was done at - res, err := httpClient.Commit(int(query.Height)) - if err != nil { - logger.Error("Error fetching header and commits", "height", query.Height, "error", err.Error()) - continue OUTER - } +// // get the header and commit from the height the query was done at +// res, err := httpClient.Commit(int(query.Height)) +// if err != nil { +// logger.Error("Error fetching header and commits", "height", query.Height, "error", err.Error()) +// continue OUTER +// } - // update the chain state on the other chain - updateTx := ibc.IBCUpdateChainTx{ - Header: *res.Header, - Commit: *res.Commit, - } - logger.Info("Updating chain", "src-chain", id1, "height", res.Header.Height, "appHash", res.Header.AppHash) - if err := thisRelayer.appTx(updateTx); err != nil { - logger.Error("Error creating/sending IBCUpdateChainTx", "error", err.Error()) - continue OUTER - } +// // update the chain state on the other chain +// updateTx := ibc.IBCUpdateChainTx{ +// Header: *res.Header, +// Commit: *res.Commit, +// } +// logger.Info("Updating chain", "src-chain", id1, "height", res.Header.Height, "appHash", res.Header.AppHash) +// if err := thisRelayer.appTx(updateTx); err != nil { +// logger.Error("Error creating/sending IBCUpdateChainTx", "error", err.Error()) +// continue OUTER +// } - // relay the packet and proof - logger.Info("Relaying packet", "src-chain", id1, "height", query.Height, "sequence", nextSeq) - postTx := ibc.IBCPacketPostTx{ - FromChainID: id1, - FromChainHeight: query.Height, - Packet: packet, - Proof: proof, - } +// // relay the packet and proof +// logger.Info("Relaying packet", "src-chain", id1, "height", query.Height, "sequence", nextSeq) +// postTx := ibc.IBCPacketPostTx{ +// FromChainID: id1, +// FromChainHeight: query.Height, +// Packet: packet, +// Proof: proof, +// } - if err := thisRelayer.appTx(postTx); err != nil { - logger.Error("Error creating/sending IBCPacketPostTx", "error", err.Error()) - // dont `continue OUTER` here. the error might be eg. Already exists - // TODO: catch this programmatically ? - } - } - } -} +// if err := thisRelayer.appTx(postTx); err != nil { +// logger.Error("Error creating/sending IBCPacketPostTx", "error", err.Error()) +// // dont `continue OUTER` here. the error might be eg. Already exists +// // TODO: catch this programmatically ? +// } +// } +// } +// } -type relayer struct { - privKey *Key - chainID string - nodeAddr string - client *client.HTTP -} +// type relayer struct { +// privKey *Key +// chainID string +// nodeAddr string +// client *client.HTTP +// } -func newRelayer(privKey *Key, chainID, nodeAddr string) *relayer { - httpClient := client.NewHTTP(nodeAddr, "/websocket") - return &relayer{ - privKey: privKey, - chainID: chainID, - nodeAddr: nodeAddr, - client: httpClient, - } -} +// func newRelayer(privKey *Key, chainID, nodeAddr string) *relayer { +// httpClient := client.NewHTTP(nodeAddr, "/websocket") +// return &relayer{ +// privKey: privKey, +// chainID: chainID, +// nodeAddr: nodeAddr, +// client: httpClient, +// } +// } -func (r *relayer) appTx(ibcTx ibc.IBCTx) error { - acc, err := getAccWithClient(r.client, r.privKey.Address[:]) - if err != nil { - return err - } - sequence := acc.Sequence + 1 +// func (r *relayer) appTx(ibcTx ibc.IBCTx) error { +// acc, err := getAccWithClient(r.client, r.privKey.Address[:]) +// if err != nil { +// return err +// } +// sequence := acc.Sequence + 1 - data := []byte(wire.BinaryBytes(struct { - ibc.IBCTx `json:"unwrap"` - }{ibcTx})) +// data := []byte(wire.BinaryBytes(struct { +// ibc.IBCTx `json:"unwrap"` +// }{ibcTx})) - smallCoins := types.Coin{"mycoin", 1} +// smallCoins := types.Coin{"mycoin", 1} - input := types.NewTxInput(r.privKey.PubKey, types.Coins{smallCoins}, sequence) - tx := &types.AppTx{ - Gas: 0, - Fee: smallCoins, - Name: "IBC", - Input: input, - Data: data, - } +// input := types.NewTxInput(r.privKey.PubKey, types.Coins{smallCoins}, sequence) +// tx := &types.AppTx{ +// Gas: 0, +// Fee: smallCoins, +// Name: "IBC", +// Input: input, +// Data: data, +// } - tx.Input.Signature = r.privKey.Sign(tx.SignBytes(r.chainID)) - txBytes := []byte(wire.BinaryBytes(struct { - types.Tx `json:"unwrap"` - }{tx})) +// tx.Input.Signature = r.privKey.Sign(tx.SignBytes(r.chainID)) +// txBytes := []byte(wire.BinaryBytes(struct { +// types.Tx `json:"unwrap"` +// }{tx})) - data, log, err := broadcastTxWithClient(r.client, txBytes) - if err != nil { - return err - } - _, _ = data, log - return nil -} +// data, log, err := broadcastTxWithClient(r.client, txBytes) +// if err != nil { +// return err +// } +// _, _ = data, log +// return nil +// } -// broadcast the transaction to tendermint -func broadcastTxWithClient(httpClient *client.HTTP, tx tmtypes.Tx) ([]byte, string, error) { - res, err := httpClient.BroadcastTxCommit(tx) - if err != nil { - return nil, "", errors.Errorf("Error on broadcast tx: %v", err) - } +// // broadcast the transaction to tendermint +// func broadcastTxWithClient(httpClient *client.HTTP, tx tmtypes.Tx) ([]byte, string, error) { +// res, err := httpClient.BroadcastTxCommit(tx) +// if err != nil { +// return nil, "", errors.Errorf("Error on broadcast tx: %v", err) +// } - if !res.CheckTx.Code.IsOK() { - r := res.CheckTx - return nil, "", errors.Errorf("BroadcastTxCommit got non-zero exit code: %v. %X; %s", r.Code, r.Data, r.Log) - } +// if !res.CheckTx.Code.IsOK() { +// r := res.CheckTx +// return nil, "", errors.Errorf("BroadcastTxCommit got non-zero exit code: %v. %X; %s", r.Code, r.Data, r.Log) +// } - if !res.DeliverTx.Code.IsOK() { - r := res.DeliverTx - return nil, "", errors.Errorf("BroadcastTxCommit got non-zero exit code: %v. %X; %s", r.Code, r.Data, r.Log) - } +// if !res.DeliverTx.Code.IsOK() { +// r := res.DeliverTx +// return nil, "", errors.Errorf("BroadcastTxCommit got non-zero exit code: %v. %X; %s", r.Code, r.Data, r.Log) +// } - return res.DeliverTx.Data, res.DeliverTx.Log, nil -} +// return res.DeliverTx.Data, res.DeliverTx.Log, nil +// } diff --git a/cmd/basecoin/commands/start.go b/cmd/basecoin/commands/start.go index 31211bd7a8..8f6abd75f5 100644 --- a/cmd/basecoin/commands/start.go +++ b/cmd/basecoin/commands/start.go @@ -70,7 +70,7 @@ func startCmd(cmd *cobra.Command, args []string) error { basecoinApp.SetLogger(logger.With("module", "app")) // register IBC plugn - basecoinApp.RegisterPlugin(NewIBCPlugin()) + // basecoinApp.RegisterPlugin(NewIBCPlugin()) // register all other plugins for _, p := range plugins { diff --git a/cmd/basecoin/main.go b/cmd/basecoin/main.go index a0ce5a2630..7190a8ee59 100644 --- a/cmd/basecoin/main.go +++ b/cmd/basecoin/main.go @@ -13,7 +13,7 @@ func main() { rt.AddCommand( commands.InitCmd, commands.StartCmd, - commands.RelayCmd, + // commands.RelayCmd, commands.UnsafeResetAllCmd, commands.VersionCmd, ) From ef0ab758edd924bcb4776d576c4a515b421f8be2 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 16:54:47 +0200 Subject: [PATCH 02/24] Cleaned up logger on constructors in App, State --- app/app.go | 12 ++++-------- app/app_test.go | 6 ++---- app/genesis_test.go | 7 ++++--- cmd/basecoin/commands/start.go | 3 +-- docs/guide/counter/plugins/counter/counter_test.go | 3 ++- state/execution_test.go | 3 +-- state/state.go | 8 ++------ state/state_test.go | 9 +++------ tests/tmsp/tmsp_test.go | 5 ++--- 9 files changed, 21 insertions(+), 35 deletions(-) diff --git a/app/app.go b/app/app.go index 6f54782ff7..3b802d372f 100644 --- a/app/app.go +++ b/app/app.go @@ -29,23 +29,19 @@ type Basecoin struct { logger log.Logger } -func NewBasecoin(eyesCli *eyes.Client) *Basecoin { - state := sm.NewState(eyesCli) +func NewBasecoin(eyesCli *eyes.Client, l log.Logger) *Basecoin { + state := sm.NewState(eyesCli, l.With("module", "state")) + plugins := types.NewPlugins() return &Basecoin{ eyesCli: eyesCli, state: state, cacheState: nil, plugins: plugins, - logger: log.NewNopLogger(), + logger: l, } } -func (app *Basecoin) SetLogger(l log.Logger) { - app.logger = l - app.state.SetLogger(l.With("module", "state")) -} - // XXX For testing, not thread safe! func (app *Basecoin) GetState() *sm.State { return app.state.CacheWrap() diff --git a/app/app_test.go b/app/app_test.go index 1f150e8c48..e407b1e55d 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -56,8 +56,7 @@ func (at *appTest) reset() { at.accOut = types.MakeAcc("output0") eyesCli := eyes.NewLocalClient("", 0) - at.app = NewBasecoin(eyesCli) - at.app.SetLogger(log.TestingLogger().With("module", "app")) + at.app = NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) res := at.app.SetOption("base/chain_id", at.chainID) require.EqualValues(at.t, res, "Success") @@ -106,8 +105,7 @@ func TestSetOption(t *testing.T) { require := require.New(t) eyesCli := eyes.NewLocalClient("", 0) - app := NewBasecoin(eyesCli) - app.SetLogger(log.TestingLogger().With("module", "app")) + app := NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) //testing ChainID chainID := "testChain" diff --git a/app/genesis_test.go b/app/genesis_test.go index dc398afd97..1fb84acd97 100644 --- a/app/genesis_test.go +++ b/app/genesis_test.go @@ -11,6 +11,7 @@ import ( crypto "github.com/tendermint/go-crypto" eyescli "github.com/tendermint/merkleeyes/client" cmn "github.com/tendermint/tmlibs/common" + "github.com/tendermint/tmlibs/log" ) const genesisFilepath = "./testdata/genesis.json" @@ -18,7 +19,7 @@ const genesisAcctFilepath = "./testdata/genesis2.json" func TestLoadGenesisDoNotFailIfAppOptionsAreMissing(t *testing.T) { eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(eyesCli) + app := NewBasecoin(eyesCli, log.TestingLogger()) err := app.LoadGenesis("./testdata/genesis3.json") require.Nil(t, err, "%+v", err) } @@ -27,7 +28,7 @@ func TestLoadGenesis(t *testing.T) { assert, require := assert.New(t), require.New(t) eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(eyesCli) + app := NewBasecoin(eyesCli, log.TestingLogger()) err := app.LoadGenesis(genesisFilepath) require.Nil(err, "%+v", err) @@ -64,7 +65,7 @@ func TestLoadGenesisAccountAddress(t *testing.T) { assert, require := assert.New(t), require.New(t) eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(eyesCli) + app := NewBasecoin(eyesCli, log.TestingLogger()) err := app.LoadGenesis(genesisAcctFilepath) require.Nil(err, "%+v", err) diff --git a/cmd/basecoin/commands/start.go b/cmd/basecoin/commands/start.go index 8f6abd75f5..d27ddd152e 100644 --- a/cmd/basecoin/commands/start.go +++ b/cmd/basecoin/commands/start.go @@ -66,8 +66,7 @@ func startCmd(cmd *cobra.Command, args []string) error { } // Create Basecoin app - basecoinApp := app.NewBasecoin(eyesCli) - basecoinApp.SetLogger(logger.With("module", "app")) + basecoinApp := app.NewBasecoin(eyesCli, logger.With("module", "app")) // register IBC plugn // basecoinApp.RegisterPlugin(NewIBCPlugin()) diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index f1c7d9d354..a15ad3ecc5 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -11,6 +11,7 @@ import ( "github.com/tendermint/basecoin/types" "github.com/tendermint/go-wire" eyescli "github.com/tendermint/merkleeyes/client" + "github.com/tendermint/tmlibs/log" ) func TestCounterPlugin(t *testing.T) { @@ -19,7 +20,7 @@ func TestCounterPlugin(t *testing.T) { // Basecoin initialization eyesCli := eyescli.NewLocalClient("", 0) chainID := "test_chain_id" - bcApp := app.NewBasecoin(eyesCli) + bcApp := app.NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) bcApp.SetOption("base/chain_id", chainID) // t.Log(bcApp.Info()) diff --git a/state/execution_test.go b/state/execution_test.go index 91ce5a6ec2..983ae8d599 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -60,8 +60,7 @@ func (et *execTest) reset() { et.accOut = types.MakeAcc("bar") et.store = types.NewMemKVStore() - et.state = NewState(et.store) - et.state.SetLogger(log.TestingLogger()) + et.state = NewState(et.store, log.TestingLogger()) et.state.SetChainID(et.chainID) // NOTE we dont run acc2State here diff --git a/state/state.go b/state/state.go index e9daf73b12..da370da817 100644 --- a/state/state.go +++ b/state/state.go @@ -17,20 +17,16 @@ type State struct { logger log.Logger } -func NewState(store types.KVStore) *State { +func NewState(store types.KVStore, l log.Logger) *State { return &State{ chainID: "", store: store, readCache: make(map[string][]byte), writeCache: nil, - logger: log.NewNopLogger(), + logger: l, } } -func (s *State) SetLogger(l log.Logger) { - s.logger = l -} - func (s *State) SetChainID(chainID string) { s.chainID = chainID s.store.Set([]byte("base/chain_id"), []byte(chainID)) diff --git a/state/state_test.go b/state/state_test.go index dae620027a..990659d361 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -16,8 +16,7 @@ func TestState(t *testing.T) { //States and Stores for tests store := types.NewMemKVStore() - state := NewState(store) - state.SetLogger(log.TestingLogger()) + state := NewState(store, log.TestingLogger()) cache := state.CacheWrap() eyesCli := eyes.NewLocalClient("", 0) @@ -30,15 +29,13 @@ func TestState(t *testing.T) { //reset the store/state/cache reset := func() { store = types.NewMemKVStore() - state = NewState(store) - state.SetLogger(log.TestingLogger()) + state = NewState(store, log.TestingLogger()) cache = state.CacheWrap() } //set the state to using the eyesCli instead of MemKVStore useEyesCli := func() { - state = NewState(eyesCli) - state.SetLogger(log.TestingLogger()) + state = NewState(eyesCli, log.TestingLogger()) cache = state.CacheWrap() } diff --git a/tests/tmsp/tmsp_test.go b/tests/tmsp/tmsp_test.go index 19b32341b7..9de055ad65 100644 --- a/tests/tmsp/tmsp_test.go +++ b/tests/tmsp/tmsp_test.go @@ -17,8 +17,7 @@ import ( func TestSendTx(t *testing.T) { eyesCli := eyescli.NewLocalClient("", 0) chainID := "test_chain_id" - bcApp := app.NewBasecoin(eyesCli) - bcApp.SetLogger(log.TestingLogger().With("module", "app")) + bcApp := app.NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) bcApp.SetOption("base/chain_id", chainID) // t.Log(bcApp.Info()) @@ -64,7 +63,7 @@ func TestSendTx(t *testing.T) { func TestSequence(t *testing.T) { eyesCli := eyescli.NewLocalClient("", 0) chainID := "test_chain_id" - bcApp := app.NewBasecoin(eyesCli) + bcApp := app.NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) bcApp.SetOption("base/chain_id", chainID) // t.Log(bcApp.Info()) From 159574db8904f540e2deb69c4ec12de84a5dffef Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 17:32:01 +0200 Subject: [PATCH 03/24] Move ChainID into context --- app/app.go | 175 ++++++++++++++++----------------- app/app_test.go | 6 +- app/genesis_test.go | 6 +- cmd/basecoin/commands/start.go | 30 ++++-- context.go | 1 + modules/coin/handler_test.go | 4 +- stack/chain.go | 12 +-- stack/chain_test.go | 4 +- stack/context.go | 11 ++- stack/helpers_test.go | 6 +- stack/middleware.go | 4 +- stack/middleware_test.go | 2 +- stack/mock.go | 8 +- stack/recovery_test.go | 2 +- stack/signature_test.go | 2 +- tx.go | 21 ++++ 16 files changed, 170 insertions(+), 124 deletions(-) diff --git a/app/app.go b/app/app.go index 3b802d372f..940520285a 100644 --- a/app/app.go +++ b/app/app.go @@ -1,23 +1,22 @@ package app import ( - "encoding/hex" - "encoding/json" "strings" abci "github.com/tendermint/abci/types" - wire "github.com/tendermint/go-wire" + "github.com/tendermint/basecoin" eyes "github.com/tendermint/merkleeyes/client" cmn "github.com/tendermint/tmlibs/common" "github.com/tendermint/tmlibs/log" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" sm "github.com/tendermint/basecoin/state" - "github.com/tendermint/basecoin/types" "github.com/tendermint/basecoin/version" ) const ( - maxTxSize = 10240 PluginNameBase = "base" ) @@ -25,23 +24,29 @@ type Basecoin struct { eyesCli *eyes.Client state *sm.State cacheState *sm.State - plugins *types.Plugins + handler basecoin.Handler logger log.Logger } -func NewBasecoin(eyesCli *eyes.Client, l log.Logger) *Basecoin { +func NewBasecoin(h basecoin.Handler, eyesCli *eyes.Client, l log.Logger) *Basecoin { state := sm.NewState(eyesCli, l.With("module", "state")) - plugins := types.NewPlugins() return &Basecoin{ + handler: h, eyesCli: eyesCli, state: state, cacheState: nil, - plugins: plugins, logger: l, } } +// placeholder to just handle sendtx +func DefaultHandler() basecoin.Handler { + // use the default stack + h := coin.NewHandler() + return stack.NewDefault().Use(h) +} + // XXX For testing, not thread safe! func (app *Basecoin) GetState() *sm.State { return app.state.CacheWrap() @@ -60,87 +65,85 @@ func (app *Basecoin) Info() abci.ResponseInfo { } } -func (app *Basecoin) RegisterPlugin(plugin types.Plugin) { - app.plugins.RegisterPlugin(plugin) -} - // ABCI::SetOption func (app *Basecoin) SetOption(key string, value string) string { - pluginName, key := splitKey(key) - if pluginName != PluginNameBase { - // Set option on plugin - plugin := app.plugins.GetByName(pluginName) - if plugin == nil { - return "Invalid plugin name: " + pluginName - } - app.logger.Info("SetOption on plugin", "plugin", pluginName, "key", key, "value", value) - return plugin.SetOption(app.state, key, value) - } else { - // Set option on basecoin - switch key { - case "chain_id": - app.state.SetChainID(value) - return "Success" - case "account": - var acc GenesisAccount - err := json.Unmarshal([]byte(value), &acc) - if err != nil { - return "Error decoding acc message: " + err.Error() - } - acc.Balance.Sort() - addr, err := acc.GetAddr() - if err != nil { - return "Invalid address: " + err.Error() - } - app.state.SetAccount(addr, acc.ToAccount()) - app.logger.Info("SetAccount", "addr", hex.EncodeToString(addr), "acc", acc) + // TODO + return "todo" + // pluginName, key := splitKey(key) + // if pluginName != PluginNameBase { + // // Set option on plugin + // plugin := app.plugins.GetByName(pluginName) + // if plugin == nil { + // return "Invalid plugin name: " + pluginName + // } + // app.logger.Info("SetOption on plugin", "plugin", pluginName, "key", key, "value", value) + // return plugin.SetOption(app.state, key, value) + // } else { + // // Set option on basecoin + // switch key { + // case "chain_id": + // app.state.SetChainID(value) + // return "Success" + // case "account": + // var acc GenesisAccount + // err := json.Unmarshal([]byte(value), &acc) + // if err != nil { + // return "Error decoding acc message: " + err.Error() + // } + // acc.Balance.Sort() + // addr, err := acc.GetAddr() + // if err != nil { + // return "Invalid address: " + err.Error() + // } + // app.state.SetAccount(addr, acc.ToAccount()) + // app.logger.Info("SetAccount", "addr", hex.EncodeToString(addr), "acc", acc) - return "Success" - } - return "Unrecognized option key " + key - } + // return "Success" + // } + // return "Unrecognized option key " + key + // } } // ABCI::DeliverTx -func (app *Basecoin) DeliverTx(txBytes []byte) (res abci.Result) { - if len(txBytes) > maxTxSize { - return abci.ErrBaseEncodingError.AppendLog("Tx size exceeds maximum") - } - - // Decode tx - var tx types.Tx - err := wire.ReadBinaryBytes(txBytes, &tx) +func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { + tx, err := basecoin.LoadTx(txBytes) if err != nil { - return abci.ErrBaseEncodingError.AppendLog("Error decoding tx: " + err.Error()) + return errors.Result(err) } - // Validate and exec tx - res = sm.ExecTx(app.state, app.plugins, tx, false, nil) - if res.IsErr() { - return res.PrependLog("Error in DeliverTx") + // TODO: can we abstract this setup and commit logic?? + cache := app.state.CacheWrap() + ctx := stack.NewContext(app.state.GetChainID(), + app.logger.With("call", "delivertx")) + res, err := app.handler.DeliverTx(ctx, cache, tx) + + if err != nil { + // discard the cache... + return errors.Result(err) } - return res + // commit the cache and return result + cache.CacheSync() + return res.ToABCI() } // ABCI::CheckTx -func (app *Basecoin) CheckTx(txBytes []byte) (res abci.Result) { - if len(txBytes) > maxTxSize { - return abci.ErrBaseEncodingError.AppendLog("Tx size exceeds maximum") - } - - // Decode tx - var tx types.Tx - err := wire.ReadBinaryBytes(txBytes, &tx) +func (app *Basecoin) CheckTx(txBytes []byte) abci.Result { + tx, err := basecoin.LoadTx(txBytes) if err != nil { - return abci.ErrBaseEncodingError.AppendLog("Error decoding tx: " + err.Error()) + return errors.Result(err) } - // Validate tx - res = sm.ExecTx(app.cacheState, app.plugins, tx, true, nil) - if res.IsErr() { - return res.PrependLog("Error in CheckTx") + // TODO: can we abstract this setup and commit logic?? + ctx := stack.NewContext(app.state.GetChainID(), + app.logger.With("call", "checktx")) + // checktx generally shouldn't touch the state, but we don't care + // here on the framework level, since the cacheState is thrown away next block + res, err := app.handler.CheckTx(ctx, app.cacheState, tx) + + if err != nil { + return errors.Result(err) } - return abci.OK + return res.ToABCI() } // ABCI::Query @@ -151,12 +154,6 @@ func (app *Basecoin) Query(reqQuery abci.RequestQuery) (resQuery abci.ResponseQu return } - // handle special path for account info - if reqQuery.Path == "/account" { - reqQuery.Path = "/key" - reqQuery.Data = types.AccountKey(reqQuery.Data) - } - resQuery, err := app.eyesCli.QuerySync(reqQuery) if err != nil { resQuery.Log = "Failed to query MerkleEyes: " + err.Error() @@ -183,24 +180,24 @@ func (app *Basecoin) Commit() (res abci.Result) { // ABCI::InitChain func (app *Basecoin) InitChain(validators []*abci.Validator) { - for _, plugin := range app.plugins.GetList() { - plugin.InitChain(app.state, validators) - } + // for _, plugin := range app.plugins.GetList() { + // plugin.InitChain(app.state, validators) + // } } // ABCI::BeginBlock func (app *Basecoin) BeginBlock(hash []byte, header *abci.Header) { - for _, plugin := range app.plugins.GetList() { - plugin.BeginBlock(app.state, hash, header) - } + // for _, plugin := range app.plugins.GetList() { + // plugin.BeginBlock(app.state, hash, header) + // } } // ABCI::EndBlock func (app *Basecoin) EndBlock(height uint64) (res abci.ResponseEndBlock) { - for _, plugin := range app.plugins.GetList() { - pluginRes := plugin.EndBlock(app.state, height) - res.Diffs = append(res.Diffs, pluginRes.Diffs...) - } + // for _, plugin := range app.plugins.GetList() { + // pluginRes := plugin.EndBlock(app.state, height) + // res.Diffs = append(res.Diffs, pluginRes.Diffs...) + // } return } diff --git a/app/app_test.go b/app/app_test.go index e407b1e55d..1645853206 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -56,7 +56,8 @@ func (at *appTest) reset() { at.accOut = types.MakeAcc("output0") eyesCli := eyes.NewLocalClient("", 0) - at.app = NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) + at.app = NewBasecoin(DefaultHandler(), eyesCli, + log.TestingLogger().With("module", "app")) res := at.app.SetOption("base/chain_id", at.chainID) require.EqualValues(at.t, res, "Success") @@ -105,7 +106,8 @@ func TestSetOption(t *testing.T) { require := require.New(t) eyesCli := eyes.NewLocalClient("", 0) - app := NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) + app := NewBasecoin(DefaultHandler(), eyesCli, + log.TestingLogger().With("module", "app")) //testing ChainID chainID := "testChain" diff --git a/app/genesis_test.go b/app/genesis_test.go index 1fb84acd97..2575f3fa92 100644 --- a/app/genesis_test.go +++ b/app/genesis_test.go @@ -19,7 +19,7 @@ const genesisAcctFilepath = "./testdata/genesis2.json" func TestLoadGenesisDoNotFailIfAppOptionsAreMissing(t *testing.T) { eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(eyesCli, log.TestingLogger()) + app := NewBasecoin(DefaultHandler(), eyesCli, log.TestingLogger()) err := app.LoadGenesis("./testdata/genesis3.json") require.Nil(t, err, "%+v", err) } @@ -28,7 +28,7 @@ func TestLoadGenesis(t *testing.T) { assert, require := assert.New(t), require.New(t) eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(eyesCli, log.TestingLogger()) + app := NewBasecoin(DefaultHandler(), eyesCli, log.TestingLogger()) err := app.LoadGenesis(genesisFilepath) require.Nil(err, "%+v", err) @@ -65,7 +65,7 @@ func TestLoadGenesisAccountAddress(t *testing.T) { assert, require := assert.New(t), require.New(t) eyesCli := eyescli.NewLocalClient("", 0) - app := NewBasecoin(eyesCli, log.TestingLogger()) + app := NewBasecoin(DefaultHandler(), eyesCli, log.TestingLogger()) err := app.LoadGenesis(genesisAcctFilepath) require.Nil(err, "%+v", err) diff --git a/cmd/basecoin/commands/start.go b/cmd/basecoin/commands/start.go index d27ddd152e..8f12efed35 100644 --- a/cmd/basecoin/commands/start.go +++ b/cmd/basecoin/commands/start.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/viper" "github.com/tendermint/abci/server" + "github.com/tendermint/basecoin" eyesApp "github.com/tendermint/merkleeyes/app" eyes "github.com/tendermint/merkleeyes/client" "github.com/tendermint/tmlibs/cli" @@ -21,6 +22,8 @@ import ( "github.com/tendermint/tendermint/types" "github.com/tendermint/basecoin/app" + "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" ) var StartCmd = &cobra.Command{ @@ -48,6 +51,22 @@ func init() { tcmd.AddNodeFlags(StartCmd) } +// TODO: setup handler instead of Plugins +func getHandler() basecoin.Handler { + // use the default stack + h := coin.NewHandler() + app := stack.NewDefault("change-this").Use(h) + return app + + // register IBC plugn + // basecoinApp.RegisterPlugin(NewIBCPlugin()) + + // register all other plugins + // for _, p := range plugins { + // basecoinApp.RegisterPlugin(p.newPlugin()) + // } +} + func startCmd(cmd *cobra.Command, args []string) error { rootDir := viper.GetString(cli.HomeFlag) meyes := viper.GetString(FlagEyes) @@ -66,15 +85,8 @@ func startCmd(cmd *cobra.Command, args []string) error { } // Create Basecoin app - basecoinApp := app.NewBasecoin(eyesCli, logger.With("module", "app")) - - // register IBC plugn - // basecoinApp.RegisterPlugin(NewIBCPlugin()) - - // register all other plugins - for _, p := range plugins { - basecoinApp.RegisterPlugin(p.newPlugin()) - } + h := app.DefaultHandler() + basecoinApp := app.NewBasecoin(h, eyesCli, logger.With("module", "app")) // if chain_id has not been set yet, load the genesis. // else, assume it's been loaded diff --git a/context.go b/context.go index a7737d5f52..8532cc6c9a 100644 --- a/context.go +++ b/context.go @@ -34,4 +34,5 @@ type Context interface { HasPermission(perm Actor) bool IsParent(ctx Context) bool Reset() Context + ChainID() string } diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index a2804f2591..8027b489c6 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -68,7 +68,7 @@ func TestHandlerValidation(t *testing.T) { } for i, tc := range cases { - ctx := stack.MockContext().WithPermissions(tc.perms...) + ctx := stack.MockContext("base-chain").WithPermissions(tc.perms...) _, err := checkTx(ctx, tc.tx) if tc.valid { assert.Nil(err, "%d: %+v", i, err) @@ -142,7 +142,7 @@ func TestDeliverTx(t *testing.T) { require.Nil(err, "%d: %+v", i, err) } - ctx := stack.MockContext().WithPermissions(tc.perms...) + ctx := stack.MockContext("base-chain").WithPermissions(tc.perms...) _, err := h.DeliverTx(ctx, store, tc.tx) if len(tc.final) > 0 { // valid assert.Nil(err, "%d: %+v", i, err) diff --git a/stack/chain.go b/stack/chain.go index ef174495b8..70da742f67 100644 --- a/stack/chain.go +++ b/stack/chain.go @@ -12,9 +12,7 @@ const ( ) // Chain enforces that this tx was bound to the named chain -type Chain struct { - ChainID string -} +type Chain struct{} func (_ Chain) Name() string { return NameRecovery @@ -23,7 +21,7 @@ func (_ Chain) Name() string { var _ Middleware = Chain{} func (c Chain) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { - stx, err := c.checkChain(tx) + stx, err := c.checkChain(ctx.ChainID(), tx) if err != nil { return res, err } @@ -31,7 +29,7 @@ func (c Chain) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx } func (c Chain) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { - stx, err := c.checkChain(tx) + stx, err := c.checkChain(ctx.ChainID(), tx) if err != nil { return res, err } @@ -39,12 +37,12 @@ func (c Chain) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin. } // checkChain makes sure the tx is a txs.Chain and -func (c Chain) checkChain(tx basecoin.Tx) (basecoin.Tx, error) { +func (c Chain) checkChain(chainID string, tx basecoin.Tx) (basecoin.Tx, error) { ctx, ok := tx.Unwrap().(*txs.Chain) if !ok { return tx, errors.ErrNoChain() } - if ctx.ChainID != c.ChainID { + if ctx.ChainID != chainID { return tx, errors.ErrWrongChain(ctx.ChainID) } return ctx.Tx, nil diff --git a/stack/chain_test.go b/stack/chain_test.go index 8d7833ae87..c9afaa7001 100644 --- a/stack/chain_test.go +++ b/stack/chain_test.go @@ -30,12 +30,12 @@ func TestChain(t *testing.T) { } // generic args here... - ctx := NewContext(log.NewNopLogger()) + ctx := NewContext(chainID, log.NewNopLogger()) store := types.NewMemKVStore() // build the stack ok := OKHandler{msg} - app := New(Chain{chainID}).Use(ok) + app := New(Chain{}).Use(ok) for idx, tc := range cases { i := strconv.Itoa(idx) diff --git a/stack/context.go b/stack/context.go index 09703b9445..6124a96780 100644 --- a/stack/context.go +++ b/stack/context.go @@ -17,20 +17,26 @@ type nonce int64 type secureContext struct { id nonce + chain string app string perms []basecoin.Actor log.Logger } -func NewContext(logger log.Logger) basecoin.Context { +func NewContext(chain string, logger log.Logger) basecoin.Context { return secureContext{ id: nonce(rand.Int63()), + chain: chain, Logger: logger, } } var _ basecoin.Context = secureContext{} +func (c secureContext) ChainID() string { + return c.chain +} + // WithPermissions will panic if they try to set permission without the proper app func (c secureContext) WithPermissions(perms ...basecoin.Actor) basecoin.Context { // the guard makes sure you only set permissions for the app you are inside @@ -44,6 +50,7 @@ func (c secureContext) WithPermissions(perms ...basecoin.Actor) basecoin.Context return secureContext{ id: c.id, + chain: c.chain, app: c.app, perms: append(c.perms, perms...), Logger: c.Logger, @@ -73,6 +80,7 @@ func (c secureContext) IsParent(other basecoin.Context) bool { func (c secureContext) Reset() basecoin.Context { return secureContext{ id: c.id, + chain: c.chain, app: c.app, perms: nil, Logger: c.Logger, @@ -88,6 +96,7 @@ func withApp(ctx basecoin.Context, app string) basecoin.Context { } return secureContext{ id: sc.id, + chain: sc.chain, app: app, perms: sc.perms, Logger: sc.Logger, diff --git a/stack/helpers_test.go b/stack/helpers_test.go index d792cf9535..10e3655800 100644 --- a/stack/helpers_test.go +++ b/stack/helpers_test.go @@ -15,7 +15,7 @@ import ( func TestOK(t *testing.T) { assert := assert.New(t) - ctx := NewContext(log.NewNopLogger()) + ctx := NewContext("test-chain", log.NewNopLogger()) store := types.NewMemKVStore() data := "this looks okay" tx := basecoin.Tx{} @@ -33,7 +33,7 @@ func TestOK(t *testing.T) { func TestFail(t *testing.T) { assert := assert.New(t) - ctx := NewContext(log.NewNopLogger()) + ctx := NewContext("test-chain", log.NewNopLogger()) store := types.NewMemKVStore() msg := "big problem" tx := basecoin.Tx{} @@ -53,7 +53,7 @@ func TestFail(t *testing.T) { func TestPanic(t *testing.T) { assert := assert.New(t) - ctx := NewContext(log.NewNopLogger()) + ctx := NewContext("test-chain", log.NewNopLogger()) store := types.NewMemKVStore() msg := "system crash!" tx := basecoin.Tx{} diff --git a/stack/middleware.go b/stack/middleware.go index 7dfc5cf04a..f61b233da2 100644 --- a/stack/middleware.go +++ b/stack/middleware.go @@ -57,12 +57,12 @@ func New(middlewares ...Middleware) *Stack { // NewDefault sets up the common middlewares before your custom stack. // // This is logger, recovery, signature, and chain -func NewDefault(chainID string, middlewares ...Middleware) *Stack { +func NewDefault(middlewares ...Middleware) *Stack { mids := []Middleware{ Logger{}, Recovery{}, Signatures{}, - Chain{chainID}, + Chain{}, } mids = append(mids, middlewares...) return New(mids...) diff --git a/stack/middleware_test.go b/stack/middleware_test.go index 74fa431c8d..adbb970932 100644 --- a/stack/middleware_test.go +++ b/stack/middleware_test.go @@ -17,7 +17,7 @@ func TestPermissionSandbox(t *testing.T) { require := require.New(t) // generic args - ctx := NewContext(log.NewNopLogger()) + ctx := NewContext("test-chain", log.NewNopLogger()) store := types.NewMemKVStore() raw := txs.NewRaw([]byte{1, 2, 3, 4}) rawBytes, err := data.ToWire(raw) diff --git a/stack/mock.go b/stack/mock.go index dbf8c2df74..443f83dfd8 100644 --- a/stack/mock.go +++ b/stack/mock.go @@ -10,17 +10,23 @@ import ( type mockContext struct { perms []basecoin.Actor + chain string log.Logger } -func MockContext() basecoin.Context { +func MockContext(chain string) basecoin.Context { return mockContext{ + chain: chain, Logger: log.NewNopLogger(), } } var _ basecoin.Context = mockContext{} +func (c mockContext) ChainID() string { + return c.chain +} + // WithPermissions will panic if they try to set permission without the proper app func (c mockContext) WithPermissions(perms ...basecoin.Actor) basecoin.Context { return mockContext{ diff --git a/stack/recovery_test.go b/stack/recovery_test.go index 58a6ba66ae..54fcbe6931 100644 --- a/stack/recovery_test.go +++ b/stack/recovery_test.go @@ -15,7 +15,7 @@ func TestRecovery(t *testing.T) { assert := assert.New(t) // generic args here... - ctx := NewContext(log.NewNopLogger()) + ctx := NewContext("test-chain", log.NewNopLogger()) store := types.NewMemKVStore() tx := basecoin.Tx{} diff --git a/stack/signature_test.go b/stack/signature_test.go index 1ca618c4cc..1e6f484ab3 100644 --- a/stack/signature_test.go +++ b/stack/signature_test.go @@ -17,7 +17,7 @@ func TestSignatureChecks(t *testing.T) { assert := assert.New(t) // generic args - ctx := NewContext(log.NewNopLogger()) + ctx := NewContext("test-chain", log.NewNopLogger()) store := types.NewMemKVStore() raw := txs.NewRaw([]byte{1, 2, 3, 4}) diff --git a/tx.go b/tx.go index 800a417528..8336f02b6c 100644 --- a/tx.go +++ b/tx.go @@ -1,5 +1,12 @@ package basecoin +import ( + "github.com/pkg/errors" + "github.com/tendermint/go-wire/data" +) + +const maxTxSize = 10240 + // TxInner is the interface all concrete transactions should implement. // // It adds bindings for clean un/marhsaling of the various implementations @@ -15,6 +22,20 @@ type TxInner interface { ValidateBasic() error } +// LoadTx parses a tx from data +// +// TODO: label both errors with abci.CodeType_EncodingError +// need to move errors to avoid import cycle +func LoadTx(bin []byte) (tx Tx, err error) { + if len(bin) > maxTxSize { + return tx, errors.New("Tx size exceeds maximum") + } + + // Decode tx + err = data.FromWire(bin, &tx) + return tx, err +} + // TODO: do we need this abstraction? TxLayer??? // please review again after implementing "middleware" From fa1a300943a5e2edcafa0ac823fa164b97cf826a Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 18:10:46 +0200 Subject: [PATCH 04/24] Add SetOption to all middleware and handlers --- cmd/basecoin/commands/start.go | 2 +- .../counter/plugins/counter/counter_test.go | 19 ++++-- handler.go | 49 +++++++++++---- modules/coin/handler.go | 7 +++ modules/fee/handler.go | 1 + stack/chain.go | 4 +- stack/chain_test.go | 2 +- stack/helpers.go | 7 ++- stack/helpers_test.go | 4 +- stack/helperware.go | 2 + stack/interface.go | 60 ++++++++++++++++--- stack/logger.go | 16 +++++ stack/middleware.go | 6 ++ stack/middleware_test.go | 4 +- stack/multiplexer.go | 4 +- stack/recovery.go | 11 ++++ stack/signature.go | 4 +- stack/signature_test.go | 2 +- 18 files changed, 169 insertions(+), 35 deletions(-) diff --git a/cmd/basecoin/commands/start.go b/cmd/basecoin/commands/start.go index 8f12efed35..aeb49a1c35 100644 --- a/cmd/basecoin/commands/start.go +++ b/cmd/basecoin/commands/start.go @@ -55,7 +55,7 @@ func init() { func getHandler() basecoin.Handler { // use the default stack h := coin.NewHandler() - app := stack.NewDefault("change-this").Use(h) + app := stack.NewDefault().Use(h) return app // register IBC plugn diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index a15ad3ecc5..73f5ffb112 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -7,27 +7,34 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/abci/types" + "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/app" + "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/types" "github.com/tendermint/go-wire" eyescli "github.com/tendermint/merkleeyes/client" "github.com/tendermint/tmlibs/log" ) +// TODO: actually handle the counter here... +func CounterHandler() basecoin.Handler { + // use the default stack + h := coin.NewHandler() + return stack.NewDefault().Use(h) +} + func TestCounterPlugin(t *testing.T) { assert := assert.New(t) // Basecoin initialization eyesCli := eyescli.NewLocalClient("", 0) chainID := "test_chain_id" - bcApp := app.NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) + bcApp := app.NewBasecoin(CounterHandler(), eyesCli, + log.TestingLogger().With("module", "app")) bcApp.SetOption("base/chain_id", chainID) // t.Log(bcApp.Info()) - // Add Counter plugin - counterPlugin := New() - bcApp.RegisterPlugin(counterPlugin) - // Account initialization test1PrivAcc := types.PrivAccountFromSecret("test1") @@ -44,7 +51,7 @@ func TestCounterPlugin(t *testing.T) { tx := &types.AppTx{ Gas: gas, Fee: fee, - Name: counterPlugin.Name(), + Name: "counter", Input: types.NewTxInput(test1Acc.PubKey, inputCoins, inputSequence), Data: wire.BinaryBytes(CounterTx{Valid: true, Fee: appFee}), } diff --git a/handler.go b/handler.go index 6a6d556191..b0089ed280 100644 --- a/handler.go +++ b/handler.go @@ -3,10 +3,24 @@ package basecoin import ( abci "github.com/tendermint/abci/types" "github.com/tendermint/go-wire/data" + "github.com/tendermint/tmlibs/log" "github.com/tendermint/basecoin/types" ) +// Handler is anything that processes a transaction +type Handler interface { + Checker + Deliver + SetOptioner + Named + // TODO: flesh these out as well + // SetOption(store types.KVStore, key, value string) (log string) + // InitChain(store types.KVStore, vals []*abci.Validator) + // BeginBlock(store types.KVStore, hash []byte, header *abci.Header) + // EndBlock(store types.KVStore, height uint64) abci.ResponseEndBlock +} + type Named interface { Name() string } @@ -33,16 +47,15 @@ func (c DeliverFunc) DeliverTx(ctx Context, store types.KVStore, tx Tx) (Result, return c(ctx, store, tx) } -// Handler is anything that processes a transaction -type Handler interface { - Checker - Deliver - Named - // TODO: flesh these out as well - // SetOption(store types.KVStore, key, value string) (log string) - // InitChain(store types.KVStore, vals []*abci.Validator) - // BeginBlock(store types.KVStore, hash []byte, header *abci.Header) - // EndBlock(store types.KVStore, height uint64) abci.ResponseEndBlock +type SetOptioner interface { + SetOption(l log.Logger, store types.KVStore, key, value string) (string, error) +} + +// SetOptionFunc (like http.HandlerFunc) is a shortcut for making wrapers +type SetOptionFunc func(log.Logger, types.KVStore, string, string) (string, error) + +func (c SetOptionFunc) SetOption(l log.Logger, store types.KVStore, key, value string) (string, error) { + return c(l, store, key, value) } // Result captures any non-error abci result @@ -58,3 +71,19 @@ func (r Result) ToABCI() abci.Result { Log: r.Log, } } + +// placeholders +// holders +type NopCheck struct{} + +func (_ NopCheck) CheckTx(Context, types.KVStore, Tx) (r Result, e error) { return } + +type NopDeliver struct{} + +func (_ NopDeliver) DeliverTx(Context, types.KVStore, Tx) (r Result, e error) { return } + +type NopOption struct{} + +func (_ NopOption) SetOption(log.Logger, types.KVStore, string, string) (string, error) { + return "", nil +} diff --git a/modules/coin/handler.go b/modules/coin/handler.go index 41eda35900..cabc6a062c 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -1,6 +1,8 @@ package coin import ( + "github.com/tendermint/tmlibs/log" + "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/errors" "github.com/tendermint/basecoin/types" @@ -74,6 +76,11 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoi return basecoin.Result{}, nil } +func (h Handler) SetOption(l log.Logger, store types.KVStore, key, value string) (log string, err error) { + // TODO + return "ok", nil +} + 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) diff --git a/modules/fee/handler.go b/modules/fee/handler.go index 1b4556ef4e..f711cac953 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -23,6 +23,7 @@ type AccountChecker interface { type SimpleFeeHandler struct { AccountChecker MinFee types.Coins + stack.PassOption } func (_ SimpleFeeHandler) Name() string { diff --git a/stack/chain.go b/stack/chain.go index 70da742f67..e07e6dd688 100644 --- a/stack/chain.go +++ b/stack/chain.go @@ -12,7 +12,9 @@ const ( ) // Chain enforces that this tx was bound to the named chain -type Chain struct{} +type Chain struct { + PassOption +} func (_ Chain) Name() string { return NameRecovery diff --git a/stack/chain_test.go b/stack/chain_test.go index c9afaa7001..cb1e39059e 100644 --- a/stack/chain_test.go +++ b/stack/chain_test.go @@ -34,7 +34,7 @@ func TestChain(t *testing.T) { store := types.NewMemKVStore() // build the stack - ok := OKHandler{msg} + ok := OKHandler{Log: msg} app := New(Chain{}).Use(ok) for idx, tc := range cases { diff --git a/stack/helpers.go b/stack/helpers.go index ec7247a6fe..387a2ae96c 100644 --- a/stack/helpers.go +++ b/stack/helpers.go @@ -19,6 +19,7 @@ const ( // OKHandler just used to return okay to everything type OKHandler struct { Log string + basecoin.NopOption } var _ basecoin.Handler = OKHandler{} @@ -38,7 +39,9 @@ func (ok OKHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx base } // EchoHandler returns success, echoing res.Data = tx bytes -type EchoHandler struct{} +type EchoHandler struct { + basecoin.NopOption +} var _ basecoin.Handler = EchoHandler{} @@ -61,6 +64,7 @@ func (_ EchoHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx bas // FailHandler always returns an error type FailHandler struct { Err error + basecoin.NopOption } var _ basecoin.Handler = FailHandler{} @@ -83,6 +87,7 @@ func (f FailHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx bas type PanicHandler struct { Msg string Err error + basecoin.NopOption } var _ basecoin.Handler = PanicHandler{} diff --git a/stack/helpers_test.go b/stack/helpers_test.go index 10e3655800..8787bb6a6e 100644 --- a/stack/helpers_test.go +++ b/stack/helpers_test.go @@ -20,7 +20,7 @@ func TestOK(t *testing.T) { data := "this looks okay" tx := basecoin.Tx{} - ok := OKHandler{data} + ok := OKHandler{Log: data} res, err := ok.CheckTx(ctx, store, tx) assert.Nil(err, "%+v", err) assert.Equal(data, res.Log) @@ -38,7 +38,7 @@ func TestFail(t *testing.T) { msg := "big problem" tx := basecoin.Tx{} - fail := FailHandler{errors.New(msg)} + fail := FailHandler{Err: errors.New(msg)} _, err := fail.CheckTx(ctx, store, tx) if assert.NotNil(err) { assert.Equal(msg, err.Error()) diff --git a/stack/helperware.go b/stack/helperware.go index ec2282ac84..25c93f4e51 100644 --- a/stack/helperware.go +++ b/stack/helperware.go @@ -15,6 +15,7 @@ const ( // Required Actor, otherwise passes along the call untouched type CheckMiddleware struct { Required basecoin.Actor + PassOption } var _ Middleware = CheckMiddleware{} @@ -40,6 +41,7 @@ func (p CheckMiddleware) DeliverTx(ctx basecoin.Context, store types.KVStore, tx // GrantMiddleware tries to set the permission to this Actor, which may be prohibited type GrantMiddleware struct { Auth basecoin.Actor + PassOption } var _ Middleware = GrantMiddleware{} diff --git a/stack/interface.go b/stack/interface.go index b57d6dbd04..951a7fa73f 100644 --- a/stack/interface.go +++ b/stack/interface.go @@ -1,18 +1,12 @@ package stack import ( + "github.com/tendermint/tmlibs/log" + "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/types" ) -type CheckerMiddle interface { - CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Checker) (basecoin.Result, error) -} - -type DeliverMiddle interface { - DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Deliver) (basecoin.Result, error) -} - // Middleware is anything that wraps another handler to enhance functionality. // // You can use utilities in handlers to construct them, the interfaces @@ -20,5 +14,55 @@ type DeliverMiddle interface { type Middleware interface { CheckerMiddle DeliverMiddle + SetOptionMiddle basecoin.Named } + +type CheckerMiddle interface { + CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Checker) (basecoin.Result, error) +} + +type CheckerMiddleFunc func(basecoin.Context, types.KVStore, basecoin.Tx, basecoin.Checker) (basecoin.Result, error) + +func (c CheckerMiddleFunc) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Checker) (basecoin.Result, error) { + return c(ctx, store, tx, next) +} + +type DeliverMiddle interface { + DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Deliver) (basecoin.Result, error) +} + +type DeliverMiddleFunc func(basecoin.Context, types.KVStore, basecoin.Tx, basecoin.Deliver) (basecoin.Result, error) + +func (d DeliverMiddleFunc) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Deliver) (basecoin.Result, error) { + return d(ctx, store, tx, next) +} + +type SetOptionMiddle interface { + SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (string, error) +} + +type SetOptionMiddleFunc func(log.Logger, types.KVStore, string, string, basecoin.SetOptioner) (string, error) + +func (c SetOptionMiddleFunc) SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (string, error) { + return c(l, store, key, value, next) +} + +// holders +type PassCheck struct{} + +func (_ PassCheck) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Checker) (basecoin.Result, error) { + return next.CheckTx(ctx, store, tx) +} + +type PassDeliver struct{} + +func (_ PassDeliver) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Deliver) (basecoin.Result, error) { + return next.DeliverTx(ctx, store, tx) +} + +type PassOption struct{} + +func (_ PassOption) SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (string, error) { + return next.SetOption(l, store, key, value) +} diff --git a/stack/logger.go b/stack/logger.go index 93cc8e5e50..2bd41a5fa2 100644 --- a/stack/logger.go +++ b/stack/logger.go @@ -3,6 +3,8 @@ package stack import ( "time" + "github.com/tendermint/tmlibs/log" + "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/types" ) @@ -48,6 +50,20 @@ func (_ Logger) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin return } +func (_ Logger) SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (string, error) { + start := time.Now() + res, err := next.SetOption(l, store, key, value) + delta := time.Now().Sub(start) + // TODO: log the value being set also? + l = l.With("duration", micros(delta)).With("key", key) + if err == nil { + l.Info("SetOption", "log", res) + } else { + l.Error("SetOption", "err", err) + } + return res, err +} + // micros returns how many microseconds passed in a call func micros(d time.Duration) int { return int(d.Seconds() * 1000000) diff --git a/stack/middleware.go b/stack/middleware.go index f61b233da2..fcc450b730 100644 --- a/stack/middleware.go +++ b/stack/middleware.go @@ -1,6 +1,8 @@ package stack import ( + "github.com/tendermint/tmlibs/log" + "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/types" ) @@ -37,6 +39,10 @@ func (m *middleware) DeliverTx(ctx basecoin.Context, store types.KVStore, tx bas return m.middleware.DeliverTx(ctx, store, tx, next) } +func (m *middleware) SetOption(l log.Logger, store types.KVStore, key, value string) (string, error) { + return m.middleware.SetOption(l, store, key, value, m.next) +} + // Stack is the entire application stack type Stack struct { middles []Middleware diff --git a/stack/middleware_test.go b/stack/middleware_test.go index adbb970932..4f88539191 100644 --- a/stack/middleware_test.go +++ b/stack/middleware_test.go @@ -42,8 +42,8 @@ func TestPermissionSandbox(t *testing.T) { for i, tc := range cases { app := New( Recovery{}, // we need this so panics turn to errors - GrantMiddleware{tc.grant}, - CheckMiddleware{tc.require}, + GrantMiddleware{Auth: tc.grant}, + CheckMiddleware{Required: tc.require}, ).Use(EchoHandler{}) res, err := app.CheckTx(ctx, store, raw) diff --git a/stack/multiplexer.go b/stack/multiplexer.go index b0cddfc98e..2000551cb2 100644 --- a/stack/multiplexer.go +++ b/stack/multiplexer.go @@ -14,7 +14,9 @@ const ( NameMultiplexer = "mplx" ) -type Multiplexer struct{} +type Multiplexer struct { + PassOption +} func (_ Multiplexer) Name() string { return NameMultiplexer diff --git a/stack/recovery.go b/stack/recovery.go index b6e84496c6..7dbb10a439 100644 --- a/stack/recovery.go +++ b/stack/recovery.go @@ -3,6 +3,8 @@ package stack import ( "fmt" + "github.com/tendermint/tmlibs/log" + "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/errors" "github.com/tendermint/basecoin/types" @@ -39,6 +41,15 @@ func (_ Recovery) DeliverTx(ctx basecoin.Context, store types.KVStore, tx baseco return next.DeliverTx(ctx, store, tx) } +func (_ Recovery) SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (log string, err error) { + defer func() { + if r := recover(); r != nil { + err = normalizePanic(r) + } + }() + return next.SetOption(l, store, key, value) +} + // normalizePanic makes sure we can get a nice TMError (with stack) out of it func normalizePanic(p interface{}) error { if err, isErr := p.(error); isErr { diff --git a/stack/signature.go b/stack/signature.go index 049351dd10..d996f921a7 100644 --- a/stack/signature.go +++ b/stack/signature.go @@ -13,7 +13,9 @@ const ( NameSigs = "sigs" ) -type Signatures struct{} +type Signatures struct { + PassOption +} func (_ Signatures) Name() string { return NameSigs diff --git a/stack/signature_test.go b/stack/signature_test.go index 1e6f484ab3..0dbf1a6173 100644 --- a/stack/signature_test.go +++ b/stack/signature_test.go @@ -58,7 +58,7 @@ func TestSignatureChecks(t *testing.T) { app := New( Recovery{}, // we need this so panics turn to errors Signatures{}, - CheckMiddleware{tc.check}, + CheckMiddleware{Required: tc.check}, ).Use(OKHandler{}) var tx basecoin.Tx From 7c4f408934d4b963be42ab0a40e3a884fb7809d1 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 18:58:28 +0200 Subject: [PATCH 05/24] Implement SetOption in coin module --- app/app.go | 44 +++++++--------------------- app/genesis.go | 56 +++-------------------------------- modules/coin/genesis.go | 48 ++++++++++++++++++++++++++++++ modules/coin/handler.go | 27 +++++++++++++++-- modules/coin/handler_test.go | 57 ++++++++++++++++++++++++++++++++++++ 5 files changed, 144 insertions(+), 88 deletions(-) create mode 100644 modules/coin/genesis.go diff --git a/app/app.go b/app/app.go index 940520285a..b29186a24a 100644 --- a/app/app.go +++ b/app/app.go @@ -18,6 +18,7 @@ import ( const ( PluginNameBase = "base" + ChainKey = "base/chain_id" ) type Basecoin struct { @@ -67,41 +68,16 @@ func (app *Basecoin) Info() abci.ResponseInfo { // ABCI::SetOption func (app *Basecoin) SetOption(key string, value string) string { - // TODO - return "todo" - // pluginName, key := splitKey(key) - // if pluginName != PluginNameBase { - // // Set option on plugin - // plugin := app.plugins.GetByName(pluginName) - // if plugin == nil { - // return "Invalid plugin name: " + pluginName - // } - // app.logger.Info("SetOption on plugin", "plugin", pluginName, "key", key, "value", value) - // return plugin.SetOption(app.state, key, value) - // } else { - // // Set option on basecoin - // switch key { - // case "chain_id": - // app.state.SetChainID(value) - // return "Success" - // case "account": - // var acc GenesisAccount - // err := json.Unmarshal([]byte(value), &acc) - // if err != nil { - // return "Error decoding acc message: " + err.Error() - // } - // acc.Balance.Sort() - // addr, err := acc.GetAddr() - // if err != nil { - // return "Invalid address: " + err.Error() - // } - // app.state.SetAccount(addr, acc.ToAccount()) - // app.logger.Info("SetAccount", "addr", hex.EncodeToString(addr), "acc", acc) + if key == ChainKey { + app.state.SetChainID(value) + return "Success" + } - // return "Success" - // } - // return "Unrecognized option key " + key - // } + log, err := app.handler.SetOption(app.logger, app.state, key, value) + if err == nil { + return log + } + return "Error: " + err.Error() } // ABCI::DeliverTx diff --git a/app/genesis.go b/app/genesis.go index 3901fa6898..05913c15b0 100644 --- a/app/genesis.go +++ b/app/genesis.go @@ -1,14 +1,10 @@ package app import ( - "bytes" "encoding/json" "github.com/pkg/errors" - "github.com/tendermint/basecoin/types" - crypto "github.com/tendermint/go-crypto" - "github.com/tendermint/go-wire/data" cmn "github.com/tendermint/tmlibs/common" ) @@ -22,20 +18,13 @@ func (app *Basecoin) LoadGenesis(path string) error { app.SetOption("base/chain_id", genDoc.ChainID) // set accounts - for _, acc := range genDoc.AppOptions.Accounts { - accBytes, err := json.Marshal(acc) - if err != nil { - return err - } - r := app.SetOption("base/account", string(accBytes)) - // TODO: SetOption returns an error - app.logger.Info("Done setting Account via SetOption", "result", r) + for _, acct := range genDoc.AppOptions.Accounts { + _ = app.SetOption("base/account", string(acct)) } // set plugin options for _, kv := range genDoc.AppOptions.pluginOptions { - r := app.SetOption(kv.Key, kv.Value) - app.logger.Info("Done setting Plugin key-value pair via SetOption", "result", r, "k", kv.Key, "v", kv.Value) + _ = app.SetOption(kv.Key, kv.Value) } return nil @@ -53,7 +42,7 @@ type FullGenesisDoc struct { } type GenesisDoc struct { - Accounts []GenesisAccount `json:"accounts"` + Accounts []json.RawMessage `json:"accounts"` PluginOptions []json.RawMessage `json:"plugin_options"` pluginOptions []keyValue // unmarshaled rawmessages @@ -106,40 +95,3 @@ func parseGenesisList(kvz_ []json.RawMessage) (kvz []keyValue, err error) { } return kvz, nil } - -/**** code to parse accounts from genesis docs ***/ - -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 types.Coins `json:"coins"` -} - -func (g GenesisAccount) ToAccount() *types.Account { - return &types.Account{ - PubKey: g.PubKey, - Sequence: g.Sequence, - Balance: g.Balance, - } -} - -func (g GenesisAccount) GetAddr() ([]byte, error) { - noAddr, noPk := len(g.Address) == 0, g.PubKey.Empty() - - if noAddr { - if noPk { - return nil, errors.New("No address given") - } - return g.PubKey.Address(), nil - } - if noPk { // but is addr... - return g.Address, nil - } - // now, we have both, make sure they check out - if bytes.Equal(g.Address, g.PubKey.Address()) { - return g.Address, nil - } - return nil, errors.New("Address and pubkey don't match") -} diff --git a/modules/coin/genesis.go b/modules/coin/genesis.go new file mode 100644 index 0000000000..7ce32cd8d2 --- /dev/null +++ b/modules/coin/genesis.go @@ -0,0 +1,48 @@ +package coin + +import ( + "bytes" + + "github.com/pkg/errors" + + crypto "github.com/tendermint/go-crypto" + "github.com/tendermint/go-wire/data" + + "github.com/tendermint/basecoin/types" +) + +/**** code to parse accounts from genesis docs ***/ + +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 types.Coins `json:"coins"` +} + +func (g GenesisAccount) ToAccount() Account { + return Account{ + Sequence: g.Sequence, + Coins: g.Balance, + } +} + +func (g GenesisAccount) GetAddr() ([]byte, error) { + noAddr, noPk := len(g.Address) == 0, g.PubKey.Empty() + + if noAddr { + if noPk { + return nil, errors.New("No address given") + } + return g.PubKey.Address(), nil + } + if noPk { // but is addr... + return g.Address, nil + } + // now, we have both, make sure they check out + if bytes.Equal(g.Address, g.PubKey.Address()) { + return g.Address, nil + } + return nil, errors.New("Address and pubkey don't match") +} diff --git a/modules/coin/handler.go b/modules/coin/handler.go index cabc6a062c..a8919d835f 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -1,6 +1,9 @@ package coin import ( + "fmt" + + "github.com/tendermint/go-wire/data" "github.com/tendermint/tmlibs/log" "github.com/tendermint/basecoin" @@ -77,8 +80,28 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoi } func (h Handler) SetOption(l log.Logger, store types.KVStore, key, value string) (log string, err error) { - // TODO - return "ok", nil + if key == "base/account" { + var acc GenesisAccount + err = data.FromJSON([]byte(value), &acc) + if err != nil { + return "", err + } + acc.Balance.Sort() + addr, err := acc.GetAddr() + if err != nil { + return "", ErrInvalidAddress() + } + actor := basecoin.Actor{App: NameCoin, Address: addr} + err = storeAccount(store, h.makeKey(actor), acc.ToAccount()) + if err != nil { + return "", err + } + return "Success", nil + + } else { + msg := fmt.Sprintf("Unknown key: %s", key) + return "", errors.ErrInternal(msg) + } } func checkTx(ctx basecoin.Context, tx basecoin.Tx) (send SendTx, err error) { diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index 8027b489c6..7f1e0b12b5 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -1,10 +1,15 @@ package coin import ( + "encoding/json" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + crypto "github.com/tendermint/go-crypto" + "github.com/tendermint/tmlibs/log" + "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/types" @@ -158,5 +163,57 @@ func TestDeliverTx(t *testing.T) { } } +} + +func TestSetOption(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + // some sample settings + pk := crypto.GenPrivKeySecp256k1().Wrap() + addr := pk.PubKey().Address() + actor := basecoin.Actor{App: "coin", Address: addr} + // actor2 := basecoin.Actor{App: "foo", Address: addr} + + someCoins := types.Coins{{"atom", 123}} + otherCoins := types.Coins{{"eth", 11}} + mixedCoins := someCoins.Plus(otherCoins) + + type money struct { + addr basecoin.Actor + coins types.Coins + } + + cases := []struct { + init []GenesisAccount + expected []money + }{ + { + []GenesisAccount{{Address: addr, Balance: mixedCoins}}, + []money{{actor, mixedCoins}}, + }, + } + + h := NewHandler() + l := log.NewNopLogger() + for i, tc := range cases { + store := types.NewMemKVStore() + key := "base/account" + + // set the options + for j, gen := range tc.init { + value, err := json.Marshal(gen) + require.Nil(err, "%d,%d: %+v", i, j, err) + _, err = h.SetOption(l, store, key, string(value)) + require.Nil(err) + } + + // check state is proper + for _, f := range tc.expected { + acct, err := loadAccount(store, h.makeKey(f.addr)) + assert.Nil(err, "%d: %+v", i, err) + assert.Equal(f.coins, acct.Coins) + } + } } From fc44de2141001a148b0c2fd47d68a590ac1ec709 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 21:34:08 +0200 Subject: [PATCH 06/24] Fix up BasecoinApp and tests --- app/app.go | 26 +++------ app/app_test.go | 109 ++++++++++++++++++++--------------- handler.go | 1 - modules/coin/handler.go | 10 ++-- modules/coin/handler_test.go | 6 +- modules/coin/store.go | 17 ++++-- 6 files changed, 94 insertions(+), 75 deletions(-) diff --git a/app/app.go b/app/app.go index b29186a24a..69052a0ee3 100644 --- a/app/app.go +++ b/app/app.go @@ -1,8 +1,6 @@ package app import ( - "strings" - abci "github.com/tendermint/abci/types" "github.com/tendermint/basecoin" eyes "github.com/tendermint/merkleeyes/client" @@ -89,8 +87,10 @@ func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { // TODO: can we abstract this setup and commit logic?? cache := app.state.CacheWrap() - ctx := stack.NewContext(app.state.GetChainID(), - app.logger.With("call", "delivertx")) + ctx := stack.NewContext( + app.state.GetChainID(), + app.logger.With("call", "delivertx"), + ) res, err := app.handler.DeliverTx(ctx, cache, tx) if err != nil { @@ -110,8 +110,10 @@ func (app *Basecoin) CheckTx(txBytes []byte) abci.Result { } // TODO: can we abstract this setup and commit logic?? - ctx := stack.NewContext(app.state.GetChainID(), - app.logger.With("call", "checktx")) + ctx := stack.NewContext( + app.state.GetChainID(), + app.logger.With("call", "checktx"), + ) // checktx generally shouldn't touch the state, but we don't care // here on the framework level, since the cacheState is thrown away next block res, err := app.handler.CheckTx(ctx, app.cacheState, tx) @@ -176,15 +178,3 @@ func (app *Basecoin) EndBlock(height uint64) (res abci.ResponseEndBlock) { // } return } - -//---------------------------------------- - -// Splits the string at the first '/'. -// if there are none, the second string is nil. -func splitKey(key string) (prefix string, suffix string) { - if strings.Contains(key, "/") { - keyParts := strings.SplitN(key, "/", 2) - return keyParts[0], keyParts[1] - } - return key, "" -} diff --git a/app/app_test.go b/app/app_test.go index 1645853206..ab007ce374 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -9,7 +9,12 @@ import ( "github.com/stretchr/testify/require" abci "github.com/tendermint/abci/types" + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" + "github.com/tendermint/basecoin/txs" "github.com/tendermint/basecoin/types" + crypto "github.com/tendermint/go-crypto" wire "github.com/tendermint/go-wire" eyes "github.com/tendermint/merkleeyes/client" "github.com/tendermint/tmlibs/log" @@ -36,10 +41,17 @@ func newAppTest(t *testing.T) *appTest { } // make a tx sending 5mycoin from each accIn to accOut -func (at *appTest) getTx(seq int) *types.SendTx { - tx := types.MakeSendTx(seq, at.accOut, at.accIn) - types.SignTx(at.chainID, tx, at.accIn) - return tx +func (at *appTest) getTx(seq int, coins types.Coins) basecoin.Tx { + addrIn := at.accIn.Account.PubKey.Address() + addrOut := at.accOut.Account.PubKey.Address() + + in := []coin.TxInput{{Address: stack.SigPerm(addrIn), Coins: coins, Sequence: seq}} + out := []coin.TxOutput{{Address: stack.SigPerm(addrOut), Coins: coins}} + tx := coin.NewSendTx(in, out) + tx = txs.NewChain(at.chainID, tx) + stx := txs.NewMulti(tx) + txs.Sign(stx, at.accIn.PrivKey) + return stx.Wrap() } // set the account on the app through SetOption @@ -69,45 +81,51 @@ func (at *appTest) reset() { require.True(at.t, resabci.IsOK(), resabci) } +func getBalance(pk crypto.PubKey, state types.KVStore) (types.Coins, error) { + return getAddr(pk.Address(), state) +} + +func getAddr(addr []byte, state types.KVStore) (types.Coins, error) { + actor := stack.SigPerm(addr) + acct, err := coin.NewAccountant("").GetAccount(state, actor) + return acct.Coins, err +} + // returns the final balance and expected balance for input and output accounts -func (at *appTest) exec(tx *types.SendTx, checkTx bool) (res abci.Result, inputGot, inputExp, outputGot, outputExpected types.Coins) { +func (at *appTest) exec(t *testing.T, tx basecoin.Tx, checkTx bool) (res abci.Result, diffIn, diffOut types.Coins) { + require := require.New(t) - initBalIn := at.app.GetState().GetAccount(at.accIn.Account.PubKey.Address()).Balance - initBalOut := at.app.GetState().GetAccount(at.accOut.Account.PubKey.Address()).Balance + initBalIn, err := getBalance(at.accIn.Account.PubKey, at.app.GetState()) + require.Nil(err, "%+v", err) + initBalOut, err := getBalance(at.accOut.Account.PubKey, at.app.GetState()) + require.Nil(err, "%+v", err) - txBytes := []byte(wire.BinaryBytes(struct{ types.Tx }{tx})) + txBytes := wire.BinaryBytes(tx) if checkTx { res = at.app.CheckTx(txBytes) } else { res = at.app.DeliverTx(txBytes) } - endBalIn := at.app.GetState().GetAccount(at.accIn.Account.PubKey.Address()).Balance - endBalOut := at.app.GetState().GetAccount(at.accOut.Account.PubKey.Address()).Balance - decrBalInExp := tx.Outputs[0].Coins.Plus(types.Coins{tx.Fee}) - return res, endBalIn, initBalIn.Minus(decrBalInExp), endBalOut, initBalOut.Plus(tx.Outputs[0].Coins) + endBalIn, err := getBalance(at.accIn.Account.PubKey, at.app.GetState()) + require.Nil(err, "%+v", err) + endBalOut, err := getBalance(at.accOut.Account.PubKey, at.app.GetState()) + require.Nil(err, "%+v", err) + return res, endBalIn.Minus(initBalIn), endBalOut.Minus(initBalOut) } //-------------------------------------------------------- -func TestSplitKey(t *testing.T) { - assert := assert.New(t) - prefix, suffix := splitKey("foo/bar") - assert.EqualValues("foo", prefix) - assert.EqualValues("bar", suffix) - - prefix, suffix = splitKey("foobar") - assert.EqualValues("foobar", prefix) - assert.EqualValues("", suffix) -} - func TestSetOption(t *testing.T) { assert := assert.New(t) require := require.New(t) eyesCli := eyes.NewLocalClient("", 0) - app := NewBasecoin(DefaultHandler(), eyesCli, - log.TestingLogger().With("module", "app")) + app := NewBasecoin( + DefaultHandler(), + eyesCli, + log.TestingLogger().With("module", "app"), + ) //testing ChainID chainID := "testChain" @@ -116,15 +134,16 @@ func TestSetOption(t *testing.T) { assert.EqualValues(res, "Success") // make a nice account... - accIn := types.MakeAcc("input0") - accsInBytes, err := json.Marshal(accIn.Account) + accIn := types.MakeAcc("input0").Account + accsInBytes, err := json.Marshal(accIn) assert.Nil(err) res = app.SetOption("base/account", string(accsInBytes)) require.EqualValues(res, "Success") + // make sure it is set correctly, with some balance - acct := types.GetAccount(app.GetState(), accIn.PubKey.Address()) - require.NotNil(acct) - assert.Equal(accIn.Balance, acct.Balance) + coins, err := getBalance(accIn.PubKey, app.state) + require.Nil(err) + assert.Equal(accIn.Balance, coins) // let's parse an account with badly sorted coins... unsortAddr, err := hex.DecodeString("C471FB670E44D219EE6DF2FC284BE38793ACBCE1") @@ -148,10 +167,11 @@ func TestSetOption(t *testing.T) { }` res = app.SetOption("base/account", unsortAcc) require.EqualValues(res, "Success") - acct = types.GetAccount(app.GetState(), unsortAddr) - require.NotNil(acct) - assert.True(acct.Balance.IsValid()) - assert.Equal(unsortCoins, acct.Balance) + + coins, err = getAddr(unsortAddr, app.state) + require.Nil(err) + assert.True(coins.IsValid()) + assert.Equal(unsortCoins, coins) res = app.SetOption("base/dslfkgjdas", "") assert.NotEqual(res, "Success") @@ -172,33 +192,32 @@ func TestTx(t *testing.T) { //Bad Balance at.accIn.Balance = types.Coins{{"mycoin", 2}} at.acc2app(at.accIn.Account) - res, _, _, _, _ := at.exec(at.getTx(1), true) + res, _, _ := at.exec(t, at.getTx(1, types.Coins{{"mycoin", 5}}), true) assert.True(res.IsErr(), "ExecTx/Bad CheckTx: Expected error return from ExecTx, returned: %v", res) - res, inGot, inExp, outGot, outExp := at.exec(at.getTx(1), false) + res, diffIn, diffOut := at.exec(t, at.getTx(1, types.Coins{{"mycoin", 5}}), false) assert.True(res.IsErr(), "ExecTx/Bad DeliverTx: Expected error return from ExecTx, returned: %v", res) - assert.False(inGot.IsEqual(inExp), "ExecTx/Bad DeliverTx: shouldn't be equal, inGot: %v, inExp: %v", inGot, inExp) - assert.False(outGot.IsEqual(outExp), "ExecTx/Bad DeliverTx: shouldn't be equal, outGot: %v, outExp: %v", outGot, outExp) + assert.True(diffIn.IsZero()) + assert.True(diffOut.IsZero()) //Regular CheckTx at.reset() - res, _, _, _, _ = at.exec(at.getTx(1), true) + res, _, _ = at.exec(t, at.getTx(1, types.Coins{{"mycoin", 5}}), true) assert.True(res.IsOK(), "ExecTx/Good CheckTx: Expected OK return from ExecTx, Error: %v", res) //Regular DeliverTx at.reset() - res, inGot, inExp, outGot, outExp = at.exec(at.getTx(1), false) + amt := types.Coins{{"mycoin", 3}} + res, diffIn, diffOut = at.exec(t, at.getTx(1, amt), false) assert.True(res.IsOK(), "ExecTx/Good DeliverTx: Expected OK return from ExecTx, Error: %v", res) - assert.True(inGot.IsEqual(inExp), - "ExecTx/good DeliverTx: unexpected change in input coins, inGot: %v, inExp: %v", inGot, inExp) - assert.True(outGot.IsEqual(outExp), - "ExecTx/good DeliverTx: unexpected change in output coins, outGot: %v, outExp: %v", outGot, outExp) + assert.Equal(amt.Negative(), diffIn) + assert.Equal(amt, diffOut) } func TestQuery(t *testing.T) { assert := assert.New(t) at := newAppTest(t) - res, _, _, _, _ := at.exec(at.getTx(1), false) + res, _, _ := at.exec(t, at.getTx(1, types.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/handler.go b/handler.go index b0089ed280..3e3728a09b 100644 --- a/handler.go +++ b/handler.go @@ -15,7 +15,6 @@ type Handler interface { SetOptioner Named // TODO: flesh these out as well - // SetOption(store types.KVStore, key, value string) (log string) // InitChain(store types.KVStore, vals []*abci.Validator) // BeginBlock(store types.KVStore, hash []byte, header *abci.Header) // EndBlock(store types.KVStore, height uint64) abci.ResponseEndBlock diff --git a/modules/coin/handler.go b/modules/coin/handler.go index a8919d835f..63cf3d693b 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -8,6 +8,7 @@ import ( "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/types" ) @@ -24,7 +25,7 @@ var _ basecoin.Handler = Handler{} func NewHandler() Handler { return Handler{ - Accountant: Accountant{Prefix: []byte(NameCoin + "/")}, + Accountant: NewAccountant(""), } } @@ -41,7 +42,7 @@ func (h Handler) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin. // now make sure there is money for _, in := range send.Inputs { - _, err = h.CheckCoins(store, in.Address, in.Coins, in.Sequence) + _, err = h.CheckCoins(store, in.Address, in.Coins.Negative(), in.Sequence) if err != nil { return res, err } @@ -91,8 +92,9 @@ func (h Handler) SetOption(l log.Logger, store types.KVStore, key, value string) if err != nil { return "", ErrInvalidAddress() } - actor := basecoin.Actor{App: NameCoin, Address: addr} - err = storeAccount(store, h.makeKey(actor), acc.ToAccount()) + // this sets the permission for a public key signature, use that app + actor := stack.SigPerm(addr) + err = storeAccount(store, h.MakeKey(actor), acc.ToAccount()) if err != nil { return "", err } diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index 7f1e0b12b5..534b5ee168 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -143,7 +143,7 @@ func TestDeliverTx(t *testing.T) { store := types.NewMemKVStore() for _, m := range tc.init { acct := Account{Coins: m.coins} - err := storeAccount(store, h.makeKey(m.addr), acct) + err := storeAccount(store, h.MakeKey(m.addr), acct) require.Nil(err, "%d: %+v", i, err) } @@ -153,7 +153,7 @@ func TestDeliverTx(t *testing.T) { assert.Nil(err, "%d: %+v", i, err) // make sure the final balances are correct for _, f := range tc.final { - acct, err := loadAccount(store, h.makeKey(f.addr)) + acct, err := loadAccount(store, h.MakeKey(f.addr)) assert.Nil(err, "%d: %+v", i, err) assert.Equal(f.coins, acct.Coins) } @@ -210,7 +210,7 @@ func TestSetOption(t *testing.T) { // check state is proper for _, f := range tc.expected { - acct, err := loadAccount(store, h.makeKey(f.addr)) + acct, err := loadAccount(store, h.MakeKey(f.addr)) assert.Nil(err, "%d: %+v", i, err) assert.Equal(f.coins, acct.Coins) } diff --git a/modules/coin/store.go b/modules/coin/store.go index 15a280131d..46a1c93cd0 100644 --- a/modules/coin/store.go +++ b/modules/coin/store.go @@ -14,8 +14,17 @@ type Accountant struct { Prefix []byte } +func NewAccountant(prefix string) Accountant { + if prefix == "" { + prefix = NameCoin + } + return Accountant{ + Prefix: []byte(prefix + "/"), + } +} + func (a Accountant) GetAccount(store types.KVStore, addr basecoin.Actor) (Account, error) { - acct, err := loadAccount(store, a.makeKey(addr)) + acct, err := loadAccount(store, a.MakeKey(addr)) // for empty accounts, don't return an error, but rather an empty account if IsNoAccountErr(err) { err = nil @@ -36,7 +45,7 @@ func (a Accountant) ChangeCoins(store types.KVStore, addr basecoin.Actor, coins return acct.Coins, err } - err = storeAccount(store, a.makeKey(addr), acct) + err = storeAccount(store, a.MakeKey(addr), acct) return acct.Coins, err } @@ -44,7 +53,7 @@ func (a Accountant) ChangeCoins(store types.KVStore, addr basecoin.Actor, coins // // it doesn't save anything, that is up to you to decide (Check/Change Coins) func (a Accountant) updateCoins(store types.KVStore, addr basecoin.Actor, coins types.Coins, seq int) (acct Account, err error) { - acct, err = loadAccount(store, a.makeKey(addr)) + acct, err = loadAccount(store, a.MakeKey(addr)) // we can increase an empty account... if IsNoAccountErr(err) && coins.IsPositive() { err = nil @@ -71,7 +80,7 @@ func (a Accountant) updateCoins(store types.KVStore, addr basecoin.Actor, coins return acct, nil } -func (a Accountant) makeKey(addr basecoin.Actor) []byte { +func (a Accountant) MakeKey(addr basecoin.Actor) []byte { key := addr.Bytes() if len(a.Prefix) > 0 { key = append(a.Prefix, key...) From 36be40f3af864fd2dabff5ba7bb1a91b55b9c37d Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 21:40:10 +0200 Subject: [PATCH 07/24] Fix genesis tests --- app/genesis_test.go | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/app/genesis_test.go b/app/genesis_test.go index 2575f3fa92..74947f779b 100644 --- a/app/genesis_test.go +++ b/app/genesis_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/basecoin/types" - crypto "github.com/tendermint/go-crypto" eyescli "github.com/tendermint/merkleeyes/client" cmn "github.com/tendermint/tmlibs/common" "github.com/tendermint/tmlibs/log" @@ -37,27 +36,19 @@ func TestLoadGenesis(t *testing.T) { // and check the account info - previously calculated values addr, _ := hex.DecodeString("eb98e0688217cfdeb70eddf4b33cdcc37fc53197") - pkbyte, _ := hex.DecodeString("6880db93598e283a67c4d88fc67a8858aa2de70f713fe94a5109e29c137100c2") - acct := app.GetState().GetAccount(addr) - require.NotNil(acct) + coins, err := getAddr(addr, app.state) + require.Nil(err) + assert.True(coins.IsPositive()) // make sure balance is proper - assert.Equal(2, len(acct.Balance)) - assert.True(acct.Balance.IsValid()) + assert.Equal(2, len(coins)) + assert.True(coins.IsValid()) // note, that we now sort them to be valid - assert.EqualValues(654321, acct.Balance[0].Amount) - assert.EqualValues("ETH", acct.Balance[0].Denom) - assert.EqualValues(12345, acct.Balance[1].Amount) - assert.EqualValues("blank", acct.Balance[1].Denom) - - // and public key is parsed properly - apk := acct.PubKey.Unwrap() - require.NotNil(apk) - epk, ok := apk.(crypto.PubKeyEd25519) - if assert.True(ok) { - assert.EqualValues(pkbyte, epk[:]) - } + assert.EqualValues(654321, coins[0].Amount) + assert.EqualValues("ETH", coins[0].Denom) + assert.EqualValues(12345, coins[1].Amount) + assert.EqualValues("blank", coins[1].Denom) } // Fix for issue #89, change the parse format for accounts in genesis.json @@ -90,17 +81,17 @@ func TestLoadGenesisAccountAddress(t *testing.T) { {"979F080B1DD046C452C2A8A250D18646C6B669D4", true, true, types.Coins{{"four", 444}}}, } - for _, tc := range cases { + for i, tc := range cases { addr, err := hex.DecodeString(tc.addr) require.Nil(err, tc.addr) - acct := app.GetState().GetAccount(addr) + coins, err := getAddr(addr, app.state) + require.Nil(err, "%+v", err) if !tc.exists { - assert.Nil(acct, tc.addr) - } else if assert.NotNil(acct, tc.addr) { + assert.True(coins.IsZero(), "%d", i) + } else if assert.False(coins.IsZero(), "%d", i) { // it should and does exist... - assert.True(acct.Balance.IsValid()) - assert.Equal(tc.coins, acct.Balance) - assert.Equal(!tc.hasPubkey, acct.PubKey.Empty(), tc.addr) + assert.True(coins.IsValid()) + assert.Equal(tc.coins, coins) } } } From af132fbab818a5ffa3f7e021401b65faa663c899 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 21:53:58 +0200 Subject: [PATCH 08/24] Fixed basecli query for 0.7 --- cmd/basecli/commands/query.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/basecli/commands/query.go b/cmd/basecli/commands/query.go index 250078032d..eb979d397a 100644 --- a/cmd/basecli/commands/query.go +++ b/cmd/basecli/commands/query.go @@ -10,6 +10,8 @@ import ( proofcmd "github.com/tendermint/light-client/commands/proofs" "github.com/tendermint/light-client/proofs" + "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" btypes "github.com/tendermint/basecoin/types" ) @@ -24,9 +26,9 @@ func doAccountQuery(cmd *cobra.Command, args []string) error { if err != nil { return err } - key := btypes.AccountKey(addr) + key := coin.NewAccountant("").MakeKey(stack.SigPerm(addr)) - acc := new(btypes.Account) + acc := coin.Account{} proof, err := proofcmd.GetAndParseAppProof(key, &acc) if lc.IsNoDataErr(err) { return errors.Errorf("Account bytes are empty for address %X ", addr) From 413ea2e23f8ad14b2ea693007ec9370fbfe1e53c Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 22:30:12 +0200 Subject: [PATCH 09/24] basecli works for sendtx and cli tests --- cmd/basecli/commands/cmds.go | 73 ++++++++++++++++++++--------------- cmd/basecli/commands/query.go | 4 +- context.go | 6 +-- modules/coin/store.go | 2 +- tests/cli/common.sh | 10 +++-- txs/sigs.go | 13 +------ 6 files changed, 56 insertions(+), 52 deletions(-) diff --git a/cmd/basecli/commands/cmds.go b/cmd/basecli/commands/cmds.go index d0e10e8aae..f8cbd77059 100644 --- a/cmd/basecli/commands/cmds.go +++ b/cmd/basecli/commands/cmds.go @@ -9,11 +9,15 @@ import ( flag "github.com/spf13/pflag" "github.com/spf13/viper" + "github.com/tendermint/basecoin" "github.com/tendermint/light-client/commands" txcmd "github.com/tendermint/light-client/commands/txs" ctypes "github.com/tendermint/tendermint/rpc/core/types" cmn "github.com/tendermint/tmlibs/common" + "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" + "github.com/tendermint/basecoin/txs" btypes "github.com/tendermint/basecoin/types" ) @@ -48,27 +52,25 @@ func init() { // runDemo is an example of how to make a tx func doSendTx(cmd *cobra.Command, args []string) error { // load data from json or flags - tx := new(btypes.SendTx) - found, err := txcmd.LoadJSON(tx) + var tx basecoin.Tx + found, err := txcmd.LoadJSON(&tx) if err != nil { return err } if !found { - err = readSendTxFlags(tx) + tx, err = readSendTxFlags() } if err != nil { return err } - // Wrap and add signer - send := &SendTx{ - chainID: commands.GetChainID(), - Tx: tx, - } - send.AddSigner(txcmd.GetSigner()) + // TODO: make this more flexible for middleware + // add the chain info + tx = txs.NewChain(commands.GetChainID(), tx) + stx := txs.NewSig(tx) // Sign if needed and post. This it the work-horse - bres, err := txcmd.SignAndPostTx(send) + bres, err := txcmd.SignAndPostTx(stx) if err != nil { return err } @@ -77,40 +79,50 @@ func doSendTx(cmd *cobra.Command, args []string) error { return txcmd.OutputTx(bres) } -func readSendTxFlags(tx *btypes.SendTx) error { +func readSendTxFlags() (tx basecoin.Tx, err error) { // parse to address - to, err := parseChainAddress(viper.GetString(FlagTo)) + chain, to, err := parseChainAddress(viper.GetString(FlagTo)) if err != nil { - return err + return tx, err } + toAddr := stack.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) - //parse the fee and amounts into coin types - tx.Fee, err = btypes.ParseCoin(viper.GetString(FlagFee)) - if err != nil { - return err - } amountCoins, err := btypes.ParseCoins(viper.GetString(FlagAmount)) if err != nil { - return err + return tx, err } - // set the gas - tx.Gas = viper.GetInt64(FlagGas) + // this could be much cooler with multisig... + var fromAddr basecoin.Actor + signer := txcmd.GetSigner() + if !signer.Empty() { + fromAddr = stack.SigPerm(signer.Address()) + } // craft the inputs and outputs - tx.Inputs = []btypes.TxInput{{ + ins := []coin.TxInput{{ + Address: fromAddr, Coins: amountCoins, Sequence: viper.GetInt(FlagSequence), }} - tx.Outputs = []btypes.TxOutput{{ - Address: to, + outs := []coin.TxOutput{{ + Address: toAddr, Coins: amountCoins, }} - return nil + return coin.NewSendTx(ins, outs), nil } -func parseChainAddress(toFlag string) ([]byte, error) { +func parseChainAddress(toFlag string) (string, []byte, error) { var toHex string var chainPrefix string spl := strings.Split(toFlag, "/") @@ -121,19 +133,16 @@ func parseChainAddress(toFlag string) ([]byte, error) { chainPrefix = spl[0] toHex = spl[1] default: - return nil, errors.Errorf("To address has too many slashes") + return "", nil, errors.Errorf("To address has too many slashes") } // convert destination address to bytes to, err := hex.DecodeString(cmn.StripHex(toHex)) if err != nil { - return nil, errors.Errorf("To address is invalid hex: %v\n", err) + return "", nil, errors.Errorf("To address is invalid hex: %v\n", err) } - if chainPrefix != "" { - to = []byte(chainPrefix + "/" + string(to)) - } - return to, nil + return chainPrefix, to, nil } //------------------------- diff --git a/cmd/basecli/commands/query.go b/cmd/basecli/commands/query.go index eb979d397a..7f3b8db56b 100644 --- a/cmd/basecli/commands/query.go +++ b/cmd/basecli/commands/query.go @@ -4,6 +4,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/tendermint/basecoin" wire "github.com/tendermint/go-wire" lc "github.com/tendermint/light-client" lcmd "github.com/tendermint/light-client/commands" @@ -12,7 +13,6 @@ import ( "github.com/tendermint/basecoin/modules/coin" "github.com/tendermint/basecoin/stack" - btypes "github.com/tendermint/basecoin/types" ) var AccountQueryCmd = &cobra.Command{ @@ -45,7 +45,7 @@ type BaseTxPresenter struct { } func (_ BaseTxPresenter) ParseData(raw []byte) (interface{}, error) { - var tx btypes.TxS + var tx basecoin.Tx err := wire.ReadBinaryBytes(raw, &tx) return tx, err } diff --git a/context.go b/context.go index 8532cc6c9a..dbbe7b6d92 100644 --- a/context.go +++ b/context.go @@ -12,9 +12,9 @@ import ( // It doesn't just have to be a pubkey on this chain, it could stem from // another app (like multi-sig account), or even another chain (via IBC) type Actor struct { - ChainID string // this is empty unless it comes from a different chain - App string // the app that the actor belongs to - Address data.Bytes // arbitrary app-specific unique id + ChainID string `json:"chain"` // this is empty unless it comes from a different chain + App string `json:"app"` // the app that the actor belongs to + Address data.Bytes `json:"addr"` // arbitrary app-specific unique id } func NewActor(app string, addr []byte) Actor { diff --git a/modules/coin/store.go b/modules/coin/store.go index 46a1c93cd0..6170f780a6 100644 --- a/modules/coin/store.go +++ b/modules/coin/store.go @@ -90,7 +90,7 @@ func (a Accountant) MakeKey(addr basecoin.Actor) []byte { type Account struct { Coins types.Coins `json:"coins"` - Sequence int `json:"seq"` + Sequence int `json:"sequence"` } func loadAccount(store types.KVStore, key []byte) (acct Account, err error) { diff --git a/tests/cli/common.sh b/tests/cli/common.sh index b4c553ff55..7c3b886ea3 100644 --- a/tests/cli/common.sh +++ b/tests/cli/common.sh @@ -159,9 +159,13 @@ checkSendTx() { if [ -n "$DEBUG" ]; then echo $TX; echo; fi assertEquals "proper height" $2 $(echo $TX | jq .height) - assertEquals "type=send" '"send"' $(echo $TX | jq .data.type) - assertEquals "proper sender" "\"$3\"" $(echo $TX | jq .data.data.inputs[0].address) - assertEquals "proper out amount" "$4" $(echo $TX | jq .data.data.outputs[0].coins[0].amount) + assertEquals "type=sig" '"sig"' $(echo $TX | jq .data.type) + CTX=$(echo $TX | jq .data.data.tx) + assertEquals "type=chain" '"chain"' $(echo $CTX | jq .type) + STX=$(echo $CTX | jq .data.tx) + assertEquals "type=send" '"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 $? } diff --git a/txs/sigs.go b/txs/sigs.go index 699b6aa520..d49dcb026a 100644 --- a/txs/sigs.go +++ b/txs/sigs.go @@ -64,16 +64,12 @@ func (s *OneSig) Next() 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 errors.ErrUnauthorized() - } return s.Tx.ValidateBasic() } // TxBytes returns the full data with signatures func (s *OneSig) TxBytes() ([]byte, error) { - return data.ToWire(s) + return data.ToWire(s.Wrap()) } // SignBytes returns the original data passed into `NewSig` @@ -139,17 +135,12 @@ func (s *MultiSig) Next() basecoin.Tx { } func (s *MultiSig) ValidateBasic() error { - // TODO: more efficient - _, err := s.Signers() - if err != nil { - return err - } return s.Tx.ValidateBasic() } // TxBytes returns the full data with signatures func (s *MultiSig) TxBytes() ([]byte, error) { - return data.ToWire(s) + return data.ToWire(s.Wrap()) } // SignBytes returns the original data passed into `NewSig` From 9cd303d1fd7c6aa1932cdb51b80fdcfe6ad446d6 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 3 Jul 2017 22:34:30 +0200 Subject: [PATCH 10/24] Cleaned up unneeded adapters --- cmd/basecli/commands/apptx.go | 82 ------------------------ cmd/basecli/commands/cmds.go | 70 -------------------- cmd/basecli/commands/sendtx.go | 113 --------------------------------- 3 files changed, 265 deletions(-) delete mode 100644 cmd/basecli/commands/apptx.go delete mode 100644 cmd/basecli/commands/sendtx.go diff --git a/cmd/basecli/commands/apptx.go b/cmd/basecli/commands/apptx.go deleted file mode 100644 index 6b18d52e5e..0000000000 --- a/cmd/basecli/commands/apptx.go +++ /dev/null @@ -1,82 +0,0 @@ -package commands - -import ( - "github.com/pkg/errors" - - crypto "github.com/tendermint/go-crypto" - keys "github.com/tendermint/go-crypto/keys" - wire "github.com/tendermint/go-wire" - - bc "github.com/tendermint/basecoin/types" -) - -// AppTx Application transaction structure for client -type AppTx struct { - chainID string - signers []crypto.PubKey - Tx *bc.AppTx -} - -var _ keys.Signable = &AppTx{} - -// SignBytes returned the unsigned bytes, needing a signature -func (s *AppTx) SignBytes() []byte { - return s.Tx.SignBytes(s.chainID) -} - -// Sign will add a signature and pubkey. -// -// Depending on the Signable, one may be able to call this multiple times for multisig -// Returns error if called with invalid data or too many times -func (s *AppTx) Sign(pubkey crypto.PubKey, sig crypto.Signature) error { - if len(s.signers) > 0 { - return errors.New("AppTx already signed") - } - s.Tx.SetSignature(sig) - s.signers = []crypto.PubKey{pubkey} - return nil -} - -// Signers will return the public key(s) that signed if the signature -// is valid, or an error if there is any issue with the signature, -// including if there are no signatures -func (s *AppTx) Signers() ([]crypto.PubKey, error) { - if len(s.signers) == 0 { - return nil, errors.New("No signatures on AppTx") - } - return s.signers, nil -} - -// TxBytes returns the transaction data as well as all signatures -// It should return an error if Sign was never called -func (s *AppTx) TxBytes() ([]byte, error) { - // TODO: verify it is signed - - // Code and comment from: basecoin/cmd/basecoin/commands/tx.go - // Don't you hate having to do this? - // How many times have I lost an hour over this trick?! - txBytes := wire.BinaryBytes(bc.TxS{s.Tx}) - return txBytes, nil -} - -// TODO: this should really be in the basecoin.types SendTx, -// but that code is too ugly now, needs refactor.. -func (a *AppTx) ValidateBasic() error { - if a.chainID == "" { - return errors.New("No chain-id specified") - } - in := a.Tx.Input - if len(in.Address) != 20 { - return errors.Errorf("Invalid input address length: %d", len(in.Address)) - } - if !in.Coins.IsValid() { - return errors.Errorf("Invalid input coins %v", in.Coins) - } - if in.Coins.IsZero() { - return errors.New("Input coins cannot be zero") - } - if in.Sequence <= 0 { - return errors.New("Sequence must be greater than 0") - } - return nil -} diff --git a/cmd/basecli/commands/cmds.go b/cmd/basecli/commands/cmds.go index f8cbd77059..5f64fc1a39 100644 --- a/cmd/basecli/commands/cmds.go +++ b/cmd/basecli/commands/cmds.go @@ -6,13 +6,11 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" - flag "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/tendermint/basecoin" "github.com/tendermint/light-client/commands" txcmd "github.com/tendermint/light-client/commands/txs" - ctypes "github.com/tendermint/tendermint/rpc/core/types" cmn "github.com/tendermint/tmlibs/common" "github.com/tendermint/basecoin/modules/coin" @@ -145,74 +143,6 @@ func parseChainAddress(toFlag string) (string, []byte, error) { return chainPrefix, to, nil } -//------------------------- -// AppTx - -// BroadcastAppTx wraps, signs, and executes an app tx basecoin transaction -func BroadcastAppTx(tx *btypes.AppTx) (*ctypes.ResultBroadcastTxCommit, error) { - - // Sign if needed and post to the node. This it the work-horse - return txcmd.SignAndPostTx(WrapAppTx(tx)) -} - -// AddAppTxFlags adds flags required by apptx -func AddAppTxFlags(fs *flag.FlagSet) { - fs.String(FlagAmount, "", "Coins to send in the format ,...") - fs.String(FlagFee, "0mycoin", "Coins for the transaction fee of the format ") - fs.Int64(FlagGas, 0, "Amount of gas for this transaction") - fs.Int(FlagSequence, -1, "Sequence number for this transaction") -} - -// ReadAppTxFlags reads in the standard flags -// your command should parse info to set tx.Name and tx.Data -func ReadAppTxFlags() (gas int64, fee btypes.Coin, txInput btypes.TxInput, err error) { - - // Set the gas - gas = viper.GetInt64(FlagGas) - - // Parse the fee and amounts into coin types - fee, err = btypes.ParseCoin(viper.GetString(FlagFee)) - if err != nil { - return - } - - // retrieve the amount - var amount btypes.Coins - amount, err = btypes.ParseCoins(viper.GetString(FlagAmount)) - if err != nil { - return - } - - // get the PubKey of the signer - pk := txcmd.GetSigner() - - // get addr if available - var addr []byte - if !pk.Empty() { - addr = pk.Address() - } - - // set the output - txInput = btypes.TxInput{ - Coins: amount, - Sequence: viper.GetInt(FlagSequence), - Address: addr, - } - // set the pubkey if needed - if txInput.Sequence == 1 { - txInput.PubKey = pk - } - return -} - -// WrapAppTx wraps the transaction with chain id -func WrapAppTx(tx *btypes.AppTx) *AppTx { - return &AppTx{ - chainID: commands.GetChainID(), - Tx: tx, - } -} - /** TODO copied from basecoin cli - put in common somewhere? **/ // ParseHexFlag parses a flag string to byte array diff --git a/cmd/basecli/commands/sendtx.go b/cmd/basecli/commands/sendtx.go deleted file mode 100644 index 1a338ef29e..0000000000 --- a/cmd/basecli/commands/sendtx.go +++ /dev/null @@ -1,113 +0,0 @@ -package commands - -import ( - "github.com/pkg/errors" - bc "github.com/tendermint/basecoin/types" - crypto "github.com/tendermint/go-crypto" - keys "github.com/tendermint/go-crypto/keys" - wire "github.com/tendermint/go-wire" -) - -type SendTx struct { - chainID string - signers []crypto.PubKey - Tx *bc.SendTx -} - -var _ keys.Signable = &SendTx{} - -// SignBytes returned the unsigned bytes, needing a signature -func (s *SendTx) SignBytes() []byte { - return s.Tx.SignBytes(s.chainID) -} - -// Sign will add a signature and pubkey. -// -// Depending on the Signable, one may be able to call this multiple times for multisig -// Returns error if called with invalid data or too many times -func (s *SendTx) Sign(pubkey crypto.PubKey, sig crypto.Signature) error { - addr := pubkey.Address() - set := s.Tx.SetSignature(addr, sig) - if !set { - return errors.Errorf("Cannot add signature for address %X", addr) - } - s.signers = append(s.signers, pubkey) - return nil -} - -// Signers will return the public key(s) that signed if the signature -// is valid, or an error if there is any issue with the signature, -// including if there are no signatures -func (s *SendTx) Signers() ([]crypto.PubKey, error) { - if len(s.signers) == 0 { - return nil, errors.New("No signatures on SendTx") - } - return s.signers, nil -} - -// TxBytes returns the transaction data as well as all signatures -// It should return an error if Sign was never called -func (s *SendTx) TxBytes() ([]byte, error) { - // TODO: verify it is signed - - // Code and comment from: basecoin/cmd/basecoin/commands/tx.go - // Don't you hate having to do this? - // How many times have I lost an hour over this trick?! - txBytes := wire.BinaryBytes(struct { - bc.Tx `json:"unwrap"` - }{s.Tx}) - return txBytes, nil -} - -// AddSigner sets address and pubkey info on the tx based on the key that -// will be used for signing -func (s *SendTx) AddSigner(pk crypto.PubKey) { - // get addr if available - var addr []byte - if !pk.Empty() { - addr = pk.Address() - } - - // set the send address, and pubkey if needed - in := s.Tx.Inputs - in[0].Address = addr - if in[0].Sequence == 1 { - in[0].PubKey = pk - } -} - -// TODO: this should really be in the basecoin.types SendTx, -// but that code is too ugly now, needs refactor.. -func (s *SendTx) ValidateBasic() error { - if s.chainID == "" { - return errors.New("No chain-id specified") - } - for _, in := range s.Tx.Inputs { - if len(in.Address) != 20 { - return errors.Errorf("Invalid input address length: %d", len(in.Address)) - } - if !in.Coins.IsValid() { - return errors.Errorf("Invalid input coins %v", in.Coins) - } - if in.Coins.IsZero() { - return errors.New("Input coins cannot be zero") - } - if in.Sequence <= 0 { - return errors.New("Sequence must be greater than 0") - } - } - for _, out := range s.Tx.Outputs { - // we now allow chain/addr, so it can be more than 20 bytes - if len(out.Address) < 20 { - return errors.Errorf("Invalid output address length: %d", len(out.Address)) - } - if !out.Coins.IsValid() { - return errors.Errorf("Invalid output coins %v", out.Coins) - } - if out.Coins.IsZero() { - return errors.New("Output coins cannot be zero") - } - } - - return nil -} From fcab8ac90184763bbcdafba13b36289ae0e6adaf Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 4 Jul 2017 12:22:06 +0200 Subject: [PATCH 11/24] Write dispatcher, change SetOption arguments --- app/app.go | 33 ++++++++++--- app/app_test.go | 22 +++++++-- app/genesis.go | 2 +- errors/common.go | 9 ++++ handler.go | 10 ++-- modules/coin/handler.go | 7 ++- modules/coin/handler_test.go | 7 ++- modules/coin/store.go | 2 + stack/dispatcher.go | 94 ++++++++++++++++++++++++++++++++++++ stack/interface.go | 49 ++++++++++++++++--- stack/logger.go | 6 +-- stack/middleware.go | 4 +- stack/recovery.go | 4 +- 13 files changed, 215 insertions(+), 34 deletions(-) create mode 100644 stack/dispatcher.go diff --git a/app/app.go b/app/app.go index 69052a0ee3..d8c2f9b609 100644 --- a/app/app.go +++ b/app/app.go @@ -1,6 +1,9 @@ package app import ( + "fmt" + "strings" + abci "github.com/tendermint/abci/types" "github.com/tendermint/basecoin" eyes "github.com/tendermint/merkleeyes/client" @@ -15,8 +18,8 @@ import ( ) const ( - PluginNameBase = "base" - ChainKey = "base/chain_id" + ModuleNameBase = "base" + ChainKey = "chain_id" ) type Basecoin struct { @@ -66,18 +69,26 @@ func (app *Basecoin) Info() abci.ResponseInfo { // ABCI::SetOption func (app *Basecoin) SetOption(key string, value string) string { - if key == ChainKey { - app.state.SetChainID(value) - return "Success" + module, prefix := splitKey(key) + if module == ModuleNameBase { + return app.setBaseOption(prefix, value) } - log, err := app.handler.SetOption(app.logger, app.state, key, value) + log, err := app.handler.SetOption(app.logger, app.state, module, prefix, value) if err == nil { return log } return "Error: " + err.Error() } +func (app *Basecoin) setBaseOption(key, value string) string { + if key == ChainKey { + app.state.SetChainID(value) + return "Success" + } + return fmt.Sprintf("Error: unknown base option: %s", key) +} + // ABCI::DeliverTx func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { tx, err := basecoin.LoadTx(txBytes) @@ -178,3 +189,13 @@ func (app *Basecoin) EndBlock(height uint64) (res abci.ResponseEndBlock) { // } return } + +// Splits the string at the first '/'. +// if there are none, assign default module ("base"). +func splitKey(key string) (string, string) { + if strings.Contains(key, "/") { + keyParts := strings.SplitN(key, "/", 2) + return keyParts[0], keyParts[1] + } + return ModuleNameBase, key +} diff --git a/app/app_test.go b/app/app_test.go index ab007ce374..a86f290024 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -58,7 +58,7 @@ func (at *appTest) getTx(seq int, coins types.Coins) basecoin.Tx { func (at *appTest) acc2app(acc types.Account) { accBytes, err := json.Marshal(acc) require.Nil(at.t, err) - res := at.app.SetOption("base/account", string(accBytes)) + res := at.app.SetOption("coin/account", string(accBytes)) require.EqualValues(at.t, res, "Success") } @@ -137,7 +137,7 @@ func TestSetOption(t *testing.T) { accIn := types.MakeAcc("input0").Account accsInBytes, err := json.Marshal(accIn) assert.Nil(err) - res = app.SetOption("base/account", string(accsInBytes)) + res = app.SetOption("coin/account", string(accsInBytes)) require.EqualValues(res, "Success") // make sure it is set correctly, with some balance @@ -165,7 +165,7 @@ func TestSetOption(t *testing.T) { } ] }` - res = app.SetOption("base/account", unsortAcc) + res = app.SetOption("coin/account", unsortAcc) require.EqualValues(res, "Success") coins, err = getAddr(unsortAddr, app.state) @@ -234,3 +234,19 @@ func TestQuery(t *testing.T) { }) assert.NotEqual(resQueryPreCommit, resQueryPostCommit, "Query should change before/after commit") } + +func TestSplitKey(t *testing.T) { + assert := assert.New(t) + prefix, suffix := splitKey("foo/bar") + assert.EqualValues("foo", prefix) + assert.EqualValues("bar", suffix) + + prefix, suffix = splitKey("foobar") + assert.EqualValues("base", prefix) + assert.EqualValues("foobar", suffix) + + prefix, suffix = splitKey("some/complex/issue") + assert.EqualValues("some", prefix) + assert.EqualValues("complex/issue", suffix) + +} diff --git a/app/genesis.go b/app/genesis.go index 05913c15b0..9d6050bcdb 100644 --- a/app/genesis.go +++ b/app/genesis.go @@ -19,7 +19,7 @@ func (app *Basecoin) LoadGenesis(path string) error { // set accounts for _, acct := range genDoc.AppOptions.Accounts { - _ = app.SetOption("base/account", string(acct)) + _ = app.SetOption("coin/account", string(acct)) } // set plugin options diff --git a/errors/common.go b/errors/common.go index fdc04ea8ad..ac9e2809a8 100644 --- a/errors/common.go +++ b/errors/common.go @@ -24,6 +24,7 @@ var ( errWrongChain = rawerr.New("Wrong chain for tx") errUnknownTxType = rawerr.New("Tx type unknown") errInvalidFormat = rawerr.New("Invalid format") + errUnknownModule = rawerr.New("Unknown module") ) func ErrUnknownTxType(tx basecoin.Tx) TMError { @@ -44,6 +45,14 @@ 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) +} +func IsUnknownModuleErr(err error) bool { + return IsSameError(errUnknownModule, err) +} + func ErrInternal(msg string) TMError { return New(msg, abci.CodeType_InternalError) } diff --git a/handler.go b/handler.go index 3e3728a09b..34404395b5 100644 --- a/handler.go +++ b/handler.go @@ -47,14 +47,14 @@ func (c DeliverFunc) DeliverTx(ctx Context, store types.KVStore, tx Tx) (Result, } type SetOptioner interface { - SetOption(l log.Logger, store types.KVStore, key, value string) (string, error) + SetOption(l log.Logger, store types.KVStore, module, key, value string) (string, error) } // SetOptionFunc (like http.HandlerFunc) is a shortcut for making wrapers -type SetOptionFunc func(log.Logger, types.KVStore, string, string) (string, error) +type SetOptionFunc func(log.Logger, types.KVStore, string, string, string) (string, error) -func (c SetOptionFunc) SetOption(l log.Logger, store types.KVStore, key, value string) (string, error) { - return c(l, store, key, value) +func (c SetOptionFunc) SetOption(l log.Logger, store types.KVStore, module, key, value string) (string, error) { + return c(l, store, module, key, value) } // Result captures any non-error abci result @@ -83,6 +83,6 @@ func (_ NopDeliver) DeliverTx(Context, types.KVStore, Tx) (r Result, e error) { type NopOption struct{} -func (_ NopOption) SetOption(log.Logger, types.KVStore, string, string) (string, error) { +func (_ NopOption) SetOption(log.Logger, types.KVStore, string, string, string) (string, error) { return "", nil } diff --git a/modules/coin/handler.go b/modules/coin/handler.go index 63cf3d693b..def6689c29 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -80,8 +80,11 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoi return basecoin.Result{}, nil } -func (h Handler) SetOption(l log.Logger, store types.KVStore, key, value string) (log string, err error) { - if key == "base/account" { +func (h Handler) SetOption(l log.Logger, store types.KVStore, module, key, value string) (log string, err error) { + if module != NameCoin { + return "", errors.ErrUnknownModule(module) + } + if key == "account" { var acc GenesisAccount err = data.FromJSON([]byte(value), &acc) if err != nil { diff --git a/modules/coin/handler_test.go b/modules/coin/handler_test.go index 534b5ee168..b13d1bb59a 100644 --- a/modules/coin/handler_test.go +++ b/modules/coin/handler_test.go @@ -172,8 +172,7 @@ func TestSetOption(t *testing.T) { // some sample settings pk := crypto.GenPrivKeySecp256k1().Wrap() addr := pk.PubKey().Address() - actor := basecoin.Actor{App: "coin", Address: addr} - // actor2 := basecoin.Actor{App: "foo", Address: addr} + actor := basecoin.Actor{App: stack.NameSigs, Address: addr} someCoins := types.Coins{{"atom", 123}} otherCoins := types.Coins{{"eth", 11}} @@ -198,13 +197,13 @@ func TestSetOption(t *testing.T) { l := log.NewNopLogger() for i, tc := range cases { store := types.NewMemKVStore() - key := "base/account" + key := "account" // set the options for j, gen := range tc.init { value, err := json.Marshal(gen) require.Nil(err, "%d,%d: %+v", i, j, err) - _, err = h.SetOption(l, store, key, string(value)) + _, err = h.SetOption(l, store, NameCoin, key, string(value)) require.Nil(err) } diff --git a/modules/coin/store.go b/modules/coin/store.go index 6170f780a6..9f9f671104 100644 --- a/modules/coin/store.go +++ b/modules/coin/store.go @@ -94,6 +94,7 @@ type Account struct { } func loadAccount(store types.KVStore, key []byte) (acct Account, err error) { + // fmt.Printf("load: %X\n", key) data := store.Get(key) if len(data) == 0 { return acct, ErrNoAccount() @@ -107,6 +108,7 @@ func loadAccount(store types.KVStore, key []byte) (acct Account, err error) { } func storeAccount(store types.KVStore, key []byte, acct Account) error { + // fmt.Printf("store: %X\n", key) bin := wire.BinaryBytes(acct) store.Set(key, bin) return nil // real stores can return error... diff --git a/stack/dispatcher.go b/stack/dispatcher.go new file mode 100644 index 0000000000..7c07266b37 --- /dev/null +++ b/stack/dispatcher.go @@ -0,0 +1,94 @@ +package stack + +import ( + "fmt" + + "github.com/tendermint/tmlibs/log" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/types" +) + +const ( + NameDispatcher = "disp" +) + +// Dispatcher grabs a bunch of Dispatchables and groups them into one Handler. +// +// It will route tx to the proper locations and also allows them to call each +// other synchronously through the same tx methods. +type Dispatcher struct { + routes map[string]Dispatchable +} + +func NewDispatcher(routes ...Dispatchable) *Dispatcher { + d := &Dispatcher{} + d.AddRoutes(routes...) + return d +} + +var _ basecoin.Handler = new(Dispatcher) + +// AddRoutes registers all these dispatchable choices under their subdomains +// +// Panics on attempt to double-register a route name, as this is a configuration error. +// Should I retrun an error instead? +func (d *Dispatcher) AddRoutes(routes ...Dispatchable) { + for _, r := range routes { + name := r.Name() + if _, ok := d.routes[name]; ok { + panic(fmt.Sprintf("%s already registered with dispatcher", name)) + } + d.routes[name] = r + } +} + +func (d *Dispatcher) Name() string { + return NameDispatcher +} + +func (d *Dispatcher) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { + r, err := d.lookupTx(tx) + if err != nil { + return res, err + } + // TODO: callback + return r.CheckTx(ctx, store, tx, nil) +} + +func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { + r, err := d.lookupTx(tx) + if err != nil { + return res, err + } + // TODO: callback + return r.DeliverTx(ctx, store, tx, nil) +} + +func (d *Dispatcher) SetOption(l log.Logger, store types.KVStore, module, key, value string) (string, error) { + r, err := d.lookupModule(module) + if err != nil { + return "", err + } + // TODO: callback + return r.SetOption(l, store, module, key, value, nil) +} + +func (d *Dispatcher) lookupTx(tx basecoin.Tx) (Dispatchable, error) { + // TODO + name := "foo" + r, ok := d.routes[name] + if !ok { + return nil, errors.ErrUnknownTxType(tx) + } + return r, nil +} + +func (d *Dispatcher) lookupModule(name string) (Dispatchable, error) { + r, ok := d.routes[name] + if !ok { + return nil, errors.ErrUnknownModule(name) + } + return r, nil +} diff --git a/stack/interface.go b/stack/interface.go index 951a7fa73f..09e1ad4d16 100644 --- a/stack/interface.go +++ b/stack/interface.go @@ -39,13 +39,13 @@ func (d DeliverMiddleFunc) DeliverTx(ctx basecoin.Context, store types.KVStore, } type SetOptionMiddle interface { - SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (string, error) + SetOption(l log.Logger, store types.KVStore, module, key, value string, next basecoin.SetOptioner) (string, error) } -type SetOptionMiddleFunc func(log.Logger, types.KVStore, string, string, basecoin.SetOptioner) (string, error) +type SetOptionMiddleFunc func(log.Logger, types.KVStore, string, string, string, basecoin.SetOptioner) (string, error) -func (c SetOptionMiddleFunc) SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (string, error) { - return c(l, store, key, value, next) +func (c SetOptionMiddleFunc) SetOption(l log.Logger, store types.KVStore, module, key, value string, next basecoin.SetOptioner) (string, error) { + return c(l, store, module, key, value, next) } // holders @@ -63,6 +63,43 @@ func (_ PassDeliver) DeliverTx(ctx basecoin.Context, store types.KVStore, tx bas type PassOption struct{} -func (_ PassOption) SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (string, error) { - return next.SetOption(l, store, key, value) +func (_ PassOption) SetOption(l log.Logger, store types.KVStore, module, key, value string, next basecoin.SetOptioner) (string, error) { + return next.SetOption(l, store, module, key, value) +} + +// Dispatchable is like middleware, except the meaning of "next" is different. +// Whereas in the middleware, it is the next handler that we should pass the same tx into, +// for dispatchers, it is a dispatcher, which it can use to +type Dispatchable interface { + Middleware + AssertDispatcher() +} + +// WrapHandler turns a basecoin.Handler into a Dispatchable interface +func WrapHandler(h basecoin.Handler) Dispatchable { + return wrapped{h} +} + +type wrapped struct { + h basecoin.Handler +} + +var _ Dispatchable = wrapped{} + +func (w wrapped) AssertDispatcher() {} + +func (w wrapped) Name() string { + return w.h.Name() +} + +func (w wrapped) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, _ basecoin.Checker) (basecoin.Result, error) { + return w.h.CheckTx(ctx, store, tx) +} + +func (w wrapped) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, _ basecoin.Deliver) (basecoin.Result, error) { + return w.h.DeliverTx(ctx, store, tx) +} + +func (w wrapped) SetOption(l log.Logger, store types.KVStore, module, key, value string, _ basecoin.SetOptioner) (string, error) { + return w.h.SetOption(l, store, module, key, value) } diff --git a/stack/logger.go b/stack/logger.go index 2bd41a5fa2..1f79ac3d36 100644 --- a/stack/logger.go +++ b/stack/logger.go @@ -50,12 +50,12 @@ func (_ Logger) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin return } -func (_ Logger) SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (string, error) { +func (_ Logger) SetOption(l log.Logger, store types.KVStore, module, key, value string, next basecoin.SetOptioner) (string, error) { start := time.Now() - res, err := next.SetOption(l, store, key, value) + res, err := next.SetOption(l, store, module, key, value) delta := time.Now().Sub(start) // TODO: log the value being set also? - l = l.With("duration", micros(delta)).With("key", key) + l = l.With("duration", micros(delta)).With("mod", module).With("key", key) if err == nil { l.Info("SetOption", "log", res) } else { diff --git a/stack/middleware.go b/stack/middleware.go index fcc450b730..ed9c3beba0 100644 --- a/stack/middleware.go +++ b/stack/middleware.go @@ -39,8 +39,8 @@ func (m *middleware) DeliverTx(ctx basecoin.Context, store types.KVStore, tx bas return m.middleware.DeliverTx(ctx, store, tx, next) } -func (m *middleware) SetOption(l log.Logger, store types.KVStore, key, value string) (string, error) { - return m.middleware.SetOption(l, store, key, value, m.next) +func (m *middleware) SetOption(l log.Logger, store types.KVStore, module, key, value string) (string, error) { + return m.middleware.SetOption(l, store, module, key, value, m.next) } // Stack is the entire application stack diff --git a/stack/recovery.go b/stack/recovery.go index 7dbb10a439..be213ad878 100644 --- a/stack/recovery.go +++ b/stack/recovery.go @@ -41,13 +41,13 @@ func (_ Recovery) DeliverTx(ctx basecoin.Context, store types.KVStore, tx baseco return next.DeliverTx(ctx, store, tx) } -func (_ Recovery) SetOption(l log.Logger, store types.KVStore, key, value string, next basecoin.SetOptioner) (log string, err error) { +func (_ Recovery) SetOption(l log.Logger, store types.KVStore, module, key, value string, next basecoin.SetOptioner) (log string, err error) { defer func() { if r := recover(); r != nil { err = normalizePanic(r) } }() - return next.SetOption(l, store, key, value) + return next.SetOption(l, store, module, key, value) } // normalizePanic makes sure we can get a nice TMError (with stack) out of it From 473451f020c5f22aee12ce7954aa7d84965a3a76 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 4 Jul 2017 12:46:57 +0200 Subject: [PATCH 12/24] Integrate dispatcher into app, and fix tests --- app/app.go | 3 ++- app/app_test.go | 11 +++++++++-- modules/coin/tx.go | 2 +- stack/dispatcher.go | 13 ++++++++++--- tests/cli/common.sh | 2 +- tx.go | 22 ++++++++++++++++++++++ tx_test.go | 30 ++++++++++++++++++++++++++++++ 7 files changed, 75 insertions(+), 8 deletions(-) diff --git a/app/app.go b/app/app.go index d8c2f9b609..16d8f6b654 100644 --- a/app/app.go +++ b/app/app.go @@ -46,7 +46,8 @@ func NewBasecoin(h basecoin.Handler, eyesCli *eyes.Client, l log.Logger) *Baseco func DefaultHandler() basecoin.Handler { // use the default stack h := coin.NewHandler() - return stack.NewDefault().Use(h) + d := stack.NewDispatcher(stack.WrapHandler(h)) + return stack.NewDefault().Use(d) } // XXX For testing, not thread safe! diff --git a/app/app_test.go b/app/app_test.go index a86f290024..753efcf3f7 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -3,6 +3,7 @@ package app import ( "encoding/hex" "encoding/json" + "os" "testing" "github.com/stretchr/testify/assert" @@ -68,8 +69,14 @@ func (at *appTest) reset() { at.accOut = types.MakeAcc("output0") eyesCli := eyes.NewLocalClient("", 0) - at.app = NewBasecoin(DefaultHandler(), eyesCli, - log.TestingLogger().With("module", "app")) + // l := log.TestingLogger().With("module", "app"), + l := log.NewTMLogger(os.Stdout).With("module", "app") + l = log.NewTracingLogger(l) + at.app = NewBasecoin( + DefaultHandler(), + eyesCli, + l, + ) res := at.app.SetOption("base/chain_id", at.chainID) require.EqualValues(at.t, res, "Success") diff --git a/modules/coin/tx.go b/modules/coin/tx.go index 4087d7c945..c5f7370c01 100644 --- a/modules/coin/tx.go +++ b/modules/coin/tx.go @@ -15,7 +15,7 @@ func init() { // we reserve the 0x20-0x3f range for standard modules const ( ByteSend = 0x20 - TypeSend = "send" + TypeSend = NameCoin + "/send" ) //----------------------------------------------------------------------------- diff --git a/stack/dispatcher.go b/stack/dispatcher.go index 7c07266b37..6e177ca098 100644 --- a/stack/dispatcher.go +++ b/stack/dispatcher.go @@ -2,6 +2,7 @@ package stack import ( "fmt" + "strings" "github.com/tendermint/tmlibs/log" @@ -23,7 +24,9 @@ type Dispatcher struct { } func NewDispatcher(routes ...Dispatchable) *Dispatcher { - d := &Dispatcher{} + d := &Dispatcher{ + routes: map[string]Dispatchable{}, + } d.AddRoutes(routes...) return d } @@ -76,8 +79,12 @@ func (d *Dispatcher) SetOption(l log.Logger, store types.KVStore, module, key, v } func (d *Dispatcher) lookupTx(tx basecoin.Tx) (Dispatchable, error) { - // TODO - name := "foo" + kind, err := tx.GetKind() + if err != nil { + return nil, err + } + // grab everything before the / + name := strings.SplitN(kind, "/", 2)[0] r, ok := d.routes[name] if !ok { return nil, errors.ErrUnknownTxType(tx) diff --git a/tests/cli/common.sh b/tests/cli/common.sh index 7c3b886ea3..82a35849e2 100644 --- a/tests/cli/common.sh +++ b/tests/cli/common.sh @@ -163,7 +163,7 @@ checkSendTx() { CTX=$(echo $TX | jq .data.data.tx) assertEquals "type=chain" '"chain"' $(echo $CTX | jq .type) STX=$(echo $CTX | jq .data.tx) - assertEquals "type=send" '"send"' $(echo $STX | jq .type) + 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 $? diff --git a/tx.go b/tx.go index 8336f02b6c..2e8d5f2bd8 100644 --- a/tx.go +++ b/tx.go @@ -55,3 +55,25 @@ func (t Tx) GetLayer() TxLayer { l, _ := t.Unwrap().(TxLayer) return l } + +// env lets us parse an envelope and just grab the type +type env struct { + Kind string `json:"type"` +} + +// TODO: put this functionality into go-data in a cleaner and more efficient way +func (t Tx) GetKind() (string, error) { + // render as json + d, err := data.ToJSON(t) + if err != nil { + return "", err + } + // parse json + text := env{} + err = data.FromJSON(d, &text) + if err != nil { + return "", err + } + // grab the type we used in json + return text.Kind, nil +} diff --git a/tx_test.go b/tx_test.go index b766b39328..c54a26dba6 100644 --- a/tx_test.go +++ b/tx_test.go @@ -4,6 +4,20 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func init() { + TxMapper. + RegisterImplementation(Demo{}, TypeDemo, ByteDemo). + RegisterImplementation(Fake{}, TypeFake, ByteFake) +} + +const ( + ByteDemo = 0xF0 + TypeDemo = "test/demo" + ByteFake = 0xF1 + TypeFake = "test/fake" ) // define a Demo struct that implements TxLayer @@ -35,3 +49,19 @@ func TestLayer(t *testing.T) { assert.True(l.IsLayer()) assert.NotNil(l.GetLayer()) } + +func TestKind(t *testing.T) { + cases := []struct { + tx Tx + kind string + }{ + {Demo{}.Wrap(), TypeDemo}, + {Fake{}.Wrap(), TypeFake}, + } + + for _, tc := range cases { + kind, err := tc.tx.GetKind() + require.Nil(t, err, "%+v", err) + assert.Equal(t, tc.kind, kind) + } +} From 6d56891a0fffb19de6a2eab32199c9d5daa0d7f5 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 4 Jul 2017 13:43:25 +0200 Subject: [PATCH 13/24] Re-implement counter plugin --- cmd/basecoin/commands/start.go | 27 +-- cmd/basecoin/main.go | 3 + docs/guide/counter/cmd/counter/main.go | 4 + docs/guide/counter/plugins/counter/counter.go | 206 ++++++++++++------ .../counter/plugins/counter/counter_test.go | 87 +++----- 5 files changed, 179 insertions(+), 148 deletions(-) diff --git a/cmd/basecoin/commands/start.go b/cmd/basecoin/commands/start.go index aeb49a1c35..791e810a62 100644 --- a/cmd/basecoin/commands/start.go +++ b/cmd/basecoin/commands/start.go @@ -22,8 +22,6 @@ import ( "github.com/tendermint/tendermint/types" "github.com/tendermint/basecoin/app" - "github.com/tendermint/basecoin/modules/coin" - "github.com/tendermint/basecoin/stack" ) var StartCmd = &cobra.Command{ @@ -42,6 +40,12 @@ const ( FlagWithoutTendermint = "without-tendermint" ) +var ( + // use a global to store the handler, so we can set it in main. + // TODO: figure out a cleaner way to register plugins + Handler basecoin.Handler +) + func init() { flags := StartCmd.Flags() flags.String(FlagAddress, "tcp://0.0.0.0:46658", "Listen address") @@ -51,22 +55,6 @@ func init() { tcmd.AddNodeFlags(StartCmd) } -// TODO: setup handler instead of Plugins -func getHandler() basecoin.Handler { - // use the default stack - h := coin.NewHandler() - app := stack.NewDefault().Use(h) - return app - - // register IBC plugn - // basecoinApp.RegisterPlugin(NewIBCPlugin()) - - // register all other plugins - // for _, p := range plugins { - // basecoinApp.RegisterPlugin(p.newPlugin()) - // } -} - func startCmd(cmd *cobra.Command, args []string) error { rootDir := viper.GetString(cli.HomeFlag) meyes := viper.GetString(FlagEyes) @@ -85,8 +73,7 @@ func startCmd(cmd *cobra.Command, args []string) error { } // Create Basecoin app - h := app.DefaultHandler() - basecoinApp := app.NewBasecoin(h, eyesCli, logger.With("module", "app")) + basecoinApp := app.NewBasecoin(Handler, eyesCli, logger.With("module", "app")) // if chain_id has not been set yet, load the genesis. // else, assume it's been loaded diff --git a/cmd/basecoin/main.go b/cmd/basecoin/main.go index 7190a8ee59..51c8ab4917 100644 --- a/cmd/basecoin/main.go +++ b/cmd/basecoin/main.go @@ -3,6 +3,7 @@ package main import ( "os" + "github.com/tendermint/basecoin/app" "github.com/tendermint/basecoin/cmd/basecoin/commands" "github.com/tendermint/tmlibs/cli" ) @@ -10,6 +11,8 @@ import ( func main() { rt := commands.RootCmd + commands.Handler = app.DefaultHandler() + rt.AddCommand( commands.InitCmd, commands.StartCmd, diff --git a/docs/guide/counter/cmd/counter/main.go b/docs/guide/counter/cmd/counter/main.go index c877a51eda..854cd0efe3 100644 --- a/docs/guide/counter/cmd/counter/main.go +++ b/docs/guide/counter/cmd/counter/main.go @@ -7,6 +7,7 @@ import ( "github.com/tendermint/tmlibs/cli" + "github.com/tendermint/basecoin/app" "github.com/tendermint/basecoin/cmd/basecoin/commands" "github.com/tendermint/basecoin/docs/guide/counter/plugins/counter" "github.com/tendermint/basecoin/types" @@ -18,6 +19,9 @@ func main() { Short: "demo plugin for basecoin", } + // TODO: register the counter here + commands.Handler = app.DefaultHandler() + RootCmd.AddCommand( commands.InitCmd, commands.StartCmd, diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 79694c4052..71050862e2 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -1,16 +1,31 @@ package counter import ( - "fmt" + rawerr "errors" abci "github.com/tendermint/abci/types" - "github.com/tendermint/basecoin/types" "github.com/tendermint/go-wire" + + "github.com/tendermint/basecoin" + "github.com/tendermint/basecoin/errors" + "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/types" ) -type CounterPluginState struct { - Counter int - TotalFees types.Coins +// CounterTx +//-------------------------------------------------------------------------------- + +// register the tx type with it's validation logic +// make sure to use the name of the handler as the prefix in the tx type, +// so it gets routed properly +const ( + NameCounter = "cntr" + ByteTx = 0x21 + TypeTx = NameCounter + "/count" +) + +func init() { + basecoin.TxMapper.RegisterImplementation(CounterTx{}, TypeTx, ByteTx) } type CounterTx struct { @@ -18,81 +33,130 @@ type CounterTx struct { Fee types.Coins } +func NewCounterTx(valid bool, fee types.Coins) basecoin.Tx { + return CounterTx{ + Valid: valid, + Fee: fee, + }.Wrap() +} + +func (c CounterTx) Wrap() basecoin.Tx { + return basecoin.Tx{c} +} + +// ValidateBasic just makes sure the Fee is a valid, non-negative value +func (c CounterTx) ValidateBasic() error { + if !c.Fee.IsValid() { + return coin.ErrInvalidCoins() + } + if !c.Fee.IsNonnegative() { + return coin.ErrInvalidCoins() + } + return nil +} + +// Custom errors //-------------------------------------------------------------------------------- -type CounterPlugin struct { +var ( + errInvalidCounter = rawerr.New("Counter Tx marked invalid") +) + +// This is a custom error class +func ErrInvalidCounter() error { + return errors.WithCode(errInvalidCounter, abci.CodeType_BaseInvalidInput) +} +func IsInvalidCounterErr(err error) bool { + return errors.IsSameError(errInvalidCounter, err) } -func (cp *CounterPlugin) Name() string { - return "counter" +// This is just a helper function to return a generic "internal error" +func ErrDecoding() error { + return errors.ErrInternal("Error decoding state") } -func (cp *CounterPlugin) StateKey() []byte { - return []byte(fmt.Sprintf("CounterPlugin.State")) +// CounterHandler +//-------------------------------------------------------------------------------- + +type CounterHandler struct { + basecoin.NopOption } -func New() *CounterPlugin { - return &CounterPlugin{} +var _ basecoin.Handler = CounterHandler{} + +func (_ CounterHandler) Name() string { + return NameCounter } -func (cp *CounterPlugin) SetOption(store types.KVStore, key, value string) (log string) { - return "" -} - -func (cp *CounterPlugin) RunTx(store types.KVStore, ctx types.CallContext, txBytes []byte) (res abci.Result) { - // Decode tx - var tx CounterTx - err := wire.ReadBinaryBytes(txBytes, &tx) - if err != nil { - return abci.ErrBaseEncodingError.AppendLog("Error decoding tx: " + err.Error()).PrependLog("CounterTx Error: ") - } - - // Validate tx - if !tx.Valid { - return abci.ErrInternalError.AppendLog("CounterTx.Valid must be true") - } - if !tx.Fee.IsValid() { - return abci.ErrInternalError.AppendLog("CounterTx.Fee is not sorted or has zero amounts") - } - if !tx.Fee.IsNonnegative() { - return abci.ErrInternalError.AppendLog("CounterTx.Fee must be nonnegative") - } - - // Did the caller provide enough coins? - if !ctx.Coins.IsGTE(tx.Fee) { - return abci.ErrInsufficientFunds.AppendLog("CounterTx.Fee was not provided") - } - - // TODO If there are any funds left over, return funds. - // e.g. !ctx.Coins.Minus(tx.Fee).IsZero() - // ctx.CallerAccount is synced w/ store, so just modify that and store it. - - // Load CounterPluginState - var cpState CounterPluginState - cpStateBytes := store.Get(cp.StateKey()) - if len(cpStateBytes) > 0 { - err = wire.ReadBinaryBytes(cpStateBytes, &cpState) - if err != nil { - return abci.ErrInternalError.AppendLog("Error decoding state: " + err.Error()) - } - } - - // Update CounterPluginState - cpState.Counter += 1 - cpState.TotalFees = cpState.TotalFees.Plus(tx.Fee) - - // Save CounterPluginState - store.Set(cp.StateKey(), wire.BinaryBytes(cpState)) - - return abci.OK -} - -func (cp *CounterPlugin) InitChain(store types.KVStore, vals []*abci.Validator) { -} - -func (cp *CounterPlugin) BeginBlock(store types.KVStore, hash []byte, header *abci.Header) { -} - -func (cp *CounterPlugin) EndBlock(store types.KVStore, height uint64) (res abci.ResponseEndBlock) { +// CheckTx checks if the tx is properly structured +func (h CounterHandler) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { + _, err = checkTx(ctx, tx) return } + +// DeliverTx executes the tx if valid +func (h CounterHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { + ctr, err := checkTx(ctx, tx) + if err != nil { + return res, err + } + // note that we don't assert this on CheckTx (ValidateBasic), + // as we allow them to be writen to the chain + if !ctr.Valid { + return res, ErrInvalidCounter() + } + + // TODO: handle coin movement.... ugh, need sequence to do this, right? + + // update the counter + state, err := LoadState(store) + if err != nil { + return res, err + } + state.Counter += 1 + state.TotalFees = state.TotalFees.Plus(ctr.Fee) + err = StoreState(store, state) + + return res, err +} + +func checkTx(ctx basecoin.Context, tx basecoin.Tx) (ctr CounterTx, err error) { + ctr, ok := tx.Unwrap().(CounterTx) + if !ok { + return ctr, errors.ErrInvalidFormat(tx) + } + err = ctr.ValidateBasic() + if err != nil { + return ctr, err + } + return ctr, nil +} + +// CounterStore +//-------------------------------------------------------------------------------- + +type CounterPluginState struct { + Counter int + TotalFees types.Coins +} + +func StateKey() []byte { + return []byte(NameCounter + "/state") +} + +func LoadState(store types.KVStore) (state CounterPluginState, err error) { + bytes := store.Get(StateKey()) + if len(bytes) > 0 { + err = wire.ReadBinaryBytes(bytes, &state) + if err != nil { + return state, errors.ErrDecoding() + } + } + return state, nil +} + +func StoreState(store types.KVStore, state CounterPluginState) error { + bytes := wire.BinaryBytes(state) + store.Set(StateKey(), bytes) + return nil +} diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index 73f5ffb112..3fa84849a3 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -11,6 +11,7 @@ import ( "github.com/tendermint/basecoin/app" "github.com/tendermint/basecoin/modules/coin" "github.com/tendermint/basecoin/stack" + "github.com/tendermint/basecoin/txs" "github.com/tendermint/basecoin/types" "github.com/tendermint/go-wire" eyescli "github.com/tendermint/merkleeyes/client" @@ -18,10 +19,15 @@ import ( ) // TODO: actually handle the counter here... -func CounterHandler() basecoin.Handler { +func NewCounterHandler() basecoin.Handler { // use the default stack - h := coin.NewHandler() - return stack.NewDefault().Use(h) + coin := coin.NewHandler() + counter := CounterHandler{} + dispatcher := stack.NewDispatcher( + stack.WrapHandler(coin), + stack.WrapHandler(counter), + ) + return stack.NewDefault().Use(dispatcher) } func TestCounterPlugin(t *testing.T) { @@ -30,8 +36,11 @@ func TestCounterPlugin(t *testing.T) { // Basecoin initialization eyesCli := eyescli.NewLocalClient("", 0) chainID := "test_chain_id" - bcApp := app.NewBasecoin(CounterHandler(), eyesCli, - log.TestingLogger().With("module", "app")) + bcApp := app.NewBasecoin( + NewCounterHandler(), + eyesCli, + log.TestingLogger().With("module", "app"), + ) bcApp.SetOption("base/chain_id", chainID) // t.Log(bcApp.Info()) @@ -43,68 +52,32 @@ func TestCounterPlugin(t *testing.T) { test1Acc.Balance = types.Coins{{"", 1000}, {"gold", 1000}} accOpt, err := json.Marshal(test1Acc) require.Nil(t, err) - bcApp.SetOption("base/account", string(accOpt)) + log := bcApp.SetOption("coin/account", string(accOpt)) + require.Equal(t, "Success", log) // Deliver a CounterTx - DeliverCounterTx := func(gas int64, fee types.Coin, inputCoins types.Coins, inputSequence int, appFee types.Coins) abci.Result { - // Construct an AppTx signature - tx := &types.AppTx{ - Gas: gas, - Fee: fee, - Name: "counter", - Input: types.NewTxInput(test1Acc.PubKey, inputCoins, inputSequence), - Data: wire.BinaryBytes(CounterTx{Valid: true, Fee: appFee}), - } - - // Sign request - signBytes := tx.SignBytes(chainID) - // t.Logf("Sign bytes: %X\n", signBytes) - tx.Input.Signature = test1PrivAcc.Sign(signBytes) - // t.Logf("Signed TX bytes: %X\n", wire.BinaryBytes(struct{ types.Tx }{tx})) - - // Write request - txBytes := wire.BinaryBytes(struct{ types.Tx }{tx}) + DeliverCounterTx := func(valid bool, counterFee types.Coins, inputSequence int) abci.Result { + tx := NewCounterTx(valid, counterFee) + tx = txs.NewChain(chainID, tx) + stx := txs.NewSig(tx) + txs.Sign(stx, test1PrivAcc.PrivKey) + txBytes := wire.BinaryBytes(stx.Wrap()) return bcApp.DeliverTx(txBytes) } - // REF: DeliverCounterTx(gas, fee, inputCoins, inputSequence, appFee) { - // Test a basic send, no fee - res := DeliverCounterTx(0, types.Coin{}, types.Coins{{"", 1}}, 1, types.Coins{}) + res := DeliverCounterTx(true, types.Coins{}, 1) assert.True(res.IsOK(), res.String()) - // Test fee prevented transaction - res = DeliverCounterTx(0, types.Coin{"", 2}, types.Coins{{"", 1}}, 2, types.Coins{}) + // Test an invalid send, no fee + res = DeliverCounterTx(false, types.Coins{}, 1) assert.True(res.IsErr(), res.String()) - // Test input equals fee - res = DeliverCounterTx(0, types.Coin{"", 2}, types.Coins{{"", 2}}, 2, types.Coins{}) + // Test the fee + res = DeliverCounterTx(true, types.Coins{{"gold", 100}}, 2) assert.True(res.IsOK(), res.String()) - // Test more input than fee - res = DeliverCounterTx(0, types.Coin{"", 2}, types.Coins{{"", 3}}, 3, types.Coins{}) - assert.True(res.IsOK(), res.String()) - - // Test input equals fee+appFee - res = DeliverCounterTx(0, types.Coin{"", 1}, types.Coins{{"", 3}, {"gold", 1}}, 4, types.Coins{{"", 2}, {"gold", 1}}) - assert.True(res.IsOK(), res.String()) - - // Test fee+appFee prevented transaction, not enough "" - res = DeliverCounterTx(0, types.Coin{"", 1}, types.Coins{{"", 2}, {"gold", 1}}, 5, types.Coins{{"", 2}, {"gold", 1}}) - assert.True(res.IsErr(), res.String()) - - // Test fee+appFee prevented transaction, not enough "gold" - res = DeliverCounterTx(0, types.Coin{"", 1}, types.Coins{{"", 3}, {"gold", 1}}, 5, types.Coins{{"", 2}, {"gold", 2}}) - assert.True(res.IsErr(), res.String()) - - // Test more input than fee, more "" - res = DeliverCounterTx(0, types.Coin{"", 1}, types.Coins{{"", 4}, {"gold", 1}}, 6, types.Coins{{"", 2}, {"gold", 1}}) - assert.True(res.IsOK(), res.String()) - - // Test more input than fee, more "gold" - res = DeliverCounterTx(0, types.Coin{"", 1}, types.Coins{{"", 3}, {"gold", 2}}, 7, types.Coins{{"", 2}, {"gold", 1}}) - assert.True(res.IsOK(), res.String()) - - // REF: DeliverCounterTx(gas, fee, inputCoins, inputSequence, appFee) {w - + // TODO: Test unsupported fee + // res = DeliverCounterTx(true, types.Coins{{"silver", 100}}, 3) + // assert.True(res.IsErr(), res.String()) } From 49ebe59804561e55a8acdb325f5bcc5e7e8690ce Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 4 Jul 2017 14:04:18 +0200 Subject: [PATCH 14/24] Get counter app working, with cli tests --- Makefile | 2 +- docs/guide/counter/cmd/counter/main.go | 5 +- .../cmd/countercli/commands/counter.go | 72 +++-- .../counter/cmd/countercli/commands/query.go | 4 +- docs/guide/counter/plugins/counter/counter.go | 23 +- .../counter/plugins/counter/counter_test.go | 15 - tests/cli/counter.sh | 13 +- tests/tmsp/tmsp_test.go | 270 +++++++++--------- 8 files changed, 199 insertions(+), 205 deletions(-) diff --git a/Makefile b/Makefile index bd7d851fd7..d2ae8dcddd 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ test_unit: test_cli: tests/cli/shunit2 # sudo apt-get install jq @./tests/cli/basictx.sh - # @./tests/cli/counter.sh + @./tests/cli/counter.sh @./tests/cli/restart.sh # @./tests/cli/ibc.sh diff --git a/docs/guide/counter/cmd/counter/main.go b/docs/guide/counter/cmd/counter/main.go index 854cd0efe3..85dafc555e 100644 --- a/docs/guide/counter/cmd/counter/main.go +++ b/docs/guide/counter/cmd/counter/main.go @@ -7,10 +7,8 @@ import ( "github.com/tendermint/tmlibs/cli" - "github.com/tendermint/basecoin/app" "github.com/tendermint/basecoin/cmd/basecoin/commands" "github.com/tendermint/basecoin/docs/guide/counter/plugins/counter" - "github.com/tendermint/basecoin/types" ) func main() { @@ -20,7 +18,7 @@ func main() { } // TODO: register the counter here - commands.Handler = app.DefaultHandler() + commands.Handler = counter.NewCounterHandler() RootCmd.AddCommand( commands.InitCmd, @@ -29,7 +27,6 @@ func main() { commands.VersionCmd, ) - commands.RegisterStartPlugin("counter", func() types.Plugin { return counter.New() }) cmd := cli.PrepareMainCmd(RootCmd, "CT", os.ExpandEnv("$HOME/.counter")) cmd.Execute() } diff --git a/docs/guide/counter/cmd/countercli/commands/counter.go b/docs/guide/counter/cmd/countercli/commands/counter.go index 0481e3d065..3c388c8975 100644 --- a/docs/guide/counter/cmd/countercli/commands/counter.go +++ b/docs/guide/counter/cmd/countercli/commands/counter.go @@ -4,11 +4,12 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" - wire "github.com/tendermint/go-wire" + "github.com/tendermint/basecoin" + "github.com/tendermint/light-client/commands" txcmd "github.com/tendermint/light-client/commands/txs" - bcmd "github.com/tendermint/basecoin/cmd/basecli/commands" "github.com/tendermint/basecoin/docs/guide/counter/plugins/counter" + "github.com/tendermint/basecoin/txs" btypes "github.com/tendermint/basecoin/types" ) @@ -20,64 +21,59 @@ var CounterTxCmd = &cobra.Command{ Long: `Add a vote to the counter. You must pass --valid for it to count and the countfee will be added to the counter.`, - RunE: counterTxCmd, + RunE: doCounterTx, } const ( - flagCountFee = "countfee" - flagValid = "valid" + FlagCountFee = "countfee" + FlagValid = "valid" + FlagSequence = "sequence" // FIXME: currently not supported... ) func init() { fs := CounterTxCmd.Flags() - bcmd.AddAppTxFlags(fs) - fs.String(flagCountFee, "", "Coins to send in the format ,...") - fs.Bool(flagValid, false, "Is count valid?") + 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") } -func counterTxCmd(cmd *cobra.Command, args []string) error { - // Note: we don't support loading apptx from json currently, so skip that - - // Read the app-specific flags - name, data, err := getAppData() +// TODO: doCounterTx is very similar to the sendtx one, +// maybe we can pull out some common patterns? +func doCounterTx(cmd *cobra.Command, args []string) error { + // load data from json or flags + var tx basecoin.Tx + found, err := txcmd.LoadJSON(&tx) + if err != nil { + return err + } + if !found { + tx, err = readCounterTxFlags() + } if err != nil { return err } - // Read the standard app-tx flags - gas, fee, txInput, err := bcmd.ReadAppTxFlags() - if err != nil { - return err - } + // TODO: make this more flexible for middleware + // add the chain info + tx = txs.NewChain(commands.GetChainID(), tx) + stx := txs.NewSig(tx) - // Create AppTx and broadcast - tx := &btypes.AppTx{ - Gas: gas, - Fee: fee, - Name: name, - Input: txInput, - Data: data, - } - res, err := bcmd.BroadcastAppTx(tx) + // Sign if needed and post. This it the work-horse + bres, err := txcmd.SignAndPostTx(stx) if err != nil { return err } // Output result - return txcmd.OutputTx(res) + return txcmd.OutputTx(bres) } -func getAppData() (name string, data []byte, err error) { - countFee, err := btypes.ParseCoins(viper.GetString(flagCountFee)) +func readCounterTxFlags() (tx basecoin.Tx, err error) { + feeCoins, err := btypes.ParseCoins(viper.GetString(FlagCountFee)) if err != nil { - return - } - ctx := counter.CounterTx{ - Valid: viper.GetBool(flagValid), - Fee: countFee, + return tx, err } - name = counter.New().Name() - data = wire.BinaryBytes(ctx) - return + tx = counter.NewCounterTx(viper.GetBool(FlagValid), feeCoins) + return tx, nil } diff --git a/docs/guide/counter/cmd/countercli/commands/query.go b/docs/guide/counter/cmd/countercli/commands/query.go index 692e04c7b3..0d5febfe9e 100644 --- a/docs/guide/counter/cmd/countercli/commands/query.go +++ b/docs/guide/counter/cmd/countercli/commands/query.go @@ -16,9 +16,9 @@ var CounterQueryCmd = &cobra.Command{ } func counterQueryCmd(cmd *cobra.Command, args []string) error { - key := counter.New().StateKey() + key := counter.StateKey() - var cp counter.CounterPluginState + var cp counter.CounterState proof, err := proofcmd.GetAndParseAppProof(key, &cp) if err != nil { return err diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 71050862e2..aec7775b76 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -9,6 +9,7 @@ import ( "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/errors" "github.com/tendermint/basecoin/modules/coin" + "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/types" ) @@ -78,6 +79,17 @@ func ErrDecoding() error { // CounterHandler //-------------------------------------------------------------------------------- +func NewCounterHandler() basecoin.Handler { + // use the default stack + coin := coin.NewHandler() + counter := CounterHandler{} + dispatcher := stack.NewDispatcher( + stack.WrapHandler(coin), + stack.WrapHandler(counter), + ) + return stack.NewDefault().Use(dispatcher) +} + type CounterHandler struct { basecoin.NopOption } @@ -107,6 +119,7 @@ func (h CounterHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx } // TODO: handle coin movement.... ugh, need sequence to do this, right? + // like, actually decrement the other account // update the counter state, err := LoadState(store) @@ -135,16 +148,16 @@ func checkTx(ctx basecoin.Context, tx basecoin.Tx) (ctr CounterTx, err error) { // CounterStore //-------------------------------------------------------------------------------- -type CounterPluginState struct { - Counter int - TotalFees types.Coins +type CounterState struct { + Counter int `json:"counter"` + TotalFees types.Coins `json:"total_fees"` } func StateKey() []byte { return []byte(NameCounter + "/state") } -func LoadState(store types.KVStore) (state CounterPluginState, err error) { +func LoadState(store types.KVStore) (state CounterState, err error) { bytes := store.Get(StateKey()) if len(bytes) > 0 { err = wire.ReadBinaryBytes(bytes, &state) @@ -155,7 +168,7 @@ func LoadState(store types.KVStore) (state CounterPluginState, err error) { return state, nil } -func StoreState(store types.KVStore, state CounterPluginState) error { +func StoreState(store types.KVStore, state CounterState) error { bytes := wire.BinaryBytes(state) store.Set(StateKey(), bytes) return nil diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index 3fa84849a3..e3e8793da7 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -7,10 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/abci/types" - "github.com/tendermint/basecoin" "github.com/tendermint/basecoin/app" - "github.com/tendermint/basecoin/modules/coin" - "github.com/tendermint/basecoin/stack" "github.com/tendermint/basecoin/txs" "github.com/tendermint/basecoin/types" "github.com/tendermint/go-wire" @@ -18,18 +15,6 @@ import ( "github.com/tendermint/tmlibs/log" ) -// TODO: actually handle the counter here... -func NewCounterHandler() basecoin.Handler { - // use the default stack - coin := coin.NewHandler() - counter := CounterHandler{} - dispatcher := stack.NewDispatcher( - stack.WrapHandler(coin), - stack.WrapHandler(counter), - ) - return stack.NewDefault().Use(dispatcher) -} - func TestCounterPlugin(t *testing.T) { assert := assert.New(t) diff --git a/tests/cli/counter.sh b/tests/cli/counter.sh index 02b8ac44d6..05a5858dd1 100755 --- a/tests/cli/counter.sh +++ b/tests/cli/counter.sh @@ -55,21 +55,22 @@ test02GetCounter() { checkCounter() { # make sure sender goes down ACCT=$(${CLIENT_EXE} query counter) - assertTrue "count is set" $? - assertEquals "proper count" "$1" $(echo $ACCT | jq .data.Counter) - assertEquals "proper money" "$2" $(echo $ACCT | jq .data.TotalFees[0].amount) + if assertTrue "count is set" $?; then + assertEquals "proper count" "$1" $(echo $ACCT | jq .data.counter) + assertEquals "proper money" "$2" $(echo $ACCT | jq .data.total_fees[0].amount) + fi } test03AddCount() { SENDER=$(getAddr $RICH) - assertFalse "bad password" "echo hi | ${CLIENT_EXE} tx counter --amount=1000mycoin --sequence=2 --name=${RICH} 2>/dev/null" + assertFalse "bad password" "echo hi | ${CLIENT_EXE} tx counter --countfee=100mycoin --sequence=2 --name=${RICH} 2>/dev/null" - TX=$(echo qwertyuiop | ${CLIENT_EXE} tx counter --amount=10mycoin --sequence=2 --name=${RICH} --valid --countfee=5mycoin) + TX=$(echo qwertyuiop | ${CLIENT_EXE} tx counter --countfee=10mycoin --sequence=2 --name=${RICH} --valid) txSucceeded $? "$TX" "counter" HASH=$(echo $TX | jq .hash | tr -d \") TX_HEIGHT=$(echo $TX | jq .height) - checkCounter "1" "5" + checkCounter "1" "10" # FIXME: cannot load apptx properly. # Look at the stack trace diff --git a/tests/tmsp/tmsp_test.go b/tests/tmsp/tmsp_test.go index 9de055ad65..f08b626008 100644 --- a/tests/tmsp/tmsp_test.go +++ b/tests/tmsp/tmsp_test.go @@ -1,157 +1,159 @@ package tmsp_test -import ( - "encoding/json" - "testing" +// TODO: replace with benchmarker - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/tendermint/basecoin/app" - "github.com/tendermint/basecoin/types" - wire "github.com/tendermint/go-wire" - eyescli "github.com/tendermint/merkleeyes/client" - cmn "github.com/tendermint/tmlibs/common" - "github.com/tendermint/tmlibs/log" -) +// import ( +// "encoding/json" +// "testing" -func TestSendTx(t *testing.T) { - eyesCli := eyescli.NewLocalClient("", 0) - chainID := "test_chain_id" - bcApp := app.NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) - bcApp.SetOption("base/chain_id", chainID) - // t.Log(bcApp.Info()) +// "github.com/stretchr/testify/assert" +// "github.com/stretchr/testify/require" +// "github.com/tendermint/basecoin/app" +// "github.com/tendermint/basecoin/types" +// wire "github.com/tendermint/go-wire" +// eyescli "github.com/tendermint/merkleeyes/client" +// cmn "github.com/tendermint/tmlibs/common" +// "github.com/tendermint/tmlibs/log" +// ) - test1PrivAcc := types.PrivAccountFromSecret("test1") - test2PrivAcc := types.PrivAccountFromSecret("test2") +// func TestSendTx(t *testing.T) { +// eyesCli := eyescli.NewLocalClient("", 0) +// chainID := "test_chain_id" +// bcApp := app.NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) +// bcApp.SetOption("base/chain_id", chainID) +// // t.Log(bcApp.Info()) - // Seed Basecoin with account - test1Acc := test1PrivAcc.Account - test1Acc.Balance = types.Coins{{"", 1000}} - accOpt, err := json.Marshal(test1Acc) - require.Nil(t, err) - bcApp.SetOption("base/account", string(accOpt)) +// test1PrivAcc := types.PrivAccountFromSecret("test1") +// test2PrivAcc := types.PrivAccountFromSecret("test2") - // Construct a SendTx signature - tx := &types.SendTx{ - Gas: 0, - Fee: types.Coin{"", 0}, - Inputs: []types.TxInput{ - types.NewTxInput(test1PrivAcc.Account.PubKey, types.Coins{{"", 1}}, 1), - }, - Outputs: []types.TxOutput{ - types.TxOutput{ - Address: test2PrivAcc.Account.PubKey.Address(), - Coins: types.Coins{{"", 1}}, - }, - }, - } +// // Seed Basecoin with account +// test1Acc := test1PrivAcc.Account +// test1Acc.Balance = types.Coins{{"", 1000}} +// accOpt, err := json.Marshal(test1Acc) +// require.Nil(t, err) +// bcApp.SetOption("base/account", string(accOpt)) - // Sign request - signBytes := tx.SignBytes(chainID) - // t.Log("Sign bytes: %X\n", signBytes) - sig := test1PrivAcc.Sign(signBytes) - tx.Inputs[0].Signature = sig - // t.Log("Signed TX bytes: %X\n", wire.BinaryBytes(types.TxS{tx})) +// // Construct a SendTx signature +// tx := &types.SendTx{ +// Gas: 0, +// Fee: types.Coin{"", 0}, +// Inputs: []types.TxInput{ +// types.NewTxInput(test1PrivAcc.Account.PubKey, types.Coins{{"", 1}}, 1), +// }, +// Outputs: []types.TxOutput{ +// types.TxOutput{ +// Address: test2PrivAcc.Account.PubKey.Address(), +// Coins: types.Coins{{"", 1}}, +// }, +// }, +// } - // Write request - txBytes := wire.BinaryBytes(types.TxS{tx}) - res := bcApp.DeliverTx(txBytes) - // t.Log(res) - assert.True(t, res.IsOK(), "Failed: %v", res.Error()) -} +// // Sign request +// signBytes := tx.SignBytes(chainID) +// // t.Log("Sign bytes: %X\n", signBytes) +// sig := test1PrivAcc.Sign(signBytes) +// tx.Inputs[0].Signature = sig +// // t.Log("Signed TX bytes: %X\n", wire.BinaryBytes(types.TxS{tx})) -func TestSequence(t *testing.T) { - eyesCli := eyescli.NewLocalClient("", 0) - chainID := "test_chain_id" - bcApp := app.NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) - bcApp.SetOption("base/chain_id", chainID) - // t.Log(bcApp.Info()) +// // Write request +// txBytes := wire.BinaryBytes(types.TxS{tx}) +// res := bcApp.DeliverTx(txBytes) +// // t.Log(res) +// assert.True(t, res.IsOK(), "Failed: %v", res.Error()) +// } - // Get the test account - test1PrivAcc := types.PrivAccountFromSecret("test1") - test1Acc := test1PrivAcc.Account - test1Acc.Balance = types.Coins{{"", 1 << 53}} - accOpt, err := json.Marshal(test1Acc) - require.Nil(t, err) - bcApp.SetOption("base/account", string(accOpt)) +// func TestSequence(t *testing.T) { +// eyesCli := eyescli.NewLocalClient("", 0) +// chainID := "test_chain_id" +// bcApp := app.NewBasecoin(eyesCli, log.TestingLogger().With("module", "app")) +// bcApp.SetOption("base/chain_id", chainID) +// // t.Log(bcApp.Info()) - sequence := int(1) - // Make a bunch of PrivAccounts - privAccounts := types.RandAccounts(1000, 1000000, 0) - privAccountSequences := make(map[string]int) - // Send coins to each account +// // Get the test account +// test1PrivAcc := types.PrivAccountFromSecret("test1") +// test1Acc := test1PrivAcc.Account +// test1Acc.Balance = types.Coins{{"", 1 << 53}} +// accOpt, err := json.Marshal(test1Acc) +// require.Nil(t, err) +// bcApp.SetOption("base/account", string(accOpt)) - for i := 0; i < len(privAccounts); i++ { - privAccount := privAccounts[i] +// sequence := int(1) +// // Make a bunch of PrivAccounts +// privAccounts := types.RandAccounts(1000, 1000000, 0) +// privAccountSequences := make(map[string]int) +// // Send coins to each account - tx := &types.SendTx{ - Gas: 2, - Fee: types.Coin{"", 2}, - Inputs: []types.TxInput{ - types.NewTxInput(test1Acc.PubKey, types.Coins{{"", 1000002}}, sequence), - }, - Outputs: []types.TxOutput{ - types.TxOutput{ - Address: privAccount.Account.PubKey.Address(), - Coins: types.Coins{{"", 1000000}}, - }, - }, - } - sequence += 1 +// for i := 0; i < len(privAccounts); i++ { +// privAccount := privAccounts[i] - // Sign request - signBytes := tx.SignBytes(chainID) - sig := test1PrivAcc.Sign(signBytes) - tx.Inputs[0].Signature = sig - // t.Log("ADDR: %X -> %X\n", tx.Inputs[0].Address, tx.Outputs[0].Address) +// tx := &types.SendTx{ +// Gas: 2, +// Fee: types.Coin{"", 2}, +// Inputs: []types.TxInput{ +// types.NewTxInput(test1Acc.PubKey, types.Coins{{"", 1000002}}, sequence), +// }, +// Outputs: []types.TxOutput{ +// types.TxOutput{ +// Address: privAccount.Account.PubKey.Address(), +// Coins: types.Coins{{"", 1000000}}, +// }, +// }, +// } +// sequence += 1 - // Write request - txBytes := wire.BinaryBytes(struct{ types.Tx }{tx}) - res := bcApp.DeliverTx(txBytes) - assert.True(t, res.IsOK(), "DeliverTx error: %v", res.Error()) - } +// // Sign request +// signBytes := tx.SignBytes(chainID) +// sig := test1PrivAcc.Sign(signBytes) +// tx.Inputs[0].Signature = sig +// // t.Log("ADDR: %X -> %X\n", tx.Inputs[0].Address, tx.Outputs[0].Address) - res := bcApp.Commit() - assert.True(t, res.IsOK(), "Failed Commit: %v", res.Error()) +// // Write request +// txBytes := wire.BinaryBytes(struct{ types.Tx }{tx}) +// res := bcApp.DeliverTx(txBytes) +// assert.True(t, res.IsOK(), "DeliverTx error: %v", res.Error()) +// } - t.Log("-------------------- RANDOM SENDS --------------------") +// res := bcApp.Commit() +// assert.True(t, res.IsOK(), "Failed Commit: %v", res.Error()) - // Now send coins between these accounts - for i := 0; i < 10000; i++ { - randA := cmn.RandInt() % len(privAccounts) - randB := cmn.RandInt() % len(privAccounts) - if randA == randB { - continue - } +// t.Log("-------------------- RANDOM SENDS --------------------") - privAccountA := privAccounts[randA] - privAccountASequence := privAccountSequences[privAccountA.Account.PubKey.KeyString()] - privAccountSequences[privAccountA.Account.PubKey.KeyString()] = privAccountASequence + 1 - privAccountB := privAccounts[randB] +// // Now send coins between these accounts +// for i := 0; i < 10000; i++ { +// randA := cmn.RandInt() % len(privAccounts) +// randB := cmn.RandInt() % len(privAccounts) +// if randA == randB { +// continue +// } - tx := &types.SendTx{ - Gas: 2, - Fee: types.Coin{"", 2}, - Inputs: []types.TxInput{ - types.NewTxInput(privAccountA.PubKey, types.Coins{{"", 3}}, privAccountASequence+1), - }, - Outputs: []types.TxOutput{ - types.TxOutput{ - Address: privAccountB.PubKey.Address(), - Coins: types.Coins{{"", 1}}, - }, - }, - } +// privAccountA := privAccounts[randA] +// privAccountASequence := privAccountSequences[privAccountA.Account.PubKey.KeyString()] +// privAccountSequences[privAccountA.Account.PubKey.KeyString()] = privAccountASequence + 1 +// privAccountB := privAccounts[randB] - // Sign request - signBytes := tx.SignBytes(chainID) - sig := privAccountA.Sign(signBytes) - tx.Inputs[0].Signature = sig - // t.Log("ADDR: %X -> %X\n", tx.Inputs[0].Address, tx.Outputs[0].Address) +// tx := &types.SendTx{ +// Gas: 2, +// Fee: types.Coin{"", 2}, +// Inputs: []types.TxInput{ +// types.NewTxInput(privAccountA.PubKey, types.Coins{{"", 3}}, privAccountASequence+1), +// }, +// Outputs: []types.TxOutput{ +// types.TxOutput{ +// Address: privAccountB.PubKey.Address(), +// Coins: types.Coins{{"", 1}}, +// }, +// }, +// } - // Write request - txBytes := wire.BinaryBytes(struct{ types.Tx }{tx}) - res := bcApp.DeliverTx(txBytes) - assert.True(t, res.IsOK(), "DeliverTx error: %v", res.Error()) - } -} +// // Sign request +// signBytes := tx.SignBytes(chainID) +// sig := privAccountA.Sign(signBytes) +// tx.Inputs[0].Signature = sig +// // t.Log("ADDR: %X -> %X\n", tx.Inputs[0].Address, tx.Outputs[0].Address) + +// // Write request +// txBytes := wire.BinaryBytes(struct{ types.Tx }{tx}) +// res := bcApp.DeliverTx(txBytes) +// assert.True(t, res.IsOK(), "DeliverTx error: %v", res.Error()) +// } +// } From c6c5e34c3a02ff8eba0b52c703fe0e5fa7bed630 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 4 Jul 2017 14:10:06 +0200 Subject: [PATCH 15/24] Fix shell-down tutorials --- docs/guide/basecoin-basics.md | 28 ++++++++++++++-------------- docs/guide/basecoin-plugins.md | 32 ++++++++++++++++---------------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/docs/guide/basecoin-basics.md b/docs/guide/basecoin-basics.md index 48278986eb..f5e792e1bd 100644 --- a/docs/guide/basecoin-basics.md +++ b/docs/guide/basecoin-basics.md @@ -2,47 +2,47 @@ #!/bin/bash testTutorial_BasecoinBasics() { - - #shelldown[1][3] >/dev/null - #shelldown[1][4] >/dev/null + + #shelldown[1][3] >/dev/null + #shelldown[1][4] >/dev/null KEYPASS=qwertyuiop - + RES=$((echo $KEYPASS; echo $KEYPASS) | #shelldown[1][6]) assertTrue "Line $LINENO: Expected to contain safe, got $RES" '[[ $RES == *safe* ]]' RES=$((echo $KEYPASS; echo $KEYPASS) | #shelldown[1][7]) assertTrue "Line $LINENO: Expected to contain safe, got $RES" '[[ $RES == *safe* ]]' - + #shelldown[3][-1] assertTrue "Expected true for line $LINENO" $? - + #shelldown[4][-1] >>/dev/null 2>&1 & sleep 5 PID_SERVER=$! disown - + RES=$((echo y) | #shelldown[5][-1] $1) assertTrue "Line $LINENO: Expected to contain validator, got $RES" '[[ $RES == *validator* ]]' - + #shelldown[6][0] #shelldown[6][1] RES=$(#shelldown[6][2] | jq '.data.coins[0].denom' | tr -d '"') assertTrue "Line $LINENO: Expected to have mycoins, got $RES" '[[ $RES == mycoin ]]' RES="$(#shelldown[6][3] 2>&1)" assertTrue "Line $LINENO: Expected to contain ERROR, got $RES" '[[ $RES == *ERROR* ]]' - + RES=$((echo $KEYPASS) | #shelldown[7][-1] | jq '.deliver_tx.code') assertTrue "Line $LINENO: Expected 0 code deliver_tx, got $RES" '[[ $RES == 0 ]]' - + RES=$(#shelldown[8][-1] | jq '.data.coins[0].amount') assertTrue "Line $LINENO: Expected to contain 1000 mycoin, got $RES" '[[ $RES == 1000 ]]' - + RES=$((echo $KEYPASS) | #shelldown[9][-1] | jq '.deliver_tx.code') assertTrue "Line $LINENO: Expected 0 code deliver_tx, got $RES" '[[ $RES == 0 ]]' - + RES=$((echo $KEYPASS) | #shelldown[10][-1]) assertTrue "Line $LINENO: Expected to contain insufficient funds error, got $RES" \ - '[[ $RES == *"insufficient funds"* ]]' - + '[[ $RES == *"Insufficient Funds"* ]]' + #perform a substitution within the final tests HASH=$((echo $KEYPASS) | #shelldown[11][-1] | jq '.hash' | tr -d '"') PRESUB="#shelldown[12][-1]" diff --git a/docs/guide/basecoin-plugins.md b/docs/guide/basecoin-plugins.md index 2ca621c52b..ad9ed615a9 100644 --- a/docs/guide/basecoin-plugins.md +++ b/docs/guide/basecoin-plugins.md @@ -2,49 +2,49 @@ #!/bin/bash testTutorial_BasecoinPlugins() { - + #Initialization #shelldown[0][1] #shelldown[0][2] KEYPASS=qwertyuiop - - #Making Keys + + #Making Keys RES=$((echo $KEYPASS; echo $KEYPASS) | #shelldown[0][4]) assertTrue "Line $LINENO: Expected to contain safe, got $RES" '[[ $RES == *safe* ]]' RES=$((echo $KEYPASS; echo $KEYPASS) | #shelldown[0][5]) assertTrue "Line $LINENO: Expected to contain safe, got $RES" '[[ $RES == *safe* ]]' - + #shelldown[0][7] >/dev/null assertTrue "Expected true for line $LINENO" $? - + #shelldown[0][9] >>/dev/null 2>&1 & sleep 5 PID_SERVER=$! disown - + RES=$((echo y) | #shelldown[1][0] $1) assertTrue "Line $LINENO: Expected to contain validator, got $RES" '[[ $RES == *validator* ]]' - + #shelldown[1][2] assertTrue "Expected true for line $LINENO" $? RES=$((echo $KEYPASS) | #shelldown[1][3] | jq '.deliver_tx.code') assertTrue "Line $LINENO: Expected 0 code deliver_tx, got $RES" '[[ $RES == 0 ]]' - + RES=$((echo $KEYPASS) | #shelldown[2][0]) assertTrue "Line $LINENO: Expected to contain Valid error, got $RES" \ - '[[ $RES == *"Valid must be true"* ]]' - + '[[ $RES == *"Counter Tx marked invalid"* ]]' + RES=$((echo $KEYPASS) | #shelldown[2][1] | jq '.deliver_tx.code') assertTrue "Line $LINENO: Expected 0 code deliver_tx, got $RES" '[[ $RES == 0 ]]' - RES=$(#shelldown[3][-1] | jq '.data.Counter') + RES=$(#shelldown[3][-1] | jq '.data.counter') assertTrue "Line $LINENO: Expected Counter of 1, got $RES" '[[ $RES == 1 ]]' RES=$((echo $KEYPASS) | #shelldown[4][0] | jq '.deliver_tx.code') assertTrue "Line $LINENO: Expected 0 code deliver_tx, got $RES" '[[ $RES == 0 ]]' RES=$(#shelldown[4][1]) - RESCOUNT=$(printf "$RES" | jq '.data.Counter') - RESFEE=$(printf "$RES" | jq '.data.TotalFees[0].amount') + RESCOUNT=$(printf "$RES" | jq '.data.counter') + RESFEE=$(printf "$RES" | jq '.data.total_fees[0].amount') assertTrue "Line $LINENO: Expected Counter of 2, got $RES" '[[ $RESCOUNT == 2 ]]' assertTrue "Line $LINENO: Expected TotalFees of 2, got $RES" '[[ $RESFEE == 2 ]]' } @@ -113,8 +113,8 @@ But the Counter has an additional command, `countercli tx counter`, which crafts an `AppTx` specifically for this plugin: ```shelldown[2] -countercli tx counter --name cool --amount=1mycoin --sequence=2 -countercli tx counter --name cool --amount=1mycoin --sequence=3 --valid +countercli tx counter --name cool --sequence=2 +countercli tx counter --name cool --sequence=3 --valid ``` The first transaction is rejected by the plugin because it was not marked as @@ -132,7 +132,7 @@ If we send another transaction, and then query again, we will see the value increment: ```shelldown[4] -countercli tx counter --name cool --amount=2mycoin --sequence=4 --valid --countfee=2mycoin +countercli tx counter --name cool --countfee=2mycoin --sequence=4 --valid countercli query counter ``` From 8003034bbb37c41c42e1be82d079ebb593eacefc Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 4 Jul 2017 14:19:28 +0200 Subject: [PATCH 16/24] Test reading back counter tx --- docs/guide/counter/plugins/counter/counter.go | 4 ++-- tests/cli/counter.sh | 24 ++++++++----------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index aec7775b76..1cdc31a9bc 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -30,8 +30,8 @@ func init() { } type CounterTx struct { - Valid bool - Fee types.Coins + Valid bool `json:"valid"` + Fee types.Coins `json:"fee"` } func NewCounterTx(valid bool, fee types.Coins) basecoin.Tx { diff --git a/tests/cli/counter.sh b/tests/cli/counter.sh index 05a5858dd1..72ebbc9df7 100755 --- a/tests/cli/counter.sh +++ b/tests/cli/counter.sh @@ -72,21 +72,17 @@ test03AddCount() { checkCounter "1" "10" - # FIXME: cannot load apptx properly. - # Look at the stack trace - # This cannot be fixed with the current ugly apptx structure... - # Leave for refactoring - # make sure tx is indexed - # echo hash $HASH - # TX=$(${CLIENT_EXE} query tx $HASH --trace) - # echo tx $TX - # if [-z assertTrue "found tx" $?]; then - # assertEquals "proper height" $TX_HEIGHT $(echo $TX | jq .height) - # assertEquals "type=app" '"app"' $(echo $TX | jq .data.type) - # assertEquals "proper sender" "\"$SENDER\"" $(echo $TX | jq .data.data.input.address) - # fi - # echo $TX + TX=$(${CLIENT_EXE} query tx $HASH --trace) + if assertTrue "found tx" $?; then + assertEquals "proper height" $TX_HEIGHT $(echo $TX | jq .height) + assertEquals "type=sig" '"sig"' $(echo $TX | jq .data.type) + CTX=$(echo $TX | jq .data.data.tx) + assertEquals "type=chain" '"chain"' $(echo $CTX | jq .type) + CNTX=$(echo $CTX | jq .data.tx) + assertEquals "type=cntr/count" '"cntr/count"' $(echo $CNTX | jq .type) + assertEquals "proper fee" "10" $(echo $CNTX | jq .data.fee[0].amount) + fi } # Load common then run these tests with shunit2! From 670e7b48d1a0b1dbe7e163302a5c7e8a18b345e7 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 4 Jul 2017 14:47:46 +0200 Subject: [PATCH 17/24] Counter uses dispatcher to deduct fees from account --- context.go | 1 + .../cmd/countercli/commands/counter.go | 2 +- docs/guide/counter/plugins/counter/counter.go | 46 ++++++++++++++----- .../counter/plugins/counter/counter_test.go | 25 ++++++---- stack/context.go | 11 +++++ stack/dispatcher.go | 15 +++--- stack/interface.go | 6 +++ stack/mock.go | 11 +++++ tests/cli/counter.sh | 4 ++ 9 files changed, 92 insertions(+), 29 deletions(-) diff --git a/context.go b/context.go index dbbe7b6d92..66ba9c5d3e 100644 --- a/context.go +++ b/context.go @@ -32,6 +32,7 @@ type Context interface { log.Logger WithPermissions(perms ...Actor) Context HasPermission(perm Actor) bool + GetPermissions(chain, app string) []Actor IsParent(ctx Context) bool Reset() Context ChainID() string diff --git a/docs/guide/counter/cmd/countercli/commands/counter.go b/docs/guide/counter/cmd/countercli/commands/counter.go index 3c388c8975..eab32acf1b 100644 --- a/docs/guide/counter/cmd/countercli/commands/counter.go +++ b/docs/guide/counter/cmd/countercli/commands/counter.go @@ -74,6 +74,6 @@ func readCounterTxFlags() (tx basecoin.Tx, err error) { return tx, err } - tx = counter.NewCounterTx(viper.GetBool(FlagValid), feeCoins) + tx = counter.NewCounterTx(viper.GetBool(FlagValid), feeCoins, viper.GetInt(FlagSequence)) return tx, nil } diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 1cdc31a9bc..96d409c754 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -30,14 +30,16 @@ func init() { } type CounterTx struct { - Valid bool `json:"valid"` - Fee types.Coins `json:"fee"` + Valid bool `json:"valid"` + Fee types.Coins `json:"fee"` + Sequence int `json:"sequence"` } -func NewCounterTx(valid bool, fee types.Coins) basecoin.Tx { +func NewCounterTx(valid bool, fee types.Coins, sequence int) basecoin.Tx { return CounterTx{ - Valid: valid, - Fee: fee, + Valid: valid, + Fee: fee, + Sequence: sequence, }.Wrap() } @@ -85,29 +87,31 @@ func NewCounterHandler() basecoin.Handler { counter := CounterHandler{} dispatcher := stack.NewDispatcher( stack.WrapHandler(coin), - stack.WrapHandler(counter), + counter, ) return stack.NewDefault().Use(dispatcher) } type CounterHandler struct { - basecoin.NopOption + stack.NopOption } -var _ basecoin.Handler = CounterHandler{} +var _ stack.Dispatchable = CounterHandler{} func (_ CounterHandler) Name() string { return NameCounter } +func (_ CounterHandler) AssertDispatcher() {} + // CheckTx checks if the tx is properly structured -func (h CounterHandler) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { +func (h CounterHandler) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, _ basecoin.Checker) (res basecoin.Result, err error) { _, err = checkTx(ctx, tx) return } // DeliverTx executes the tx if valid -func (h CounterHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { +func (h CounterHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, dispatch basecoin.Deliver) (res basecoin.Result, err error) { ctr, err := checkTx(ctx, tx) if err != nil { return res, err @@ -118,8 +122,22 @@ func (h CounterHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx return res, ErrInvalidCounter() } - // TODO: handle coin movement.... ugh, need sequence to do this, right? - // like, actually decrement the other account + // handle coin movement.... like, actually decrement the other account + if !ctr.Fee.IsZero() { + // take the coins and put them in out account! + senders := ctx.GetPermissions("", stack.NameSigs) + if len(senders) == 0 { + return res, errors.ErrMissingSignature() + } + in := []coin.TxInput{{Address: senders[0], Coins: ctr.Fee, Sequence: ctr.Sequence}} + out := []coin.TxOutput{{Address: CounterAcct(), Coins: ctr.Fee}} + send := coin.NewSendTx(in, out) + // if the deduction fails (too high), abort the command + _, err = dispatch.DeliverTx(ctx, store, send) + if err != nil { + return res, err + } + } // update the counter state, err := LoadState(store) @@ -148,6 +166,10 @@ func checkTx(ctx basecoin.Context, tx basecoin.Tx) (ctr CounterTx, err error) { // CounterStore //-------------------------------------------------------------------------------- +func CounterAcct() basecoin.Actor { + return basecoin.Actor{App: NameCounter, Address: []byte{0x04, 0x20}} +} + type CounterState struct { Counter int `json:"counter"` TotalFees types.Coins `json:"total_fees"` diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index e3e8793da7..8b41fcaec4 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -2,6 +2,7 @@ package counter import ( "encoding/json" + "os" "testing" "github.com/stretchr/testify/assert" @@ -21,10 +22,14 @@ func TestCounterPlugin(t *testing.T) { // Basecoin initialization eyesCli := eyescli.NewLocalClient("", 0) chainID := "test_chain_id" + + // l := log.TestingLogger().With("module", "app"), + l := log.NewTMLogger(os.Stdout).With("module", "app") + // l = log.NewTracingLogger(l) bcApp := app.NewBasecoin( NewCounterHandler(), eyesCli, - log.TestingLogger().With("module", "app"), + l, ) bcApp.SetOption("base/chain_id", chainID) // t.Log(bcApp.Info()) @@ -42,7 +47,7 @@ func TestCounterPlugin(t *testing.T) { // Deliver a CounterTx DeliverCounterTx := func(valid bool, counterFee types.Coins, inputSequence int) abci.Result { - tx := NewCounterTx(valid, counterFee) + tx := NewCounterTx(valid, counterFee, inputSequence) tx = txs.NewChain(chainID, tx) stx := txs.NewSig(tx) txs.Sign(stx, test1PrivAcc.PrivKey) @@ -50,19 +55,19 @@ func TestCounterPlugin(t *testing.T) { return bcApp.DeliverTx(txBytes) } - // Test a basic send, no fee - res := DeliverCounterTx(true, types.Coins{}, 1) + // Test a basic send, no fee (doesn't update sequence as no money spent) + res := DeliverCounterTx(true, nil, 1) assert.True(res.IsOK(), res.String()) // Test an invalid send, no fee - res = DeliverCounterTx(false, types.Coins{}, 1) + res = DeliverCounterTx(false, nil, 1) assert.True(res.IsErr(), res.String()) - // Test the fee - res = DeliverCounterTx(true, types.Coins{{"gold", 100}}, 2) + // Test the fee (increments sequence) + res = DeliverCounterTx(true, types.Coins{{"gold", 100}}, 1) assert.True(res.IsOK(), res.String()) - // TODO: Test unsupported fee - // res = DeliverCounterTx(true, types.Coins{{"silver", 100}}, 3) - // assert.True(res.IsErr(), res.String()) + // Test unsupported fee + res = DeliverCounterTx(true, types.Coins{{"silver", 100}}, 2) + assert.True(res.IsErr(), res.String()) } diff --git a/stack/context.go b/stack/context.go index 6124a96780..80ad743901 100644 --- a/stack/context.go +++ b/stack/context.go @@ -66,6 +66,17 @@ func (c secureContext) HasPermission(perm basecoin.Actor) bool { return false } +func (c secureContext) GetPermissions(chain, app string) (res []basecoin.Actor) { + for _, p := range c.perms { + if chain == p.ChainID { + if app == "" || app == p.App { + res = append(res, p) + } + } + } + return res +} + // IsParent ensures that this is derived from the given secureClient func (c secureContext) IsParent(other basecoin.Context) bool { so, ok := other.(secureContext) diff --git a/stack/dispatcher.go b/stack/dispatcher.go index 6e177ca098..5da79f7a04 100644 --- a/stack/dispatcher.go +++ b/stack/dispatcher.go @@ -56,8 +56,9 @@ func (d *Dispatcher) CheckTx(ctx basecoin.Context, store types.KVStore, tx basec if err != nil { return res, err } - // TODO: callback - return r.CheckTx(ctx, store, tx, nil) + // TODO: check on callback + cb := d + return r.CheckTx(ctx, store, tx, cb) } func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { @@ -65,8 +66,9 @@ func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store types.KVStore, tx bas if err != nil { return res, err } - // TODO: callback - return r.DeliverTx(ctx, store, tx, nil) + // TODO: check on callback + cb := d + return r.DeliverTx(ctx, store, tx, cb) } func (d *Dispatcher) SetOption(l log.Logger, store types.KVStore, module, key, value string) (string, error) { @@ -74,8 +76,9 @@ func (d *Dispatcher) SetOption(l log.Logger, store types.KVStore, module, key, v if err != nil { return "", err } - // TODO: callback - return r.SetOption(l, store, module, key, value, nil) + // TODO: check on callback + cb := d + return r.SetOption(l, store, module, key, value, cb) } func (d *Dispatcher) lookupTx(tx basecoin.Tx) (Dispatchable, error) { diff --git a/stack/interface.go b/stack/interface.go index 09e1ad4d16..3b5b5d4e61 100644 --- a/stack/interface.go +++ b/stack/interface.go @@ -67,6 +67,12 @@ func (_ PassOption) SetOption(l log.Logger, store types.KVStore, module, key, va return next.SetOption(l, store, module, key, value) } +type NopOption struct{} + +func (_ NopOption) SetOption(l log.Logger, store types.KVStore, module, key, value string, next basecoin.SetOptioner) (string, error) { + return "", nil +} + // Dispatchable is like middleware, except the meaning of "next" is different. // Whereas in the middleware, it is the next handler that we should pass the same tx into, // for dispatchers, it is a dispatcher, which it can use to diff --git a/stack/mock.go b/stack/mock.go index 443f83dfd8..781f13d4fb 100644 --- a/stack/mock.go +++ b/stack/mock.go @@ -44,6 +44,17 @@ func (c mockContext) HasPermission(perm basecoin.Actor) bool { return false } +func (c mockContext) GetPermissions(chain, app string) (res []basecoin.Actor) { + for _, p := range c.perms { + if chain == p.ChainID { + if app == "" || app == p.App { + res = append(res, p) + } + } + } + return res +} + // IsParent ensures that this is derived from the given secureClient func (c mockContext) IsParent(other basecoin.Context) bool { _, ok := other.(mockContext) diff --git a/tests/cli/counter.sh b/tests/cli/counter.sh index 72ebbc9df7..e4d7352f32 100755 --- a/tests/cli/counter.sh +++ b/tests/cli/counter.sh @@ -70,8 +70,12 @@ test03AddCount() { HASH=$(echo $TX | jq .hash | tr -d \") TX_HEIGHT=$(echo $TX | jq .height) + # make sure the counter was updated checkCounter "1" "10" + # make sure the account was debited + checkAccount $SENDER "2" "9007199254739990" + # make sure tx is indexed TX=$(${CLIENT_EXE} query tx $HASH --trace) if assertTrue "found tx" $?; then From ba38f442c8d5d4abbc53dccb70e913ec6518e923 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 4 Jul 2017 14:52:21 +0200 Subject: [PATCH 18/24] Update use of --sequence in counter tutorial --- docs/guide/basecoin-plugins.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/guide/basecoin-plugins.md b/docs/guide/basecoin-plugins.md index ad9ed615a9..36655b56bf 100644 --- a/docs/guide/basecoin-plugins.md +++ b/docs/guide/basecoin-plugins.md @@ -113,8 +113,8 @@ But the Counter has an additional command, `countercli tx counter`, which crafts an `AppTx` specifically for this plugin: ```shelldown[2] -countercli tx counter --name cool --sequence=2 -countercli tx counter --name cool --sequence=3 --valid +countercli tx counter --name cool +countercli tx counter --name cool --valid ``` The first transaction is rejected by the plugin because it was not marked as @@ -129,10 +129,11 @@ countercli query counter Tada! We can now see that our custom counter plugin tx went through. You should see a Counter value of 1 representing the number of valid transactions. If we send another transaction, and then query again, we will see the value -increment: +increment. Note that we need the sequence number here to send the coins +(it didn't increment when we just pinged the counter) ```shelldown[4] -countercli tx counter --name cool --countfee=2mycoin --sequence=4 --valid +countercli tx counter --name cool --countfee=2mycoin --sequence=2 --valid countercli query counter ``` From 375fad3becc7166d78159ffa9e2991c047ab831a Mon Sep 17 00:00:00 2001 From: rigel rozanski Date: Tue, 4 Jul 2017 23:28:27 -0400 Subject: [PATCH 19/24] go linting working --- Makefile | 26 +++---- app/app.go | 57 +++++++------- app/genesis.go | 14 ++-- cmd/basecli/commands/auto.go | 2 + cmd/basecli/commands/query.go | 4 +- docs/guide/counter/cmd/counter/main.go | 2 +- .../cmd/countercli/commands/counter.go | 3 +- .../counter/cmd/countercli/commands/query.go | 4 +- docs/guide/counter/plugins/counter/counter.go | 74 +++++++++++-------- .../counter/plugins/counter/counter_test.go | 10 +-- modules/coin/store.go | 1 + 11 files changed, 111 insertions(+), 86 deletions(-) diff --git a/Makefile b/Makefile index d2ae8dcddd..4a554e0f89 100644 --- a/Makefile +++ b/Makefile @@ -6,20 +6,20 @@ TUTORIALS=$(shell find docs/guide -name "*md" -type f) all: get_vendor_deps install test build: - go build ./cmd/... + @go build ./cmd/... install: - go install ./cmd/... - go install ./docs/guide/counter/cmd/... + @go install ./cmd/... + @go install ./docs/guide/counter/cmd/... dist: - @bash scripts/dist.sh - @bash scripts/publish.sh + @bash publish/dist.sh + @bash publish/publish.sh test: test_unit test_cli test_tutorial test_unit: - go test `glide novendor` + @go test `glide novendor` #go run tests/tendermint/*.go test_cli: tests/cli/shunit2 @@ -30,26 +30,26 @@ test_cli: tests/cli/shunit2 # @./tests/cli/ibc.sh test_tutorial: docs/guide/shunit2 - shelldown ${TUTORIALS} - for script in docs/guide/*.sh ; do \ + @shelldown ${TUTORIALS} + @for script in docs/guide/*.sh ; do \ bash $$script ; \ done tests/cli/shunit2: - wget "https://raw.githubusercontent.com/kward/shunit2/master/source/2.1/src/shunit2" \ + @wget "https://raw.githubusercontent.com/kward/shunit2/master/source/2.1/src/shunit2" \ -q -O tests/cli/shunit2 docs/guide/shunit2: - wget "https://raw.githubusercontent.com/kward/shunit2/master/source/2.1/src/shunit2" \ + @wget "https://raw.githubusercontent.com/kward/shunit2/master/source/2.1/src/shunit2" \ -q -O docs/guide/shunit2 get_vendor_deps: tools - glide install + @glide install build-docker: - docker run -it --rm -v "$(PWD):/go/src/github.com/tendermint/basecoin" -w \ + @docker run -it --rm -v "$(PWD):/go/src/github.com/tendermint/basecoin" -w \ "/go/src/github.com/tendermint/basecoin" -e "CGO_ENABLED=0" golang:alpine go build ./cmd/basecoin - docker build -t "tendermint/basecoin" . + @docker build -t "tendermint/basecoin" . tools: @go get $(GOTOOLS) diff --git a/app/app.go b/app/app.go index 16d8f6b654..4fb96772f6 100644 --- a/app/app.go +++ b/app/app.go @@ -17,11 +17,13 @@ import ( "github.com/tendermint/basecoin/version" ) +//nolint const ( ModuleNameBase = "base" ChainKey = "chain_id" ) +// Basecoin - The ABCI application type Basecoin struct { eyesCli *eyes.Client state *sm.State @@ -30,19 +32,20 @@ type Basecoin struct { logger log.Logger } -func NewBasecoin(h basecoin.Handler, eyesCli *eyes.Client, l log.Logger) *Basecoin { - state := sm.NewState(eyesCli, l.With("module", "state")) +// NewBasecoin - create a new instance of the basecoin application +func NewBasecoin(handler basecoin.Handler, eyesCli *eyes.Client, logger log.Logger) *Basecoin { + state := sm.NewState(eyesCli, logger.With("module", "state")) return &Basecoin{ - handler: h, + handler: handler, eyesCli: eyesCli, state: state, cacheState: nil, - logger: l, + logger: logger, } } -// placeholder to just handle sendtx +// DefaultHandler - placeholder to just handle sendtx func DefaultHandler() basecoin.Handler { // use the default stack h := coin.NewHandler() @@ -50,47 +53,45 @@ func DefaultHandler() basecoin.Handler { return stack.NewDefault().Use(d) } -// XXX For testing, not thread safe! +// GetState - XXX For testing, not thread safe! func (app *Basecoin) GetState() *sm.State { return app.state.CacheWrap() } -// ABCI::Info +// Info - ABCI func (app *Basecoin) Info() abci.ResponseInfo { resp, err := app.eyesCli.InfoSync() if err != nil { cmn.PanicCrisis(err) } return abci.ResponseInfo{ - Data: cmn.Fmt("Basecoin v%v", version.Version), + Data: fmt.Sprintf("Basecoin v%v", version.Version), LastBlockHeight: resp.LastBlockHeight, LastBlockAppHash: resp.LastBlockAppHash, } } -// ABCI::SetOption +// SetOption - ABCI func (app *Basecoin) SetOption(key string, value string) string { - module, prefix := splitKey(key) + + module, key := splitKey(key) + if module == ModuleNameBase { - return app.setBaseOption(prefix, value) + if key == ChainKey { + app.state.SetChainID(value) + return "Success" + } + return fmt.Sprintf("Error: unknown base option: %s", key) } - log, err := app.handler.SetOption(app.logger, app.state, module, prefix, value) + log, err := app.handler.SetOption(app.logger, app.state, module, key, value) if err == nil { return log } return "Error: " + err.Error() } -func (app *Basecoin) setBaseOption(key, value string) string { - if key == ChainKey { - app.state.SetChainID(value) - return "Success" - } - return fmt.Sprintf("Error: unknown base option: %s", key) -} - -// ABCI::DeliverTx +// DeliverTx - ABCI func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { tx, err := basecoin.LoadTx(txBytes) if err != nil { @@ -114,7 +115,7 @@ func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result { return res.ToABCI() } -// ABCI::CheckTx +// CheckTx - ABCI func (app *Basecoin) CheckTx(txBytes []byte) abci.Result { tx, err := basecoin.LoadTx(txBytes) if err != nil { @@ -136,7 +137,7 @@ func (app *Basecoin) CheckTx(txBytes []byte) abci.Result { return res.ToABCI() } -// ABCI::Query +// Query - ABCI func (app *Basecoin) Query(reqQuery abci.RequestQuery) (resQuery abci.ResponseQuery) { if len(reqQuery.Data) == 0 { resQuery.Log = "Query cannot be zero length" @@ -153,7 +154,7 @@ func (app *Basecoin) Query(reqQuery abci.RequestQuery) (resQuery abci.ResponseQu return } -// ABCI::Commit +// Commit - ABCI func (app *Basecoin) Commit() (res abci.Result) { // Commit state @@ -168,21 +169,21 @@ func (app *Basecoin) Commit() (res abci.Result) { return res } -// ABCI::InitChain +// InitChain - ABCI func (app *Basecoin) InitChain(validators []*abci.Validator) { // for _, plugin := range app.plugins.GetList() { // plugin.InitChain(app.state, validators) // } } -// ABCI::BeginBlock +// BeginBlock - ABCI func (app *Basecoin) BeginBlock(hash []byte, header *abci.Header) { // for _, plugin := range app.plugins.GetList() { // plugin.BeginBlock(app.state, hash, header) // } } -// ABCI::EndBlock +// EndBlock - ABCI func (app *Basecoin) EndBlock(height uint64) (res abci.ResponseEndBlock) { // for _, plugin := range app.plugins.GetList() { // pluginRes := plugin.EndBlock(app.state, height) @@ -191,6 +192,8 @@ func (app *Basecoin) EndBlock(height uint64) (res abci.ResponseEndBlock) { return } +//TODO move split key to tmlibs? + // Splits the string at the first '/'. // if there are none, assign default module ("base"). func splitKey(key string) (string, string) { diff --git a/app/genesis.go b/app/genesis.go index 9d6050bcdb..1549ca7071 100644 --- a/app/genesis.go +++ b/app/genesis.go @@ -8,6 +8,7 @@ import ( cmn "github.com/tendermint/tmlibs/common" ) +// LoadGenesis - Load the genesis json file func (app *Basecoin) LoadGenesis(path string) error { genDoc, err := loadGenesis(path) if err != nil { @@ -35,12 +36,13 @@ type keyValue struct { Value string `json:"value"` } -// includes tendermint (in the json, we ignore here) +// FullGenesisDoc - includes tendermint (in the json, we ignore here) type FullGenesisDoc struct { ChainID string `json:"chain_id"` AppOptions *GenesisDoc `json:"app_options"` } +// GenesisDoc - All genesis values type GenesisDoc struct { Accounts []json.RawMessage `json:"accounts"` PluginOptions []json.RawMessage `json:"plugin_options"` @@ -73,20 +75,20 @@ func loadGenesis(filePath string) (*FullGenesisDoc, error) { return genDoc, nil } -func parseGenesisList(kvz_ []json.RawMessage) (kvz []keyValue, err error) { - if len(kvz_)%2 != 0 { +func parseGenesisList(kvzIn []json.RawMessage) (kvz []keyValue, err error) { + if len(kvzIn)%2 != 0 { return nil, errors.New("genesis cannot have an odd number of items. Format = [key1, value1, key2, value2, ...]") } - for i := 0; i < len(kvz_); i += 2 { + for i := 0; i < len(kvzIn); i += 2 { kv := keyValue{} - rawK := []byte(kvz_[i]) + rawK := []byte(kvzIn[i]) err := json.Unmarshal(rawK, &(kv.Key)) if err != nil { return nil, errors.Errorf("Non-string key: %s", string(rawK)) } // convert value to string if possible (otherwise raw json) - rawV := kvz_[i+1] + rawV := kvzIn[i+1] err = json.Unmarshal(rawV, &(kv.Value)) if err != nil { kv.Value = string(rawV) diff --git a/cmd/basecli/commands/auto.go b/cmd/basecli/commands/auto.go index 4eb7810a2b..040eb6ac38 100644 --- a/cmd/basecli/commands/auto.go +++ b/cmd/basecli/commands/auto.go @@ -7,12 +7,14 @@ import ( "github.com/spf13/viper" ) +// AutoCompleteCmd - command to generate bash autocompletions var AutoCompleteCmd = &cobra.Command{ Use: "complete", Short: "generate bash autocompletions", RunE: doAutoComplete, } +// nolint - flags const ( FlagOutput = "file" ) diff --git a/cmd/basecli/commands/query.go b/cmd/basecli/commands/query.go index 7f3b8db56b..e497c798dd 100644 --- a/cmd/basecli/commands/query.go +++ b/cmd/basecli/commands/query.go @@ -15,6 +15,7 @@ import ( "github.com/tendermint/basecoin/stack" ) +// AccountQueryCmd - command to query an account var AccountQueryCmd = &cobra.Command{ Use: "account [address]", Short: "Get details of an account, with proof", @@ -44,7 +45,8 @@ type BaseTxPresenter struct { proofs.RawPresenter // this handles MakeKey as hex bytes } -func (_ BaseTxPresenter) ParseData(raw []byte) (interface{}, error) { +// ParseData - parse BaseTxPresenter Data +func (b BaseTxPresenter) ParseData(raw []byte) (interface{}, error) { var tx basecoin.Tx err := wire.ReadBinaryBytes(raw, &tx) return tx, err diff --git a/docs/guide/counter/cmd/counter/main.go b/docs/guide/counter/cmd/counter/main.go index 85dafc555e..b7ba4c48ff 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.NewCounterHandler() + commands.Handler = counter.NewHandler() 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 eab32acf1b..0bb39e6abb 100644 --- a/docs/guide/counter/cmd/countercli/commands/counter.go +++ b/docs/guide/counter/cmd/countercli/commands/counter.go @@ -24,6 +24,7 @@ You must pass --valid for it to count and the countfee will be added to the coun RunE: doCounterTx, } +// nolint - flags names const ( FlagCountFee = "countfee" FlagValid = "valid" @@ -74,6 +75,6 @@ func readCounterTxFlags() (tx basecoin.Tx, err error) { return tx, err } - tx = counter.NewCounterTx(viper.GetBool(FlagValid), feeCoins, viper.GetInt(FlagSequence)) + tx = counter.NewTx(viper.GetBool(FlagValid), feeCoins, viper.GetInt(FlagSequence)) return tx, nil } diff --git a/docs/guide/counter/cmd/countercli/commands/query.go b/docs/guide/counter/cmd/countercli/commands/query.go index 0d5febfe9e..be0c502652 100644 --- a/docs/guide/counter/cmd/countercli/commands/query.go +++ b/docs/guide/counter/cmd/countercli/commands/query.go @@ -8,7 +8,7 @@ import ( "github.com/tendermint/basecoin/docs/guide/counter/plugins/counter" ) -//CounterQueryCmd CLI command to query the counter state +//CounterQueryCmd - CLI command to query the counter state var CounterQueryCmd = &cobra.Command{ Use: "counter", Short: "Query counter state, with proof", @@ -18,7 +18,7 @@ var CounterQueryCmd = &cobra.Command{ func counterQueryCmd(cmd *cobra.Command, args []string) error { key := counter.StateKey() - var cp counter.CounterState + var cp counter.State proof, err := proofcmd.GetAndParseAppProof(key, &cp) if err != nil { return err diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 96d409c754..0aafca19b0 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -13,7 +13,7 @@ import ( "github.com/tendermint/basecoin/types" ) -// CounterTx +// Tx //-------------------------------------------------------------------------------- // register the tx type with it's validation logic @@ -21,34 +21,37 @@ import ( // so it gets routed properly const ( NameCounter = "cntr" - ByteTx = 0x21 + ByteTx = 0x21 //TODO What does this byte represent should use typebytes probably TypeTx = NameCounter + "/count" ) func init() { - basecoin.TxMapper.RegisterImplementation(CounterTx{}, TypeTx, ByteTx) + basecoin.TxMapper.RegisterImplementation(Tx{}, TypeTx, ByteTx) } -type CounterTx struct { +// Tx - struct for all counter transactions +type Tx struct { Valid bool `json:"valid"` Fee types.Coins `json:"fee"` Sequence int `json:"sequence"` } -func NewCounterTx(valid bool, fee types.Coins, sequence int) basecoin.Tx { - return CounterTx{ +// NewTx - return a new counter transaction struct wrapped as a basecoin transaction +func NewTx(valid bool, fee types.Coins, sequence int) basecoin.Tx { + return Tx{ Valid: valid, Fee: fee, Sequence: sequence, }.Wrap() } -func (c CounterTx) Wrap() basecoin.Tx { - return basecoin.Tx{c} +// Wrap - Wrap a Tx as a Basecoin Tx, used to satisfy the XXX interface +func (c Tx) Wrap() basecoin.Tx { + return basecoin.Tx{TxInner: c} } // ValidateBasic just makes sure the Fee is a valid, non-negative value -func (c CounterTx) ValidateBasic() error { +func (c Tx) ValidateBasic() error { if !c.Fee.IsValid() { return coin.ErrInvalidCoins() } @@ -65,26 +68,29 @@ var ( errInvalidCounter = rawerr.New("Counter Tx marked invalid") ) -// This is a custom error class +// ErrInvalidCounter - custom error class func ErrInvalidCounter() error { return errors.WithCode(errInvalidCounter, abci.CodeType_BaseInvalidInput) } + +// IsInvalidCounterErr - custom error class check func IsInvalidCounterErr(err error) bool { return errors.IsSameError(errInvalidCounter, err) } -// This is just a helper function to return a generic "internal error" +// ErrDecoding - This is just a helper function to return a generic "internal error" func ErrDecoding() error { return errors.ErrInternal("Error decoding state") } -// CounterHandler +// Counter Handler //-------------------------------------------------------------------------------- -func NewCounterHandler() basecoin.Handler { +// NewHandler returns a new counter transaction processing handler +func NewHandler() basecoin.Handler { // use the default stack coin := coin.NewHandler() - counter := CounterHandler{} + counter := Handler{} dispatcher := stack.NewDispatcher( stack.WrapHandler(coin), counter, @@ -92,26 +98,29 @@ func NewCounterHandler() basecoin.Handler { return stack.NewDefault().Use(dispatcher) } -type CounterHandler struct { +// Handler the counter transaction processing handler +type Handler struct { stack.NopOption } -var _ stack.Dispatchable = CounterHandler{} +var _ stack.Dispatchable = Handler{} -func (_ CounterHandler) Name() string { +// Name - return counter namespace +func (h Handler) Name() string { return NameCounter } -func (_ CounterHandler) AssertDispatcher() {} +// AssertDispatcher - placeholder to satisfy XXX +func (h Handler) AssertDispatcher() {} // CheckTx checks if the tx is properly structured -func (h CounterHandler) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, _ basecoin.Checker) (res basecoin.Result, err error) { +func (h Handler) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, _ basecoin.Checker) (res basecoin.Result, err error) { _, err = checkTx(ctx, tx) return } // DeliverTx executes the tx if valid -func (h CounterHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, dispatch basecoin.Deliver) (res basecoin.Result, err error) { +func (h Handler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, dispatch basecoin.Deliver) (res basecoin.Result, err error) { ctr, err := checkTx(ctx, tx) if err != nil { return res, err @@ -130,7 +139,7 @@ func (h CounterHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx return res, errors.ErrMissingSignature() } in := []coin.TxInput{{Address: senders[0], Coins: ctr.Fee, Sequence: ctr.Sequence}} - out := []coin.TxOutput{{Address: CounterAcct(), 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 _, err = dispatch.DeliverTx(ctx, store, send) @@ -144,15 +153,15 @@ func (h CounterHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx if err != nil { return res, err } - state.Counter += 1 + state.Counter++ state.TotalFees = state.TotalFees.Plus(ctr.Fee) - err = StoreState(store, state) + err = SaveState(store, state) return res, err } -func checkTx(ctx basecoin.Context, tx basecoin.Tx) (ctr CounterTx, err error) { - ctr, ok := tx.Unwrap().(CounterTx) +func checkTx(ctx basecoin.Context, tx basecoin.Tx) (ctr Tx, err error) { + ctr, ok := tx.Unwrap().(Tx) if !ok { return ctr, errors.ErrInvalidFormat(tx) } @@ -166,20 +175,24 @@ func checkTx(ctx basecoin.Context, tx basecoin.Tx) (ctr CounterTx, err error) { // CounterStore //-------------------------------------------------------------------------------- -func CounterAcct() basecoin.Actor { - return basecoin.Actor{App: NameCounter, Address: []byte{0x04, 0x20}} +// StoreActor - return the basecoin actor for the account +func StoreActor() basecoin.Actor { + return basecoin.Actor{App: NameCounter, Address: []byte{0x04, 0x20}} //XXX what do these bytes represent? - should use typebyte variables } -type CounterState struct { +// State - state of the counter applicaton +type State struct { Counter int `json:"counter"` TotalFees types.Coins `json:"total_fees"` } +// StateKey - store key for the counter state func StateKey() []byte { return []byte(NameCounter + "/state") } -func LoadState(store types.KVStore) (state CounterState, err error) { +// LoadState - retrieve the counter state from the store +func LoadState(store types.KVStore) (state State, err error) { bytes := store.Get(StateKey()) if len(bytes) > 0 { err = wire.ReadBinaryBytes(bytes, &state) @@ -190,7 +203,8 @@ func LoadState(store types.KVStore) (state CounterState, err error) { return state, nil } -func StoreState(store types.KVStore, state CounterState) error { +// SaveState - save the counter state to the provided store +func SaveState(store types.KVStore, state State) error { bytes := wire.BinaryBytes(state) store.Set(StateKey(), bytes) return nil diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index 8b41fcaec4..c29f9f4bdd 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) { l := log.NewTMLogger(os.Stdout).With("module", "app") // l = log.NewTracingLogger(l) bcApp := app.NewBasecoin( - NewCounterHandler(), + NewHandler(), eyesCli, l, ) @@ -39,7 +39,7 @@ func TestCounterPlugin(t *testing.T) { // Seed Basecoin with account test1Acc := test1PrivAcc.Account - test1Acc.Balance = types.Coins{{"", 1000}, {"gold", 1000}} + test1Acc.Balance = types.Coins{{"", 1000}, {"gold", 1000}} //nolint accOpt, err := json.Marshal(test1Acc) require.Nil(t, err) log := bcApp.SetOption("coin/account", string(accOpt)) @@ -47,7 +47,7 @@ func TestCounterPlugin(t *testing.T) { // Deliver a CounterTx DeliverCounterTx := func(valid bool, counterFee types.Coins, inputSequence int) abci.Result { - tx := NewCounterTx(valid, counterFee, inputSequence) + tx := NewTx(valid, counterFee, inputSequence) tx = txs.NewChain(chainID, tx) stx := txs.NewSig(tx) txs.Sign(stx, test1PrivAcc.PrivKey) @@ -64,10 +64,10 @@ func TestCounterPlugin(t *testing.T) { assert.True(res.IsErr(), res.String()) // Test the fee (increments sequence) - res = DeliverCounterTx(true, types.Coins{{"gold", 100}}, 1) + res = DeliverCounterTx(true, types.Coins{{"gold", 100}}, 1) //nolint assert.True(res.IsOK(), res.String()) // Test unsupported fee - res = DeliverCounterTx(true, types.Coins{{"silver", 100}}, 2) + res = DeliverCounterTx(true, types.Coins{{"silver", 100}}, 2) //nolint assert.True(res.IsErr(), res.String()) } diff --git a/modules/coin/store.go b/modules/coin/store.go index 9f9f671104..9490f302b3 100644 --- a/modules/coin/store.go +++ b/modules/coin/store.go @@ -25,6 +25,7 @@ func NewAccountant(prefix string) Accountant { func (a Accountant) GetAccount(store types.KVStore, addr basecoin.Actor) (Account, error) { acct, err := loadAccount(store, a.MakeKey(addr)) + // for empty accounts, don't return an error, but rather an empty account if IsNoAccountErr(err) { err = nil From 5044032a23d89e2ea7811a39876abd7aba169eb5 Mon Sep 17 00:00:00 2001 From: rigel rozanski Date: Wed, 5 Jul 2017 06:57:52 -0400 Subject: [PATCH 20/24] golint compliant for app, cmd folders --- cmd/basecli/commands/query.go | 2 +- cmd/basecli/main.go | 23 +++++----- cmd/basecoin/commands/init.go | 23 +++++----- cmd/basecoin/commands/key.go | 9 +++- cmd/basecoin/commands/plugin_util.go | 4 +- cmd/basecoin/commands/reset.go | 1 + cmd/basecoin/commands/root.go | 2 + cmd/basecoin/commands/start.go | 12 ++--- cmd/basecoin/commands/utils.go | 66 +--------------------------- cmd/basecoin/commands/version.go | 1 + cmd/basecoin/main.go | 6 +-- 11 files changed, 48 insertions(+), 101 deletions(-) diff --git a/cmd/basecli/commands/query.go b/cmd/basecli/commands/query.go index e497c798dd..d5728d05e2 100644 --- a/cmd/basecli/commands/query.go +++ b/cmd/basecli/commands/query.go @@ -45,7 +45,7 @@ type BaseTxPresenter struct { proofs.RawPresenter // this handles MakeKey as hex bytes } -// ParseData - parse BaseTxPresenter Data +// ParseData - unmarshal raw bytes to a basecoin tx func (b BaseTxPresenter) ParseData(raw []byte) (interface{}, error) { var tx basecoin.Tx err := wire.ReadBinaryBytes(raw, &tx) diff --git a/cmd/basecli/main.go b/cmd/basecli/main.go index 4f78f792f1..4d96fc3f08 100644 --- a/cmd/basecli/main.go +++ b/cmd/basecli/main.go @@ -18,7 +18,7 @@ import ( coincmd "github.com/tendermint/basecoin/cmd/basecoin/commands" ) -// BaseCli represents the base command when called without any subcommands +// BaseCli - main basecoin client command var BaseCli = &cobra.Command{ Use: "basecli", Short: "Light client for tendermint", @@ -34,16 +34,19 @@ func main() { commands.AddBasicFlags(BaseCli) // Prepare queries - pr := proofs.RootCmd - // These are default parsers, but optional in your app (you can remove key) - pr.AddCommand(proofs.TxCmd) - pr.AddCommand(proofs.KeyCmd) - pr.AddCommand(bcmd.AccountQueryCmd) + proofs.RootCmd.AddCommand( + // These are default parsers, but optional in your app (you can remove key) + proofs.TxCmd, + proofs.KeyCmd, + bcmd.AccountQueryCmd, + ) // you will always want this for the base send command proofs.TxPresenters.Register("base", bcmd.BaseTxPresenter{}) - tr := txs.RootCmd - tr.AddCommand(bcmd.SendTxCmd) + txs.RootCmd.AddCommand( + // This is the default transaction, optional in your app + bcmd.SendTxCmd, + ) // Set up the various commands to use BaseCli.AddCommand( @@ -52,8 +55,8 @@ func main() { keycmd.RootCmd, seeds.RootCmd, rpccmd.RootCmd, - pr, - tr, + proofs.RootCmd, + txs.RootCmd, proxy.RootCmd, coincmd.VersionCmd, bcmd.AutoCompleteCmd, diff --git a/cmd/basecoin/commands/init.go b/cmd/basecoin/commands/init.go index b0b6a3fc28..58688a1504 100644 --- a/cmd/basecoin/commands/init.go +++ b/cmd/basecoin/commands/init.go @@ -11,23 +11,21 @@ import ( tcmd "github.com/tendermint/tendermint/cmd/tendermint/commands" ) -//commands -var ( - InitCmd = &cobra.Command{ - Use: "init [address]", - Short: "Initialize a basecoin blockchain", - RunE: initCmd, - } -) +// InitCmd - node initialization command +var InitCmd = &cobra.Command{ + Use: "init [address]", + Short: "Initialize a basecoin blockchain", + RunE: initCmd, +} //flags var ( - chainIDFlag string + flagChainID string ) func init() { flags := []Flag2Register{ - {&chainIDFlag, "chain-id", "test_chain_id", "Chain ID"}, + {&flagChainID, "chain-id", "test_chain_id", "Chain ID"}, } RegisterFlags(InitCmd, flags) } @@ -62,7 +60,7 @@ func initCmd(cmd *cobra.Command, args []string) error { privValFile := cfg.PrivValidatorFile() keyFile := path.Join(cfg.RootDir, "key.json") - mod1, err := setupFile(genesisFile, GetGenesisJSON(chainIDFlag, userAddr), 0644) + mod1, err := setupFile(genesisFile, GetGenesisJSON(flagChainID, userAddr), 0644) if err != nil { return err } @@ -85,6 +83,7 @@ func initCmd(cmd *cobra.Command, args []string) error { return nil } +// PrivValJSON - validator private key file contents in json var PrivValJSON = `{ "address": "7A956FADD20D3A5B2375042B2959F8AB172A058F", "last_height": 0, @@ -134,7 +133,7 @@ func GetGenesisJSON(chainID, addr string) string { }`, chainID, addr) } -// TODO: remove this once not needed for relay +// KeyJSON - TODO: remove this once not needed for relay var KeyJSON = `{ "address": "1B1BE55F969F54064628A63B9559E7C21C925165", "priv_key": { diff --git a/cmd/basecoin/commands/key.go b/cmd/basecoin/commands/key.go index 08f54e0a42..b5b03e3fba 100644 --- a/cmd/basecoin/commands/key.go +++ b/cmd/basecoin/commands/key.go @@ -19,12 +19,15 @@ import ( //--------------------------------------------- // simple implementation of a key +// Address - public address for a key type Address [20]byte +// MarshalJSON - marshal the json bytes of the address func (a Address) MarshalJSON() ([]byte, error) { return []byte(fmt.Sprintf(`"%x"`, a[:])), nil } +// UnmarshalJSON - unmarshal the json bytes of the address func (a *Address) UnmarshalJSON(addrHex []byte) error { addr, err := hex.DecodeString(strings.Trim(string(addrHex), `"`)) if err != nil { @@ -34,17 +37,19 @@ func (a *Address) UnmarshalJSON(addrHex []byte) error { return nil } +// Key - full private key type Key struct { Address Address `json:"address"` PubKey crypto.PubKey `json:"pub_key"` PrivKey crypto.PrivKey `json:"priv_key"` } -// Implements Signer +// Sign - Implements Signer func (k *Key) Sign(msg []byte) crypto.Signature { return k.PrivKey.Sign(msg) } +// LoadKey - load key from json file func LoadKey(keyFile string) (*Key, error) { filePath := keyFile @@ -61,7 +66,7 @@ func LoadKey(keyFile string) (*Key, error) { key := new(Key) err = json.Unmarshal(keyJSONBytes, key) if err != nil { - return nil, fmt.Errorf("Error reading key from %v: %v\n", filePath, err) //never stack trace + return nil, fmt.Errorf("Error reading key from %v: %v", filePath, err) //never stack trace } return key, nil diff --git a/cmd/basecoin/commands/plugin_util.go b/cmd/basecoin/commands/plugin_util.go index d4f47b323a..ee56d7b920 100644 --- a/cmd/basecoin/commands/plugin_util.go +++ b/cmd/basecoin/commands/plugin_util.go @@ -14,12 +14,12 @@ type plugin struct { var plugins = []plugin{} -// RegisterStartPlugin is used to enable a plugin +// RegisterStartPlugin - used to enable a plugin func RegisterStartPlugin(name string, newPlugin func() types.Plugin) { plugins = append(plugins, plugin{name: name, newPlugin: newPlugin}) } -//Returns a version command based on version input +// QuickVersionCmd - returns a version command based on version input func QuickVersionCmd(version string) *cobra.Command { return &cobra.Command{ Use: "version", diff --git a/cmd/basecoin/commands/reset.go b/cmd/basecoin/commands/reset.go index 00c3d71718..b982fe0230 100644 --- a/cmd/basecoin/commands/reset.go +++ b/cmd/basecoin/commands/reset.go @@ -6,6 +6,7 @@ import ( tcmd "github.com/tendermint/tendermint/cmd/tendermint/commands" ) +// UnsafeResetAllCmd - extension of the tendermint command, resets initialization var UnsafeResetAllCmd = &cobra.Command{ Use: "unsafe_reset_all", Short: "Reset all blockchain data", diff --git a/cmd/basecoin/commands/root.go b/cmd/basecoin/commands/root.go index bdcc25d728..5d76c83e5d 100644 --- a/cmd/basecoin/commands/root.go +++ b/cmd/basecoin/commands/root.go @@ -11,6 +11,7 @@ import ( "github.com/tendermint/tmlibs/log" ) +//nolint const ( defaultLogLevel = "error" FlagLogLevel = "log_level" @@ -20,6 +21,7 @@ var ( logger = log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "main") ) +// RootCmd - main node command var RootCmd = &cobra.Command{ Use: "basecoin", Short: "A cryptocurrency framework in Golang based on Tendermint-Core", diff --git a/cmd/basecoin/commands/start.go b/cmd/basecoin/commands/start.go index 791e810a62..4fd2f19b2c 100644 --- a/cmd/basecoin/commands/start.go +++ b/cmd/basecoin/commands/start.go @@ -24,13 +24,14 @@ import ( "github.com/tendermint/basecoin/app" ) +// StartCmd - command to start running the basecoin node! var StartCmd = &cobra.Command{ Use: "start", Short: "Start basecoin", RunE: startCmd, } -// TODO: move to config file +// nolint TODO: move to config file const EyesCacheSize = 10000 //nolint @@ -41,7 +42,7 @@ const ( ) var ( - // use a global to store the handler, so we can set it in main. + // Handler - use a global to store the handler, so we can set it in main. // TODO: figure out a cleaner way to register plugins Handler basecoin.Handler ) @@ -95,11 +96,10 @@ func startCmd(cmd *cobra.Command, args []string) error { logger.Info("Starting Basecoin without Tendermint", "chain_id", chainID) // run just the abci app/server return startBasecoinABCI(basecoinApp) - } else { - logger.Info("Starting Basecoin with Tendermint", "chain_id", chainID) - // start the app with tendermint in-process - return startTendermint(rootDir, basecoinApp) } + logger.Info("Starting Basecoin with Tendermint", "chain_id", chainID) + // start the app with tendermint in-process + return startTendermint(rootDir, basecoinApp) } func startBasecoinABCI(basecoinApp *app.Basecoin) error { diff --git a/cmd/basecoin/commands/utils.go b/cmd/basecoin/commands/utils.go index dc0b5f1620..a039f00146 100644 --- a/cmd/basecoin/commands/utils.go +++ b/cmd/basecoin/commands/utils.go @@ -5,8 +5,6 @@ import ( "fmt" "github.com/pkg/errors" - "github.com/spf13/cobra" - "github.com/spf13/pflag" abci "github.com/tendermint/abci/types" wire "github.com/tendermint/go-wire" @@ -17,68 +15,6 @@ import ( tmtypes "github.com/tendermint/tendermint/types" ) -//Quickly registering flags can be quickly achieved through using the utility functions -//RegisterFlags, and RegisterPersistentFlags. Ex: -// flags := []Flag2Register{ -// {&myStringFlag, "mystringflag", "foobar", "description of what this flag does"}, -// {&myBoolFlag, "myboolflag", false, "description of what this flag does"}, -// {&myInt64Flag, "myintflag", 333, "description of what this flag does"}, -// } -// RegisterFlags(MyCobraCmd, flags) -type Flag2Register struct { - Pointer interface{} - Use string - Value interface{} - Desc string -} - -//register flag utils -func RegisterFlags(c *cobra.Command, flags []Flag2Register) { - registerFlags(c, flags, false) -} - -func RegisterPersistentFlags(c *cobra.Command, flags []Flag2Register) { - registerFlags(c, flags, true) -} - -func registerFlags(c *cobra.Command, flags []Flag2Register, persistent bool) { - - var flagset *pflag.FlagSet - if persistent { - flagset = c.PersistentFlags() - } else { - flagset = c.Flags() - } - - for _, f := range flags { - - ok := false - - switch f.Value.(type) { - case string: - if _, ok = f.Pointer.(*string); ok { - flagset.StringVar(f.Pointer.(*string), f.Use, f.Value.(string), f.Desc) - } - case int: - if _, ok = f.Pointer.(*int); ok { - flagset.IntVar(f.Pointer.(*int), f.Use, f.Value.(int), f.Desc) - } - case uint64: - if _, ok = f.Pointer.(*uint64); ok { - flagset.Uint64Var(f.Pointer.(*uint64), f.Use, f.Value.(uint64), f.Desc) - } - case bool: - if _, ok = f.Pointer.(*bool); ok { - flagset.BoolVar(f.Pointer.(*bool), f.Use, f.Value.(bool), f.Desc) - } - } - - if !ok { - panic("improper use of RegisterFlags") - } - } -} - // Returns true for non-empty hex-string prefixed with "0x" func isHex(s string) bool { if len(s) > 2 && s[:2] == "0x" { @@ -91,6 +27,7 @@ func isHex(s string) bool { return false } +// StripHex remove the first two hex bytes func StripHex(s string) string { if isHex(s) { return s[2:] @@ -98,6 +35,7 @@ func StripHex(s string) string { return s } +// Query - send an abci query func Query(tmAddr string, key []byte) (*abci.ResultQuery, error) { httpClient := client.NewHTTP(tmAddr, "/websocket") return queryWithClient(httpClient, key) diff --git a/cmd/basecoin/commands/version.go b/cmd/basecoin/commands/version.go index 1cbf891e43..fe72d26ca5 100644 --- a/cmd/basecoin/commands/version.go +++ b/cmd/basecoin/commands/version.go @@ -8,6 +8,7 @@ import ( "github.com/tendermint/basecoin/version" ) +// VersionCmd - command to show the application version var VersionCmd = &cobra.Command{ Use: "version", Short: "Show version info", diff --git a/cmd/basecoin/main.go b/cmd/basecoin/main.go index 51c8ab4917..2185262b73 100644 --- a/cmd/basecoin/main.go +++ b/cmd/basecoin/main.go @@ -16,13 +16,11 @@ func main() { rt.AddCommand( commands.InitCmd, commands.StartCmd, - // commands.RelayCmd, + //commands.RelayCmd, commands.UnsafeResetAllCmd, commands.VersionCmd, ) cmd := cli.PrepareMainCmd(rt, "BC", os.ExpandEnv("$HOME/.basecoin")) - if err := cmd.Execute(); err != nil { - os.Exit(1) - } + cmd.Execute() } From 35845a958f1a6c7e2cb0a1d96a84d2367f87a993 Mon Sep 17 00:00:00 2001 From: rigel rozanski Date: Wed, 5 Jul 2017 07:08:56 -0400 Subject: [PATCH 21/24] fix cmd/bascoin/commands/init old flags --- cmd/basecoin/commands/init.go | 12 +++++------- docs/guide/counter/plugins/counter/counter_test.go | 6 +++--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/cmd/basecoin/commands/init.go b/cmd/basecoin/commands/init.go index 58688a1504..ac4520539a 100644 --- a/cmd/basecoin/commands/init.go +++ b/cmd/basecoin/commands/init.go @@ -7,6 +7,7 @@ import ( "path" "github.com/spf13/cobra" + "github.com/spf13/viper" tcmd "github.com/tendermint/tendermint/cmd/tendermint/commands" ) @@ -18,16 +19,13 @@ var InitCmd = &cobra.Command{ RunE: initCmd, } -//flags +//nolint - flags var ( - flagChainID string + FlagChainID = "chain-id" //TODO group with other flags or remove? is this already a flag here? ) func init() { - flags := []Flag2Register{ - {&flagChainID, "chain-id", "test_chain_id", "Chain ID"}, - } - RegisterFlags(InitCmd, flags) + InitCmd.Flags().String(FlagChainID, "test_chain_id", "Chain ID") } // returns 1 iff it set a file, otherwise 0 (so we can add them) @@ -60,7 +58,7 @@ func initCmd(cmd *cobra.Command, args []string) error { privValFile := cfg.PrivValidatorFile() keyFile := path.Join(cfg.RootDir, "key.json") - mod1, err := setupFile(genesisFile, GetGenesisJSON(flagChainID, userAddr), 0644) + mod1, err := setupFile(genesisFile, GetGenesisJSON(viper.GetString(FlagChainID), userAddr), 0644) if err != nil { return err } diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index c29f9f4bdd..fcc36a15c1 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -39,7 +39,7 @@ func TestCounterPlugin(t *testing.T) { // Seed Basecoin with account test1Acc := test1PrivAcc.Account - test1Acc.Balance = types.Coins{{"", 1000}, {"gold", 1000}} //nolint + test1Acc.Balance = types.Coins{{"", 1000}, {"gold", 1000}} accOpt, err := json.Marshal(test1Acc) require.Nil(t, err) log := bcApp.SetOption("coin/account", string(accOpt)) @@ -64,10 +64,10 @@ func TestCounterPlugin(t *testing.T) { assert.True(res.IsErr(), res.String()) // Test the fee (increments sequence) - res = DeliverCounterTx(true, types.Coins{{"gold", 100}}, 1) //nolint + res = DeliverCounterTx(true, types.Coins{{"gold", 100}}, 1) assert.True(res.IsOK(), res.String()) // Test unsupported fee - res = DeliverCounterTx(true, types.Coins{{"silver", 100}}, 2) //nolint + res = DeliverCounterTx(true, types.Coins{{"silver", 100}}, 2) assert.True(res.IsErr(), res.String()) } From 0303f2aaaa3c87e5408a759a0921e5f2be068e3d Mon Sep 17 00:00:00 2001 From: rigel rozanski Date: Thu, 6 Jul 2017 05:30:03 -0400 Subject: [PATCH 22/24] golinted up to and incl modules --- app/genesis.go | 2 +- errors/common.go | 28 ++++++++++++---------------- errors/main.go | 5 +---- modules/coin/errors.go | 1 + modules/coin/genesis.go | 3 +++ modules/coin/handler.go | 17 +++++++++-------- modules/coin/store.go | 9 ++++++++- modules/coin/tx.go | 12 +++++++++++- modules/fee/errors.go | 1 + modules/fee/handler.go | 12 ++++++++---- version/version.go | 1 + 11 files changed, 56 insertions(+), 35 deletions(-) diff --git a/app/genesis.go b/app/genesis.go index 1549ca7071..87c60227f9 100644 --- a/app/genesis.go +++ b/app/genesis.go @@ -8,7 +8,7 @@ import ( cmn "github.com/tendermint/tmlibs/common" ) -// LoadGenesis - Load the genesis json file +// LoadGenesis - Load the genesis file into memory func (app *Basecoin) LoadGenesis(path string) error { genDoc, err := loadGenesis(path) if err != nil { diff --git a/errors/common.go b/errors/common.go index ac9e2809a8..43d722c8ad 100644 --- a/errors/common.go +++ b/errors/common.go @@ -1,11 +1,7 @@ +//nolint package errors -/** -* Copyright (C) 2017 Ethan Frey -**/ - import ( - rawerr "errors" "fmt" "github.com/pkg/errors" @@ -14,17 +10,17 @@ import ( ) var ( - errDecoding = rawerr.New("Error decoding input") - errUnauthorized = rawerr.New("Unauthorized") - errInvalidSignature = rawerr.New("Invalid Signature") - errTooLarge = rawerr.New("Input size too large") - errMissingSignature = rawerr.New("Signature missing") - errTooManySignatures = rawerr.New("Too many signatures") - errNoChain = rawerr.New("No chain id provided") - errWrongChain = rawerr.New("Wrong chain for tx") - errUnknownTxType = rawerr.New("Tx type unknown") - errInvalidFormat = rawerr.New("Invalid format") - errUnknownModule = rawerr.New("Unknown module") + errDecoding = fmt.Errorf("Error decoding input") + errUnauthorized = fmt.Errorf("Unauthorized") + errInvalidSignature = fmt.Errorf("Invalid Signature") + errTooLarge = fmt.Errorf("Input size too large") + errMissingSignature = fmt.Errorf("Signature missing") + errTooManySignatures = fmt.Errorf("Too many signatures") + errNoChain = fmt.Errorf("No chain id provided") + errWrongChain = fmt.Errorf("Wrong chain for tx") + errUnknownTxType = fmt.Errorf("Tx type unknown") + errInvalidFormat = fmt.Errorf("Invalid format") + errUnknownModule = fmt.Errorf("Unknown module") ) func ErrUnknownTxType(tx basecoin.Tx) TMError { diff --git a/errors/main.go b/errors/main.go index f19a7f692e..21f261148a 100644 --- a/errors/main.go +++ b/errors/main.go @@ -1,9 +1,5 @@ package errors -/** -* Copyright (C) 2017 Ethan Frey -**/ - import ( "fmt" @@ -23,6 +19,7 @@ type causer interface { Cause() error } +// TMError is the tendermint abci return type with stack trace type TMError interface { stackTracer ErrorCode() abci.CodeType diff --git a/modules/coin/errors.go b/modules/coin/errors.go index d9b2fea6c0..7b86f6343f 100644 --- a/modules/coin/errors.go +++ b/modules/coin/errors.go @@ -1,3 +1,4 @@ +//nolint package coin import ( diff --git a/modules/coin/genesis.go b/modules/coin/genesis.go index 7ce32cd8d2..4b26fa1e68 100644 --- a/modules/coin/genesis.go +++ b/modules/coin/genesis.go @@ -13,6 +13,7 @@ import ( /**** code to parse accounts from genesis docs ***/ +// GenesisAccount - genesis account parameters type GenesisAccount struct { Address data.Bytes `json:"address"` // this from types.Account (don't know how to embed this properly) @@ -21,6 +22,7 @@ type GenesisAccount struct { Balance types.Coins `json:"coins"` } +// ToAccount - GenesisAccount struct to a basecoin Account func (g GenesisAccount) ToAccount() Account { return Account{ Sequence: g.Sequence, @@ -28,6 +30,7 @@ func (g GenesisAccount) ToAccount() Account { } } +// GetAddr - Get the address of the genesis account func (g GenesisAccount) GetAddr() ([]byte, error) { noAddr, noPk := len(g.Address) == 0, g.PubKey.Empty() diff --git a/modules/coin/handler.go b/modules/coin/handler.go index def6689c29..9ed218a34a 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -12,24 +12,25 @@ import ( "github.com/tendermint/basecoin/types" ) -const ( - NameCoin = "coin" -) +//NameCoin - name space of the coin module +const NameCoin = "coin" -// Handler writes +// Handler includes an accountant type Handler struct { Accountant } var _ basecoin.Handler = Handler{} +// NewHandler - new accountant handler for the coin module func NewHandler() Handler { return Handler{ Accountant: NewAccountant(""), } } -func (_ Handler) Name() string { +// Name - return name space +func (h Handler) Name() string { return NameCoin } @@ -80,6 +81,7 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoi return basecoin.Result{}, nil } +// SetOption - sets the genesis account balance func (h Handler) SetOption(l log.Logger, store types.KVStore, module, key, value string) (log string, err error) { if module != NameCoin { return "", errors.ErrUnknownModule(module) @@ -103,10 +105,9 @@ func (h Handler) SetOption(l log.Logger, store types.KVStore, module, key, value } return "Success", nil - } else { - msg := fmt.Sprintf("Unknown key: %s", key) - return "", errors.ErrInternal(msg) } + msg := fmt.Sprintf("Unknown key: %s", key) + return "", errors.ErrInternal(msg) } func checkTx(ctx basecoin.Context, tx basecoin.Tx) (send SendTx, err error) { diff --git a/modules/coin/store.go b/modules/coin/store.go index 9490f302b3..b5d14ca850 100644 --- a/modules/coin/store.go +++ b/modules/coin/store.go @@ -10,10 +10,13 @@ import ( "github.com/tendermint/basecoin/types" ) +// Accountant - custom object to manage coins for the coin module +// TODO prefix should be post-fix if maintaining the same key space type Accountant struct { Prefix []byte } +// NewAccountant - create the new accountant with prefix information func NewAccountant(prefix string) Accountant { if prefix == "" { prefix = NameCoin @@ -23,6 +26,7 @@ func NewAccountant(prefix string) Accountant { } } +// GetAccount - Get account from store and address func (a Accountant) GetAccount(store types.KVStore, addr basecoin.Actor) (Account, error) { acct, err := loadAccount(store, a.MakeKey(addr)) @@ -68,7 +72,7 @@ func (a Accountant) updateCoins(store types.KVStore, addr basecoin.Actor, coins if seq != acct.Sequence+1 { return acct, ErrInvalidSequence() } - acct.Sequence += 1 + acct.Sequence++ } // check amount @@ -81,6 +85,8 @@ func (a Accountant) updateCoins(store types.KVStore, addr basecoin.Actor, coins return acct, nil } +// MakeKey - generate key bytes from address using accountant prefix +// TODO Prefix -> PostFix for consistent namespace func (a Accountant) MakeKey(addr basecoin.Actor) []byte { key := addr.Bytes() if len(a.Prefix) > 0 { @@ -89,6 +95,7 @@ func (a Accountant) MakeKey(addr basecoin.Actor) []byte { return key } +// Account - coin account structure type Account struct { Coins types.Coins `json:"coins"` Sequence int `json:"sequence"` diff --git a/modules/coin/tx.go b/modules/coin/tx.go index c5f7370c01..c6b9c66a73 100644 --- a/modules/coin/tx.go +++ b/modules/coin/tx.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/tendermint/basecoin" - "github.com/tendermint/basecoin/types" ) @@ -20,12 +19,14 @@ const ( //----------------------------------------------------------------------------- +// TxInput - expected coin movement outputs, used with SendTx type TxInput struct { Address basecoin.Actor `json:"address"` Coins types.Coins `json:"coins"` Sequence int `json:"sequence"` // Nonce: Must be 1 greater than the last committed TxInput } +// ValidateBasic - validate transaction input func (txIn TxInput) ValidateBasic() error { if txIn.Address.App == "" { return ErrInvalidAddress() @@ -50,6 +51,7 @@ func (txIn TxInput) String() string { return fmt.Sprintf("TxInput{%v,%v,%v}", txIn.Address, txIn.Coins, txIn.Sequence) } +// NewTxInput - create a transaction input, used with SendTx func NewTxInput(addr basecoin.Actor, coins types.Coins, sequence int) TxInput { input := TxInput{ Address: addr, @@ -61,11 +63,13 @@ func NewTxInput(addr basecoin.Actor, coins types.Coins, sequence int) TxInput { //----------------------------------------------------------------------------- +// TxOutput - expected coin movement output, used with SendTx type TxOutput struct { Address basecoin.Actor `json:"address"` Coins types.Coins `json:"coins"` } +// ValidateBasic - validate transaction output func (txOut TxOutput) ValidateBasic() error { if txOut.Address.App == "" { return ErrInvalidAddress() @@ -87,6 +91,7 @@ func (txOut TxOutput) String() string { return fmt.Sprintf("TxOutput{%X,%v}", txOut.Address, txOut.Coins) } +// NewTxOutput - create a transaction output, used with SendTx func NewTxOutput(addr basecoin.Actor, coins types.Coins) TxOutput { output := TxOutput{ Address: addr, @@ -97,6 +102,8 @@ func NewTxOutput(addr basecoin.Actor, coins types.Coins) TxOutput { //----------------------------------------------------------------------------- +// SendTx - high level transaction of the coin module +// Satisfies: TxInner type SendTx struct { Inputs []TxInput `json:"inputs"` Outputs []TxOutput `json:"outputs"` @@ -104,10 +111,12 @@ type SendTx struct { var _ basecoin.Tx = NewSendTx(nil, nil) +// NewSendTx - new SendTx func NewSendTx(in []TxInput, out []TxOutput) basecoin.Tx { 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, // not that they actually have the money inside @@ -142,6 +151,7 @@ func (tx SendTx) String() string { return fmt.Sprintf("SendTx{%v->%v}", tx.Inputs, tx.Outputs) } +// Wrap - used to satisfy TxInner func (tx SendTx) Wrap() basecoin.Tx { return basecoin.Tx{tx} } diff --git a/modules/fee/errors.go b/modules/fee/errors.go index 0b6d7fcaf8..f29d3cbe16 100644 --- a/modules/fee/errors.go +++ b/modules/fee/errors.go @@ -1,3 +1,4 @@ +//nolint package fee import ( diff --git a/modules/fee/handler.go b/modules/fee/handler.go index f711cac953..7f4f579f07 100644 --- a/modules/fee/handler.go +++ b/modules/fee/handler.go @@ -7,10 +7,10 @@ import ( "github.com/tendermint/basecoin/types" ) -const ( - NameFee = "fee" -) +// 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 types.KVStore, addr basecoin.Actor) (types.Coins, error) @@ -20,13 +20,15 @@ type AccountChecker interface { ChangeAmount(store types.KVStore, addr basecoin.Actor, coins types.Coins) (types.Coins, error) } +// SimpleFeeHandler - checker object for fee checking type SimpleFeeHandler struct { AccountChecker MinFee types.Coins stack.PassOption } -func (_ SimpleFeeHandler) Name() string { +// Name - return the namespace for the fee module +func (SimpleFeeHandler) Name() string { return NameFee } @@ -34,6 +36,7 @@ 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 types.KVStore, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) { feeTx, ok := tx.Unwrap().(*Fee) if !ok { @@ -57,6 +60,7 @@ func (h SimpleFeeHandler) CheckTx(ctx basecoin.Context, store types.KVStore, tx return basecoin.Result{Log: "Valid tx"}, nil } +// DeliverTx - send the fee handler transaction func (h SimpleFeeHandler) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) { feeTx, ok := tx.Unwrap().(*Fee) if !ok { diff --git a/version/version.go b/version/version.go index 1e25edb4a6..c89c513ace 100644 --- a/version/version.go +++ b/version/version.go @@ -1,3 +1,4 @@ +//nolint package version const Maj = "0" From 9b561344fe8e986d4c7eff781ea1e1a072418468 Mon Sep 17 00:00:00 2001 From: rigel rozanski Date: Thu, 6 Jul 2017 05:39:58 -0400 Subject: [PATCH 23/24] addressing PR 154 comments --- cmd/basecli/commands/query.go | 2 +- docs/guide/counter/cmd/countercli/commands/counter.go | 6 +++--- docs/guide/counter/plugins/counter/counter.go | 4 ++-- modules/coin/handler.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/basecli/commands/query.go b/cmd/basecli/commands/query.go index d5728d05e2..1bb05aef7a 100644 --- a/cmd/basecli/commands/query.go +++ b/cmd/basecli/commands/query.go @@ -46,7 +46,7 @@ type BaseTxPresenter struct { } // ParseData - unmarshal raw bytes to a basecoin tx -func (b BaseTxPresenter) ParseData(raw []byte) (interface{}, error) { +func (BaseTxPresenter) ParseData(raw []byte) (interface{}, error) { var tx basecoin.Tx err := wire.ReadBinaryBytes(raw, &tx) return tx, err diff --git a/docs/guide/counter/cmd/countercli/commands/counter.go b/docs/guide/counter/cmd/countercli/commands/counter.go index 0bb39e6abb..81d95e4b46 100644 --- a/docs/guide/counter/cmd/countercli/commands/counter.go +++ b/docs/guide/counter/cmd/countercli/commands/counter.go @@ -21,7 +21,7 @@ var CounterTxCmd = &cobra.Command{ Long: `Add a vote to the counter. You must pass --valid for it to count and the countfee will be added to the counter.`, - RunE: doCounterTx, + RunE: counterTx, } // nolint - flags names @@ -38,9 +38,9 @@ func init() { fs.Int(FlagSequence, -1, "Sequence number for this transaction") } -// TODO: doCounterTx is very similar to the sendtx one, +// TODO: counterTx is very similar to the sendtx one, // maybe we can pull out some common patterns? -func doCounterTx(cmd *cobra.Command, args []string) error { +func counterTx(cmd *cobra.Command, args []string) error { // load data from json or flags var tx basecoin.Tx found, err := txcmd.LoadJSON(&tx) diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 0aafca19b0..7587b7a796 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -106,12 +106,12 @@ type Handler struct { var _ stack.Dispatchable = Handler{} // Name - return counter namespace -func (h Handler) Name() string { +func (Handler) Name() string { return NameCounter } // AssertDispatcher - placeholder to satisfy XXX -func (h Handler) AssertDispatcher() {} +func (Handler) AssertDispatcher() {} // CheckTx checks if the tx is properly structured func (h Handler) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx, _ basecoin.Checker) (res basecoin.Result, err error) { diff --git a/modules/coin/handler.go b/modules/coin/handler.go index 9ed218a34a..44526bc9f3 100644 --- a/modules/coin/handler.go +++ b/modules/coin/handler.go @@ -30,7 +30,7 @@ func NewHandler() Handler { } // Name - return name space -func (h Handler) Name() string { +func (Handler) Name() string { return NameCoin } From 390fdf95cf3b94283190125e999c641c90e537fb Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 6 Jul 2017 12:18:20 +0200 Subject: [PATCH 24/24] Cleanup as requested on PR --- app/app_test.go | 8 +++---- docs/guide/counter/plugins/counter/counter.go | 4 ++-- .../counter/plugins/counter/counter_test.go | 9 ++++---- stack/dispatcher.go | 22 +++++++++++++++++++ 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index 753efcf3f7..2cb636e1c2 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -69,13 +69,13 @@ func (at *appTest) reset() { at.accOut = types.MakeAcc("output0") eyesCli := eyes.NewLocalClient("", 0) - // l := log.TestingLogger().With("module", "app"), - l := log.NewTMLogger(os.Stdout).With("module", "app") - l = log.NewTracingLogger(l) + // logger := log.TestingLogger().With("module", "app"), + logger := log.NewTMLogger(os.Stdout).With("module", "app") + logger = log.NewTracingLogger(logger) at.app = NewBasecoin( DefaultHandler(), eyesCli, - l, + logger, ) res := at.app.SetOption("base/chain_id", at.chainID) diff --git a/docs/guide/counter/plugins/counter/counter.go b/docs/guide/counter/plugins/counter/counter.go index 7587b7a796..79394249ef 100644 --- a/docs/guide/counter/plugins/counter/counter.go +++ b/docs/guide/counter/plugins/counter/counter.go @@ -1,7 +1,7 @@ package counter import ( - rawerr "errors" + "fmt" abci "github.com/tendermint/abci/types" "github.com/tendermint/go-wire" @@ -65,7 +65,7 @@ func (c Tx) ValidateBasic() error { //-------------------------------------------------------------------------------- var ( - errInvalidCounter = rawerr.New("Counter Tx marked invalid") + errInvalidCounter = fmt.Errorf("Counter Tx marked invalid") ) // ErrInvalidCounter - custom error class diff --git a/docs/guide/counter/plugins/counter/counter_test.go b/docs/guide/counter/plugins/counter/counter_test.go index fcc36a15c1..ceeb9a5526 100644 --- a/docs/guide/counter/plugins/counter/counter_test.go +++ b/docs/guide/counter/plugins/counter/counter_test.go @@ -23,16 +23,15 @@ func TestCounterPlugin(t *testing.T) { eyesCli := eyescli.NewLocalClient("", 0) chainID := "test_chain_id" - // l := log.TestingLogger().With("module", "app"), - l := log.NewTMLogger(os.Stdout).With("module", "app") - // l = log.NewTracingLogger(l) + // logger := log.TestingLogger().With("module", "app"), + logger := log.NewTMLogger(os.Stdout).With("module", "app") + // logger = log.NewTracingLogger(logger) bcApp := app.NewBasecoin( NewHandler(), eyesCli, - l, + logger, ) bcApp.SetOption("base/chain_id", chainID) - // t.Log(bcApp.Info()) // Account initialization test1PrivAcc := types.PrivAccountFromSecret("test1") diff --git a/stack/dispatcher.go b/stack/dispatcher.go index 5da79f7a04..df065973d7 100644 --- a/stack/dispatcher.go +++ b/stack/dispatcher.go @@ -11,6 +11,7 @@ import ( "github.com/tendermint/basecoin/types" ) +// nolint const ( NameDispatcher = "disp" ) @@ -19,10 +20,16 @@ const ( // // It will route tx to the proper locations and also allows them to call each // other synchronously through the same tx methods. +// +// Please note that iterating through a map is a non-deteministic operation +// and, as such, should never be done in the context of an ABCI app. Only +// use this map to look up an exact route by name. type Dispatcher struct { routes map[string]Dispatchable } +// NewDispatcher creates a dispatcher and adds the given routes. +// You can also add routes later with .AddRoutes() func NewDispatcher(routes ...Dispatchable) *Dispatcher { d := &Dispatcher{ routes: map[string]Dispatchable{}, @@ -47,10 +54,16 @@ func (d *Dispatcher) AddRoutes(routes ...Dispatchable) { } } +// Name - defines the name of this module func (d *Dispatcher) Name() string { return NameDispatcher } +// CheckTx - implements Handler interface +// +// Tries to find a registered module (Dispatchable) based on the name of the tx. +// The tx name (as registered with go-data) should be in the form `/XXXX`, +// where `module name` must match the name of a dispatchable and XXX can be any string. func (d *Dispatcher) CheckTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { r, err := d.lookupTx(tx) if err != nil { @@ -61,6 +74,11 @@ func (d *Dispatcher) CheckTx(ctx basecoin.Context, store types.KVStore, tx basec return r.CheckTx(ctx, store, tx, cb) } +// DeliverTx - implements Handler interface +// +// Tries to find a registered module (Dispatchable) based on the name of the tx. +// The tx name (as registered with go-data) should be in the form `/XXXX`, +// where `module name` must match the name of a dispatchable and XXX can be any string. func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store types.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) { r, err := d.lookupTx(tx) if err != nil { @@ -71,6 +89,10 @@ func (d *Dispatcher) DeliverTx(ctx basecoin.Context, store types.KVStore, tx bas return r.DeliverTx(ctx, store, tx, cb) } +// SetOption - implements Handler interface +// +// Tries to find a registered module (Dispatchable) based on the +// module name from SetOption of the tx. func (d *Dispatcher) SetOption(l log.Logger, store types.KVStore, module, key, value string) (string, error) { r, err := d.lookupModule(module) if err != nil {