Merge pull request #196 from tendermint/feature/184-overhaul-handler-…

…interface

Feature/184 overhaul handler interface

TODO.md

@@ -0,0 +1,21 @@
+Alexis:
+
+* merkle - proof (non-existence - maybe range)
+* intro to light-client and proofs
+light-client proofs:
+* make this sensible -> very tied to merkle proofs and API
+* support new proof types
+
+* expose more proof types in basecoin.Query
+
+
+* merkle - api cleanup (also Bonsai)
+* later: C bindings (to Bonsai?)
+
+
+* crypto-ledger (while ethan gone)
+
+light-client provider:
+* caching checkpoint on Verify
+* cleanup (trim old node)
+

app/app.go

@@ -1,6 +1,7 @@
 package app
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 
@@ -23,11 +24,12 @@ const (
 
 // Basecoin - The ABCI application
 type Basecoin struct {
-	info *sm.ChainState
-
+	info  *sm.ChainState
 	state *Store
 
 	handler basecoin.Handler
+
+	pending []*abci.Validator
 	height  uint64
 	logger  log.Logger
 }
@@ -65,8 +67,9 @@ func (app *Basecoin) Info() abci.ResponseInfo {
 	}
 }
 
-// SetOption - ABCI
-func (app *Basecoin) SetOption(key string, value string) string {
+// InitState - used to setup state (was SetOption)
+// to be used by InitChain later
+func (app *Basecoin) InitState(key string, value string) string {
 
 	module, key := splitKey(key)
 	state := app.state.Append()
@@ -79,13 +82,18 @@ func (app *Basecoin) SetOption(key string, value string) string {
 		return fmt.Sprintf("Error: unknown base option: %s", key)
 	}
 
-	log, err := app.handler.SetOption(app.logger, state, module, key, value)
+	log, err := app.handler.InitState(app.logger, state, module, key, value)
 	if err == nil {
 		return log
 	}
 	return "Error: " + err.Error()
 }
 
+// SetOption - ABCI
+func (app *Basecoin) SetOption(key string, value string) string {
+	return "Not Implemented"
+}
+
 // DeliverTx - ABCI
 func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result {
 	tx, err := basecoin.LoadTx(txBytes)
@@ -103,7 +111,8 @@ func (app *Basecoin) DeliverTx(txBytes []byte) abci.Result {
 	if err != nil {
 		return errors.Result(err)
 	}
-	return res.ToABCI()
+	app.addValChange(res.Diff)
+	return basecoin.ToABCI(res)
 }
 
 // CheckTx - ABCI
@@ -123,7 +132,7 @@ func (app *Basecoin) CheckTx(txBytes []byte) abci.Result {
 	if err != nil {
 		return errors.Result(err)
 	}
-	return res.ToABCI()
+	return basecoin.ToABCI(res)
 }
 
 // Query - ABCI
@@ -163,14 +172,35 @@ func (app *Basecoin) BeginBlock(hash []byte, header *abci.Header) {
 }
 
 // EndBlock - ABCI
+// Returns a list of all validator changes made in this block
 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...)
-	// }
+	// TODO: cleanup in case a validator exists multiple times in the list
+	res.Diffs = app.pending
+	app.pending = nil
 	return
 }
 
+func (app *Basecoin) addValChange(diffs []*abci.Validator) {
+	for _, d := range diffs {
+		idx := pubKeyIndex(d, app.pending)
+		if idx >= 0 {
+			app.pending[idx] = d
+		} else {
+			app.pending = append(app.pending, d)
+		}
+	}
+}
+
+// return index of list with validator of same PubKey, or -1 if no match
+func pubKeyIndex(val *abci.Validator, list []*abci.Validator) int {
+	for i, v := range list {
+		if bytes.Equal(val.PubKey, v.PubKey) {
+			return i
+		}
+	}
+	return -1
+}
+
 //TODO move split key to tmlibs?
 
 // Splits the string at the first '/'.

app/app_test.go

@@ -98,9 +98,9 @@ func (at *appTest) feeTx(coins coin.Coins, toll coin.Coin, sequence uint32) base
 	return at.signTx(tx)
 }
 
-// set the account on the app through SetOption
+// set the account on the app through InitState
 func (at *appTest) initAccount(acct *coin.AccountWithKey) {
-	res := at.app.SetOption("coin/account", acct.MakeOption())
+	res := at.app.InitState("coin/account", acct.MakeOption())
 	require.EqualValues(at.t, res, "Success")
 }
 
@@ -121,7 +121,7 @@ func (at *appTest) reset() {
 		logger.With("module", "app"),
 	)
 
-	res := at.app.SetOption("base/chain_id", at.chainID)
+	res := at.app.InitState("base/chain_id", at.chainID)
 	require.EqualValues(at.t, res, "Success")
 
 	at.initAccount(at.acctIn)
@@ -167,7 +167,7 @@ func (at *appTest) exec(t *testing.T, tx basecoin.Tx, checkTx bool) (res abci.Re
 
 //--------------------------------------------------------
 
-func TestSetOption(t *testing.T) {
+func TestInitState(t *testing.T) {
 	assert := assert.New(t)
 	require := require.New(t)
 
@@ -183,14 +183,14 @@ func TestSetOption(t *testing.T) {
 
 	//testing ChainID
 	chainID := "testChain"
-	res := app.SetOption("base/chain_id", chainID)
+	res := app.InitState("base/chain_id", chainID)
 	assert.EqualValues(app.GetChainID(), chainID)
 	assert.EqualValues(res, "Success")
 
 	// make a nice account...
 	bal := coin.Coins{{"atom", 77}, {"eth", 12}}
 	acct := coin.NewAccountWithKey(bal)
-	res = app.SetOption("coin/account", acct.MakeOption())
+	res = app.InitState("coin/account", acct.MakeOption())
 	require.EqualValues(res, "Success")
 
 	// make sure it is set correctly, with some balance
@@ -218,21 +218,21 @@ func TestSetOption(t *testing.T) {
     }
   ]
 }`
-	res = app.SetOption("coin/account", unsortAcc)
+	res = app.InitState("coin/account", unsortAcc)
 	require.EqualValues(res, "Success")
 
 	coins, err = getAddr(unsortAddr, app.GetState())
 	require.Nil(err)
 	assert.True(coins.IsValid())
 	assert.Equal(unsortCoins, coins)
 
-	res = app.SetOption("base/dslfkgjdas", "")
+	res = app.InitState("base/dslfkgjdas", "")
 	assert.NotEqual(res, "Success")
 
-	res = app.SetOption("dslfkgjdas", "")
+	res = app.InitState("dslfkgjdas", "")
 	assert.NotEqual(res, "Success")
 
-	res = app.SetOption("dslfkgjdas/szfdjzs", "")
+	res = app.InitState("dslfkgjdas/szfdjzs", "")
 	assert.NotEqual(res, "Success")
 
 }

app/app_val_test.go

@@ -0,0 +1,88 @@
+package app
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/abci/types"
+	"github.com/tendermint/basecoin/modules/base"
+	wire "github.com/tendermint/go-wire"
+	cmn "github.com/tendermint/tmlibs/common"
+	"github.com/tendermint/tmlibs/log"
+)
+
+//-----------------------------------
+// Test cases start here
+
+func randPower() uint64 {
+	return uint64(cmn.RandInt()%50 + 60)
+}
+
+func makeVal() *abci.Validator {
+	return &abci.Validator{
+		PubKey: cmn.RandBytes(10),
+		Power:  randPower(),
+	}
+}
+
+// withNewPower returns a copy of the validator with a different power
+func withNewPower(val *abci.Validator) *abci.Validator {
+	res := *val
+	res.Power = randPower()
+	if res.Power == val.Power {
+		panic("no no")
+	}
+	return &res
+}
+
+func TestEndBlock(t *testing.T) {
+	assert, require := assert.New(t), require.New(t)
+
+	logger := log.NewNopLogger()
+	store := MockStore()
+	handler := base.ValSetHandler{}
+	app := NewBasecoin(handler, store, logger)
+
+	val1 := makeVal()
+	val2 := makeVal()
+	val3 := makeVal()
+	val1a := withNewPower(val1)
+	val2a := withNewPower(val2)
+
+	cases := [...]struct {
+		changes  [][]*abci.Validator
+		expected []*abci.Validator
+	}{
+		// Nothing in, nothing out, no crash
+		0: {},
+		// One in, one out, no problem
+		1: {
+			changes:  [][]*abci.Validator{{val1}},
+			expected: []*abci.Validator{val1},
+		},
+		// Combine a few ones
+		2: {
+			changes:  [][]*abci.Validator{{val1}, {val2, val3}},
+			expected: []*abci.Validator{val1, val2, val3},
+		},
+		// Make sure changes all to one validators are squished into one diff
+		3: {
+			changes:  [][]*abci.Validator{{val1}, {val2, val1a}, {val2a, val3}},
+			expected: []*abci.Validator{val1a, val2a, val3},
+		},
+	}
+
+	for i, tc := range cases {
+		app.BeginBlock(nil, nil)
+		for _, c := range tc.changes {
+			tx := base.ValChangeTx{c}.Wrap()
+			txBytes := wire.BinaryBytes(tx)
+			res := app.DeliverTx(txBytes)
+			require.True(res.IsOK(), "%#v", res)
+		}
+		diff := app.EndBlock(app.height)
+		// TODO: don't care about order here...
+		assert.Equal(tc.expected, diff.Diffs, "%d", i)
+	}
+}

app/genesis.go

@@ -16,16 +16,16 @@ func (app *Basecoin) LoadGenesis(path string) error {
 	}
 
 	// set chain_id
-	app.SetOption("base/chain_id", genDoc.ChainID)
+	app.InitState("base/chain_id", genDoc.ChainID)
 
 	// set accounts
 	for _, acct := range genDoc.AppOptions.Accounts {
-		_ = app.SetOption("coin/account", string(acct))
+		_ = app.InitState("coin/account", string(acct))
 	}
 
 	// set plugin options
 	for _, kv := range genDoc.AppOptions.pluginOptions {
-		_ = app.SetOption(kv.Key, kv.Value)
+		_ = app.InitState(kv.Key, kv.Value)
 	}
 
 	return nil

app/store.go

@@ -36,6 +36,16 @@ type ChainState struct {
 	Height uint64
 }
 
+// MockStore returns an in-memory store only intended for testing
+func MockStore() *Store {
+	res, err := NewStore("", 0, log.NewNopLogger())
+	if err != nil {
+		// should never happen, abort test if it does
+		panic(err)
+	}
+	return res
+}
+
 // 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, error) {

benchmarks/app_test.go

@@ -72,7 +72,7 @@ func NewBenchApp(h basecoin.Handler, chainID string, n int,
 		store,
 		logger.With("module", "app"),
 	)
-	res := app.SetOption("base/chain_id", chainID)
+	res := app.InitState("base/chain_id", chainID)
 	if res != "Success" {
 		panic("cannot set chain")
 	}
@@ -82,7 +82,7 @@ func NewBenchApp(h basecoin.Handler, chainID string, n int,
 	accts := make([]*coin.AccountWithKey, n)
 	for i := 0; i < n; i++ {
 		accts[i] = coin.NewAccountWithKey(money)
-		res := app.SetOption("coin/account", accts[i].MakeOption())
+		res := app.InitState("coin/account", accts[i].MakeOption())
 		if res != "Success" {
 			panic("can't set account")
 		}

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

@@ -114,7 +114,8 @@ func NewHandler(feeDenom string) basecoin.Handler {
 
 // Handler the counter transaction processing handler
 type Handler struct {
-	stack.NopOption
+	stack.PassInitState
+	stack.PassInitValidate
 }
 
 var _ stack.Dispatchable = Handler{}
@@ -128,13 +129,13 @@ func (Handler) Name() string {
 func (Handler) AssertDispatcher() {}
 
 // CheckTx checks if the tx is properly structured
-func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, _ basecoin.Checker) (res basecoin.Result, err error) {
+func (h Handler) CheckTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, _ basecoin.Checker) (res basecoin.CheckResult, err error) {
 	_, err = checkTx(ctx, tx)
 	return
 }
 
 // DeliverTx executes the tx if valid
-func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, dispatch basecoin.Deliver) (res basecoin.Result, err error) {
+func (h Handler) DeliverTx(ctx basecoin.Context, store state.SimpleDB, tx basecoin.Tx, dispatch basecoin.Deliver) (res basecoin.DeliverResult, err error) {
 	ctr, err := checkTx(ctx, tx)
 	if err != nil {
 		return res, err

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

@@ -36,12 +36,12 @@ func TestCounterPlugin(t *testing.T) {
 		store,
 		logger.With("module", "app"),
 	)
-	bcApp.SetOption("base/chain_id", chainID)
+	bcApp.InitState("base/chain_id", chainID)
 
 	// Account initialization
 	bal := coin.Coins{{"", 1000}, {"gold", 1000}}
 	acct := coin.NewAccountWithKey(bal)
-	log := bcApp.SetOption("coin/account", acct.MakeOption())
+	log := bcApp.InitState("coin/account", acct.MakeOption())
 	require.Equal("Success", log)
 
 	// Deliver a CounterTx