diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 115748ca59..3eebb1eb4d 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -322,9 +322,8 @@ func (app *BaseApp) DeliverTx(txBytes []byte) (res abci.ResponseDeliverTx) { if result.IsOK() { app.valUpdates = append(app.valUpdates, result.ValidatorUpdates...) } else { - // Even though the Code is not OK, there will be some side - // effects, like those caused by fee deductions or sequence - // incrementations. + // Even though the Result.Code is not OK, there are still effects, + // namely fee deductions and sequence incrementing. } // Tell the blockchain engine (i.e. Tendermint). @@ -453,7 +452,7 @@ func (app *BaseApp) Commit() (res abci.ResponseCommit) { // Use the header from this latest block. app.setCheckState(header) - // Emtpy the Deliver state + // Empty the Deliver state app.deliverState = nil return abci.ResponseCommit{ diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index 49b5bc7248..0fa4f41815 100644 --- a/cmd/gaia/app/app.go +++ b/cmd/gaia/app/app.go @@ -76,7 +76,7 @@ func NewGaiaApp(logger log.Logger, dbs map[string]dbm.DB) *GaiaApp { app.MountStoreWithDB(app.capKeyStakeStore, sdk.StoreTypeIAVL, dbs["stake"]) // NOTE: Broken until #532 lands - //app.MountStoresIAVL(app.capKeyMainStore, app.capKeyIBCStore, app.capKeyStakingStore) + //app.MountStoresIAVL(app.capKeyMainStore, app.capKeyIBCStore, app.capKeyStakeStore) app.SetAnteHandler(auth.NewAnteHandler(app.accountMapper)) err := app.LoadLatestVersion(app.capKeyMainStore) if err != nil { diff --git a/cmd/gaia/app/app_test.go b/cmd/gaia/app/app_test.go index 544249a8a1..4ca1cd9590 100644 --- a/cmd/gaia/app/app_test.go +++ b/cmd/gaia/app/app_test.go @@ -88,10 +88,10 @@ var ( func loggerAndDBs() (log.Logger, map[string]dbm.DB) { logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "sdk/app") dbs := map[string]dbm.DB{ - "main": dbm.NewMemDB(), - "acc": dbm.NewMemDB(), - "ibc": dbm.NewMemDB(), - "staking": dbm.NewMemDB(), + "main": dbm.NewMemDB(), + "acc": dbm.NewMemDB(), + "ibc": dbm.NewMemDB(), + "stake": dbm.NewMemDB(), } return logger, dbs } @@ -418,12 +418,59 @@ func TestStakeMsgs(t *testing.T) { require.Equal(t, acc1, res1) require.Equal(t, acc2, res2) + // Declare Candidacy + description := stake.NewDescription("foo_moniker", "", "", "") declareCandidacyMsg := stake.NewMsgDeclareCandidacy( addr1, priv1.PubKey(), bondCoin, description, ) - SignCheckDeliver(t, gapp, declareCandidacyMsg, []int64{0}, true, priv1) + + ctxDeliver := gapp.BaseApp.NewContext(false, abci.Header{}) + res1 = gapp.accountMapper.GetAccount(ctxDeliver, addr1) + require.Equal(t, genCoins.Minus(sdk.Coins{bondCoin}), res1.GetCoins()) + candidate, found := gapp.stakeKeeper.GetCandidate(ctxDeliver, addr1) + require.True(t, found) + require.Equal(t, candidate.Address, addr1) + + // Edit Candidacy + + description = stake.NewDescription("bar_moniker", "", "", "") + editCandidacyMsg := stake.NewMsgEditCandidacy( + addr1, description, + ) + SignDeliver(t, gapp, editCandidacyMsg, []int64{1}, true, priv1) + + candidate, found = gapp.stakeKeeper.GetCandidate(ctxDeliver, addr1) + require.True(t, found) + require.Equal(t, candidate.Description, description) + + // Delegate + + delegateMsg := stake.NewMsgDelegate( + addr2, addr1, bondCoin, + ) + SignDeliver(t, gapp, delegateMsg, []int64{0}, true, priv2) + + ctxDeliver = gapp.BaseApp.NewContext(false, abci.Header{}) + res2 = gapp.accountMapper.GetAccount(ctxDeliver, addr2) + require.Equal(t, genCoins.Minus(sdk.Coins{bondCoin}), res2.GetCoins()) + bond, found := gapp.stakeKeeper.GetDelegatorBond(ctxDeliver, addr2, addr1) + require.True(t, found) + require.Equal(t, bond.DelegatorAddr, addr2) + + // Unbond + + unbondMsg := stake.NewMsgUnbond( + addr2, addr1, "MAX", + ) + SignDeliver(t, gapp, unbondMsg, []int64{1}, true, priv2) + + ctxDeliver = gapp.BaseApp.NewContext(false, abci.Header{}) + res2 = gapp.accountMapper.GetAccount(ctxDeliver, addr2) + require.Equal(t, genCoins, res2.GetCoins()) + _, found = gapp.stakeKeeper.GetDelegatorBond(ctxDeliver, addr2, addr1) + require.False(t, found) } //____________________________________________________________________________________ @@ -452,6 +499,7 @@ func SignCheckDeliver(t *testing.T, gapp *GaiaApp, msg sdk.Msg, seq []int64, exp // Sign the tx tx := genTx(msg, seq, priv...) + // Run a Check res := gapp.Check(tx) if expPass { @@ -469,5 +517,27 @@ func SignCheckDeliver(t *testing.T, gapp *GaiaApp, msg sdk.Msg, seq []int64, exp require.NotEqual(t, sdk.CodeOK, res.Code, res.Log) } gapp.EndBlock(abci.RequestEndBlock{}) + + // XXX fix code or add explaination as to why using commit breaks a bunch of these tests //gapp.Commit() } + +// XXX the only reason we are using Sign Deliver here is because the tests +// break on check tx the second time you use SignCheckDeliver in a test because +// the checktx state has not been updated likely because commit is not being +// called! +func SignDeliver(t *testing.T, gapp *GaiaApp, msg sdk.Msg, seq []int64, expPass bool, priv ...crypto.PrivKeyEd25519) { + + // Sign the tx + tx := genTx(msg, seq, priv...) + + // Simulate a Block + gapp.BeginBlock(abci.RequestBeginBlock{}) + res := gapp.Deliver(tx) + if expPass { + require.Equal(t, sdk.CodeOK, res.Code, res.Log) + } else { + require.NotEqual(t, sdk.CodeOK, res.Code, res.Log) + } + gapp.EndBlock(abci.RequestEndBlock{}) +} diff --git a/types/result.go b/types/result.go index 7b0c1a5933..f4f7454e2c 100644 --- a/types/result.go +++ b/types/result.go @@ -20,7 +20,7 @@ type Result struct { // GasWanted is the maximum units of work we allow this tx to perform. GasWanted int64 - // GasUsed is the amount of gas actually consumed. NOTE: not used. + // GasUsed is the amount of gas actually consumed. NOTE: unimplemented GasUsed int64 // Tx fee amount and denom. diff --git a/x/stake/handler.go b/x/stake/handler.go index 36b4509da4..cf36cf10a1 100644 --- a/x/stake/handler.go +++ b/x/stake/handler.go @@ -105,9 +105,6 @@ func handleMsgEditCandidacy(ctx sdk.Context, msg MsgEditCandidacy, k Keeper) sdk GasUsed: GasEditCandidacy, } } - if candidate.Status == Unbonded { //candidate has been withdrawn - return ErrBondNotNominated(k.codespace).Result() - } // XXX move to types // replace all editable fields (clients should autofill existing values) @@ -149,7 +146,7 @@ func delegate(ctx sdk.Context, k Keeper, delegatorAddr sdk.Address, bondAmt sdk.Coin, candidate Candidate) sdk.Error { // Get or create the delegator bond - bond, found := k.getDelegatorBond(ctx, delegatorAddr, candidate.Address) + bond, found := k.GetDelegatorBond(ctx, delegatorAddr, candidate.Address) if !found { bond = DelegatorBond{ DelegatorAddr: delegatorAddr, @@ -176,7 +173,7 @@ func delegate(ctx sdk.Context, k Keeper, delegatorAddr sdk.Address, func handleMsgUnbond(ctx sdk.Context, msg MsgUnbond, k Keeper) sdk.Result { // check if bond has any shares in it unbond - bond, found := k.getDelegatorBond(ctx, msg.DelegatorAddr, msg.CandidateAddr) + bond, found := k.GetDelegatorBond(ctx, msg.DelegatorAddr, msg.CandidateAddr) if !found { return ErrNoDelegatorForAddress(k.codespace).Result() } @@ -184,11 +181,7 @@ func handleMsgUnbond(ctx sdk.Context, msg MsgUnbond, k Keeper) sdk.Result { return ErrInsufficientFunds(k.codespace).Result() } - // test getting rational number from decimal provided - shares, err := sdk.NewRatFromDecimal(msg.Shares) - if err != nil { - return err.Result() - } + var shares sdk.Rat // test that there are enough shares to unbond if msg.Shares == "MAX" { @@ -196,6 +189,11 @@ func handleMsgUnbond(ctx sdk.Context, msg MsgUnbond, k Keeper) sdk.Result { return ErrNotEnoughBondShares(k.codespace, msg.Shares).Result() } } else { + var err sdk.Error + shares, err = sdk.NewRatFromDecimal(msg.Shares) + if err != nil { + return err.Result() + } if bond.Shares.LT(shares) { return ErrNotEnoughBondShares(k.codespace, msg.Shares).Result() } diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index 82b33cf2c8..00ec077125 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -92,7 +92,7 @@ func TestIncrementsMsgDelegate(t *testing.T) { //Check that the accounts and the bond account have the appropriate values candidate, found := keeper.GetCandidate(ctx, candidateAddr) require.True(t, found) - bond, found := keeper.getDelegatorBond(ctx, delegatorAddr, candidateAddr) + bond, found := keeper.GetDelegatorBond(ctx, delegatorAddr, candidateAddr) require.True(t, found) expBond := int64(i+1) * bondAmount @@ -148,7 +148,7 @@ func TestIncrementsMsgUnbond(t *testing.T) { //Check that the accounts and the bond account have the appropriate values candidate, found = keeper.GetCandidate(ctx, candidateAddr) require.True(t, found) - bond, found := keeper.getDelegatorBond(ctx, delegatorAddr, candidateAddr) + bond, found := keeper.GetDelegatorBond(ctx, delegatorAddr, candidateAddr) require.True(t, found) expBond := initBond - int64(i+1)*unbondShares @@ -263,7 +263,7 @@ func TestMultipleMsgDelegate(t *testing.T) { require.True(t, got.IsOK(), "expected msg %d to be ok, got %v", i, got) //Check that the account is bonded - bond, found := keeper.getDelegatorBond(ctx, delegatorAddr, candidateAddr) + bond, found := keeper.GetDelegatorBond(ctx, delegatorAddr, candidateAddr) require.True(t, found) require.NotNil(t, bond, "expected delegatee bond %d to exist", bond) } @@ -275,7 +275,7 @@ func TestMultipleMsgDelegate(t *testing.T) { require.True(t, got.IsOK(), "expected msg %d to be ok, got %v", i, got) //Check that the account is unbonded - _, found := keeper.getDelegatorBond(ctx, delegatorAddr, candidateAddr) + _, found := keeper.GetDelegatorBond(ctx, delegatorAddr, candidateAddr) require.False(t, found) } } diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 8b73e81db4..c2b054eba6 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -297,7 +297,8 @@ func (k Keeper) clearAccUpdateValidators(ctx sdk.Context) { //_____________________________________________________________________ -func (k Keeper) getDelegatorBond(ctx sdk.Context, +// load a delegator bong +func (k Keeper) GetDelegatorBond(ctx sdk.Context, delegatorAddr, candidateAddr sdk.Address) (bond DelegatorBond, found bool) { store := ctx.KVStore(k.storeKey) @@ -314,7 +315,7 @@ func (k Keeper) getDelegatorBond(ctx sdk.Context, } // load all bonds of a delegator -func (k Keeper) getDelegatorBonds(ctx sdk.Context, delegator sdk.Address, maxRetrieve int16) (bonds []DelegatorBond) { +func (k Keeper) GetDelegatorBonds(ctx sdk.Context, delegator sdk.Address, maxRetrieve int16) (bonds []DelegatorBond) { store := ctx.KVStore(k.storeKey) delegatorPrefixKey := GetDelegatorBondsKey(delegator, k.cdc) iterator := store.Iterator(subspace(delegatorPrefixKey)) //smallest to largest diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index e8d8e05943..9d6f69cc63 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -127,19 +127,19 @@ func TestBond(t *testing.T) { } // check the empty keeper first - _, found := keeper.getDelegatorBond(ctx, addrDels[0], addrVals[0]) + _, found := keeper.GetDelegatorBond(ctx, addrDels[0], addrVals[0]) assert.False(t, found) // set and retrieve a record keeper.setDelegatorBond(ctx, bond1to1) - resBond, found := keeper.getDelegatorBond(ctx, addrDels[0], addrVals[0]) + resBond, found := keeper.GetDelegatorBond(ctx, addrDels[0], addrVals[0]) assert.True(t, found) assert.True(t, bondsEqual(bond1to1, resBond)) // modify a records, save, and retrieve bond1to1.Shares = sdk.NewRat(99) keeper.setDelegatorBond(ctx, bond1to1) - resBond, found = keeper.getDelegatorBond(ctx, addrDels[0], addrVals[0]) + resBond, found = keeper.GetDelegatorBond(ctx, addrDels[0], addrVals[0]) assert.True(t, found) assert.True(t, bondsEqual(bond1to1, resBond)) @@ -158,16 +158,16 @@ func TestBond(t *testing.T) { keeper.setDelegatorBond(ctx, bond2to3) // test all bond retrieve capabilities - resBonds := keeper.getDelegatorBonds(ctx, addrDels[0], 5) + resBonds := keeper.GetDelegatorBonds(ctx, addrDels[0], 5) require.Equal(t, 3, len(resBonds)) assert.True(t, bondsEqual(bond1to1, resBonds[0])) assert.True(t, bondsEqual(bond1to2, resBonds[1])) assert.True(t, bondsEqual(bond1to3, resBonds[2])) - resBonds = keeper.getDelegatorBonds(ctx, addrDels[0], 3) + resBonds = keeper.GetDelegatorBonds(ctx, addrDels[0], 3) require.Equal(t, 3, len(resBonds)) - resBonds = keeper.getDelegatorBonds(ctx, addrDels[0], 2) + resBonds = keeper.GetDelegatorBonds(ctx, addrDels[0], 2) require.Equal(t, 2, len(resBonds)) - resBonds = keeper.getDelegatorBonds(ctx, addrDels[1], 5) + resBonds = keeper.GetDelegatorBonds(ctx, addrDels[1], 5) require.Equal(t, 3, len(resBonds)) assert.True(t, bondsEqual(bond2to1, resBonds[0])) assert.True(t, bondsEqual(bond2to2, resBonds[1])) @@ -175,9 +175,9 @@ func TestBond(t *testing.T) { // delete a record keeper.removeDelegatorBond(ctx, bond2to3) - _, found = keeper.getDelegatorBond(ctx, addrDels[1], addrVals[2]) + _, found = keeper.GetDelegatorBond(ctx, addrDels[1], addrVals[2]) assert.False(t, found) - resBonds = keeper.getDelegatorBonds(ctx, addrDels[1], 5) + resBonds = keeper.GetDelegatorBonds(ctx, addrDels[1], 5) require.Equal(t, 2, len(resBonds)) assert.True(t, bondsEqual(bond2to1, resBonds[0])) assert.True(t, bondsEqual(bond2to2, resBonds[1])) @@ -185,11 +185,11 @@ func TestBond(t *testing.T) { // delete all the records from delegator 2 keeper.removeDelegatorBond(ctx, bond2to1) keeper.removeDelegatorBond(ctx, bond2to2) - _, found = keeper.getDelegatorBond(ctx, addrDels[1], addrVals[0]) + _, found = keeper.GetDelegatorBond(ctx, addrDels[1], addrVals[0]) assert.False(t, found) - _, found = keeper.getDelegatorBond(ctx, addrDels[1], addrVals[1]) + _, found = keeper.GetDelegatorBond(ctx, addrDels[1], addrVals[1]) assert.False(t, found) - resBonds = keeper.getDelegatorBonds(ctx, addrDels[1], 5) + resBonds = keeper.GetDelegatorBonds(ctx, addrDels[1], 5) require.Equal(t, 0, len(resBonds)) }