Code cleanup from emmanuel

app/app_test.go

@@ -83,7 +83,9 @@ func (at *appTest) reset() {
 	// Note: switch logger if you want to get more info
 	logger := log.TestingLogger()
 	// logger := log.NewTracingLogger(log.NewTMLogger(os.Stdout))
-	store := NewStore("", 0, logger.With("module", "store"))
+	store, err := NewStore("", 0, logger.With("module", "store"))
+	require.Nil(at.t, err, "%+v", err)
+
 	at.app = NewBasecoin(
 		DefaultHandler("mycoin"),
 		store,
@@ -141,7 +143,9 @@ func TestSetOption(t *testing.T) {
 	require := require.New(t)
 
 	logger := log.TestingLogger()
-	store := NewStore("", 0, logger.With("module", "store"))
+	store, err := NewStore("", 0, logger.With("module", "store"))
+	require.Nil(err, "%+v", err)
+
 	app := NewBasecoin(
 		DefaultHandler("atom"),
 		store,

app/genesis_test.go

@@ -19,9 +19,10 @@ const genesisAcctFilepath = "./testdata/genesis2.json"
 
 func TestLoadGenesisDoNotFailIfAppOptionsAreMissing(t *testing.T) {
 	logger := log.TestingLogger()
-	store := NewStore("", 0, logger)
+	store, err := NewStore("", 0, logger)
+	require.Nil(t, err, "%+v", err)
 	app := NewBasecoin(DefaultHandler("mycoin"), store, logger)
-	err := app.LoadGenesis("./testdata/genesis3.json")
+	err = app.LoadGenesis("./testdata/genesis3.json")
 	require.Nil(t, err, "%+v", err)
 }
 
@@ -29,9 +30,11 @@ func TestLoadGenesis(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	logger := log.TestingLogger()
-	store := NewStore("", 0, logger)
+	store, err := NewStore("", 0, logger)
+	require.Nil(err, "%+v", err)
+
 	app := NewBasecoin(DefaultHandler("mycoin"), store, logger)
-	err := app.LoadGenesis(genesisFilepath)
+	err = app.LoadGenesis(genesisFilepath)
 	require.Nil(err, "%+v", err)
 
 	// check the chain id
@@ -59,9 +62,11 @@ func TestLoadGenesisAccountAddress(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	logger := log.TestingLogger()
-	store := NewStore("", 0, logger)
+	store, err := NewStore("", 0, logger)
+	require.Nil(err, "%+v", err)
+
 	app := NewBasecoin(DefaultHandler("mycoin"), store, logger)
-	err := app.LoadGenesis(genesisAcctFilepath)
+	err = app.LoadGenesis(genesisAcctFilepath)
 	require.Nil(err, "%+v", err)
 
 	// check the chain id

app/store.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/pkg/errors"
 	abci "github.com/tendermint/abci/types"
 	"github.com/tendermint/go-wire"
 	"github.com/tendermint/merkleeyes/iavl"
@@ -37,7 +38,7 @@ type ChainState struct {
 
 // NewStore initializes an in-memory IAVLTree, or attempts to load a persistant
 // tree from disk
-func NewStore(dbName string, cacheSize int, logger log.Logger) *Store {
+func NewStore(dbName string, cacheSize int, logger log.Logger) (*Store, error) {
 	// start at 1 so the height returned by query is for the
 	// next block, ie. the one that includes the AppHash for our current state
 	initialHeight := uint64(1)
@@ -48,17 +49,18 @@ func NewStore(dbName string, cacheSize int, logger log.Logger) *Store {
 			0,
 			nil,
 		)
-		return &Store{
+		store := &Store{
 			State:  state.NewState(tree, false),
 			height: initialHeight,
 			logger: logger,
 		}
+		return store, nil
 	}
 
 	// Expand the path fully
 	dbPath, err := filepath.Abs(dbName)
 	if err != nil {
-		panic(fmt.Sprintf("Invalid Database Name: %s", dbName))
+		return nil, errors.Wrap(err, "Invalid Database Name")
 	}
 
 	// Some external calls accidently add a ".db", which is now removed
@@ -94,22 +96,16 @@ func NewStore(dbName string, cacheSize int, logger log.Logger) *Store {
 		tree.Load(chainState.Hash)
 	}
 
-	return &Store{
+	res := &Store{
 		State:     state.NewState(tree, true),
 		height:    chainState.Height,
 		hash:      chainState.Hash,
 		persisted: true,
 		logger:    logger,
 	}
+	return res, nil
 }
 
-// CloseDB closes the database
-// func (s *Store) CloseDB() {
-// 	if s.db != nil {
-// 		s.db.Close()
-// 	}
-// }
-
 // Info implements abci.Application. It returns the height, hash and size (in the data).
 // The height is the block that holds the transactions, not the apphash itself.
 func (s *Store) Info() abci.ResponseInfo {
@@ -136,9 +132,12 @@ func (s *Store) Commit() abci.Result {
 		Height: s.height,
 	}))
 
-	hash := s.State.Commit()
+	hash, err := s.State.Commit()
+	if err != nil {
+		return abci.NewError(abci.CodeType_InternalError, err.Error())
+	}
 	if !bytes.Equal(hash, s.hash) {
-		panic("AppHash is incorrect")
+		return abci.NewError(abci.CodeType_InternalError, "AppHash is incorrect")
 	}
 
 	if s.State.Committed().Size() == 0 {

benchmarks/app_test.go

@@ -55,12 +55,16 @@ func NewBenchApp(h basecoin.Handler, chainID string, n int,
 
 	// TODO: disk writing
 	var store *app.Store
+	var err error
 
 	if persist {
 		tmpDir, _ := ioutil.TempDir("", "bc-app-benchmark")
-		store = app.NewStore(tmpDir, 500, logger)
+		store, err = app.NewStore(tmpDir, 500, logger)
 	} else {
-		store = app.NewStore("", 0, logger)
+		store, err = app.NewStore("", 0, logger)
+	}
+	if err != nil {
+		panic(err)
 	}
 
 	app := app.NewBasecoin(

benchmarks/bonsai-speed.txt

@@ -0,0 +1,19 @@
+BenchmarkMakeTx-4                   2000        603153 ns/op
+BenchmarkSimpleTransfer/100-10-nofee-memdb-4                 5000        313154 ns/op
+BenchmarkSimpleTransfer/100-10-fee-memdb-4                   5000        366534 ns/op
+BenchmarkSimpleTransfer/100-200-nofee-memdb-4                5000        296381 ns/op
+BenchmarkSimpleTransfer/100-200-fee-memdb-4                  5000        350973 ns/op
+BenchmarkSimpleTransfer/10000-10-nofee-memdb-4               5000        351425 ns/op
+BenchmarkSimpleTransfer/10000-10-fee-memdb-4                 3000        410855 ns/op
+BenchmarkSimpleTransfer/10000-200-nofee-memdb-4              5000        344839 ns/op
+BenchmarkSimpleTransfer/10000-200-fee-memdb-4                5000        394080 ns/op
+BenchmarkSimpleTransfer/100-10-nofee-persist-4               3000        433890 ns/op
+BenchmarkSimpleTransfer/100-10-fee-persist-4                 3000        496133 ns/op
+BenchmarkSimpleTransfer/100-200-nofee-persist-4              5000        310174 ns/op
+BenchmarkSimpleTransfer/100-200-fee-persist-4                5000        366868 ns/op
+BenchmarkSimpleTransfer/10000-10-nofee-persist-4             2000        815755 ns/op
+BenchmarkSimpleTransfer/10000-10-fee-persist-4               2000        874532 ns/op
+BenchmarkSimpleTransfer/10000-200-nofee-persist-4            5000        567349 ns/op
+BenchmarkSimpleTransfer/10000-200-fee-persist-4              5000        621833 ns/op
+PASS
+ok      github.com/tendermint/basecoin/benchmarks    93.047s

cmd/basecoin/commands/start.go

@@ -55,11 +55,14 @@ func init() {
 func startCmd(cmd *cobra.Command, args []string) error {
 	rootDir := viper.GetString(cli.HomeFlag)
 
-	store := app.NewStore(
+	store, err := app.NewStore(
 		path.Join(rootDir, "data", "merkleeyes.db"),
 		EyesCacheSize,
 		logger.With("module", "store"),
 	)
+	if err != nil {
+		return err
+	}
 
 	// Create Basecoin app
 	basecoinApp := app.NewBasecoin(Handler, store, logger.With("module", "app"))

docs/guide/counter/plugins/counter/counter_test.go

@@ -20,13 +20,16 @@ import (
 
 func TestCounterPlugin(t *testing.T) {
 	assert := assert.New(t)
+	require := require.New(t)
 
 	// Basecoin initialization
 	chainID := "test_chain_id"
 	logger := log.TestingLogger()
 	// logger := log.NewTracingLogger(log.NewTMLogger(os.Stdout))
 
-	store := app.NewStore("", 0, logger.With("module", "store"))
+	store, err := app.NewStore("", 0, logger.With("module", "store"))
+	require.Nil(err, "%+v", err)
+
 	h := NewHandler("gold")
 	bcApp := app.NewBasecoin(
 		h,
@@ -39,7 +42,7 @@ func TestCounterPlugin(t *testing.T) {
 	bal := coin.Coins{{"", 1000}, {"gold", 1000}}
 	acct := coin.NewAccountWithKey(bal)
 	log := bcApp.SetOption("coin/account", acct.MakeOption())
-	require.Equal(t, "Success", log)
+	require.Equal("Success", log)
 
 	// Deliver a CounterTx
 	DeliverCounterTx := func(valid bool, counterFee coin.Coins, sequence uint32) abci.Result {

state/bonsai.go

@@ -1,7 +1,6 @@
 package state
 
 import (
-	"errors"
 	"math/rand"
 
 	"github.com/tendermint/tmlibs/merkle"
@@ -47,7 +46,6 @@ func (b *Bonsai) List(start, end []byte, limit int) []Model {
 	stopAtCount := func(key []byte, value []byte) (stop bool) {
 		m := Model{key, value}
 		res = append(res, m)
-		// return false
 		return limit > 0 && len(res) >= limit
 	}
 	b.Tree.IterateRange(start, end, true, stopAtCount)
@@ -87,7 +85,7 @@ func (b *Bonsai) Checkpoint() SimpleDB {
 func (b *Bonsai) Commit(sub SimpleDB) error {
 	bb, ok := sub.(*Bonsai)
 	if !ok || (b.id != bb.id) {
-		return errors.New("Not a sub-transaction")
+		return ErrNotASubTransaction()
 	}
 	b.Tree = bb.Tree
 	return nil

state/chainstate.go

@@ -10,10 +10,12 @@ func NewChainState() *ChainState {
 	return &ChainState{}
 }
 
+var baseChainIDKey = []byte("base/chain_id")
+
 // SetChainID stores the chain id in the store
 func (s *ChainState) SetChainID(store KVStore, chainID string) {
 	s.chainID = chainID
-	store.Set([]byte("base/chain_id"), []byte(chainID))
+	store.Set(baseChainIDKey, []byte(chainID))
 }
 
 // GetChainID gets the chain id from the cache or the store
@@ -21,6 +23,6 @@ func (s *ChainState) GetChainID(store KVStore) string {
 	if s.chainID != "" {
 		return s.chainID
 	}
-	s.chainID = string(store.Get([]byte("base/chain_id")))
+	s.chainID = string(store.Get(baseChainIDKey))
 	return s.chainID
 }

state/errors.go

@@ -0,0 +1,20 @@
+//nolint
+package state
+
+import (
+	"fmt"
+
+	abci "github.com/tendermint/abci/types"
+	"github.com/tendermint/basecoin/errors"
+)
+
+var (
+	errNotASubTransaction = fmt.Errorf("Not a sub-transaction")
+)
+
+func ErrNotASubTransaction() errors.TMError {
+	return errors.WithCode(errNotASubTransaction, abci.CodeType_InternalError)
+}
+func IsNotASubTransactionErr(err error) bool {
+	return errors.IsSameError(errNotASubTransaction, err)
+}

state/kvcache.go

@@ -1,14 +1,12 @@
 package state
 
-import "errors"
-
 // MemKVCache is designed to wrap MemKVStore as a cache
 type MemKVCache struct {
 	store SimpleDB
 	cache *MemKVStore
 }
 
-var _ SimpleDB = NewMemKVStore()
+var _ SimpleDB = (*MemKVCache)(nil)
 
 // NewMemKVCache wraps a cache around MemKVStore
 //
@@ -114,7 +112,7 @@ func (c *MemKVCache) Checkpoint() SimpleDB {
 func (c *MemKVCache) Commit(sub SimpleDB) error {
 	cache, ok := sub.(*MemKVCache)
 	if !ok {
-		return errors.New("sub is not a cache")
+		return ErrNotASubTransaction()
 	}
 	// TODO: see if it points to us
 

state/merkle.go

@@ -53,10 +53,10 @@ func (s *State) BatchSet(key, value []byte) {
 }
 
 // Commit save persistent nodes to the database and re-copies the trees
-func (s *State) Commit() []byte {
+func (s *State) Commit() ([]byte, error) {
 	err := s.committed.Commit(s.deliverTx)
 	if err != nil {
-		panic(err) // ugh, TODO?
+		return nil, err
 	}
 
 	var hash []byte
@@ -68,5 +68,5 @@ func (s *State) Commit() []byte {
 
 	s.deliverTx = s.committed.Checkpoint().(*Bonsai)
 	s.checkTx = s.committed.Checkpoint().(*Bonsai)
-	return hash
+	return hash, nil
 }