From 952c66b459c2b1cb94703fb30a524b9ed6f2bf6b Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 16 Jun 2023 17:53:51 +0200 Subject: [PATCH 01/16] refactor: baseapp audit changes --- baseapp/abci_test.go | 141 ++++++++++++++++++++++++++++--------------- 1 file changed, 91 insertions(+), 50 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 9d12ed1ac121..f9c6118db658 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -3,6 +3,7 @@ package baseapp_test import ( "bytes" "context" + "encoding/hex" "errors" "fmt" "strings" @@ -50,12 +51,14 @@ func TestABCI_First_block_Height(t *testing.T) { suite := NewBaseAppSuite(t, baseapp.SetChainID("test-chain-id")) app := suite.baseApp - app.InitChain(&abci.RequestInitChain{ + _, err := app.InitChain(&abci.RequestInitChain{ ChainId: "test-chain-id", ConsensusParams: &cmtproto.ConsensusParams{Block: &cmtproto.BlockParams{MaxGas: 5000000}}, InitialHeight: 1, }) - app.Commit() + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) ctx := app.GetContextForCheckTx(nil) require.Equal(t, int64(1), ctx.BlockHeight()) @@ -84,8 +87,8 @@ func TestABCI_InitChain(t *testing.T) { Data: key, } + // initChain is nil and chain ID is wrong - errors _, err := app.InitChain(&abci.RequestInitChain{ChainId: "wrong-chain-id"}) - // initChain is nil and chain ID is wrong - panics require.Error(t, err) // initChain is nil - nothing happens @@ -109,9 +112,12 @@ func TestABCI_InitChain(t *testing.T) { // The AppHash returned by a new chain is the sha256 hash of "". // $ echo -n '' | sha256sum // e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 + apphash, err := hex.DecodeString("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") + require.NoError(t, err) + require.Equal( t, - []byte{0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55}, + apphash, initChainRes.AppHash, ) @@ -122,12 +128,15 @@ func TestABCI_InitChain(t *testing.T) { chainID = getCheckStateCtx(app).ChainID() require.Equal(t, "test-chain-id", chainID, "ChainID in checkState not set correctly in InitChain") - app.FinalizeBlock(&abci.RequestFinalizeBlock{ + _, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{ Hash: initChainRes.AppHash, Height: 1, }) + require.NoError(t, err) + + _, err = app.Commit() + require.NoError(t, err) - app.Commit() resQ, err = app.Query(context.TODO(), &query) require.NoError(t, err) require.Equal(t, int64(1), app.LastBlockHeight()) @@ -147,8 +156,10 @@ func TestABCI_InitChain(t *testing.T) { require.Equal(t, value, resQ.Value) // commit and ensure we can still query - app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: app.LastBlockHeight() + 1}) - app.Commit() + _, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: app.LastBlockHeight() + 1}) + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) resQ, err = app.Query(context.TODO(), &query) require.NoError(t, err) @@ -160,12 +171,14 @@ func TestABCI_InitChain_WithInitialHeight(t *testing.T) { db := dbm.NewMemDB() app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil) - app.InitChain( + _, err := app.InitChain( &abci.RequestInitChain{ InitialHeight: 3, }, ) - app.Commit() + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) require.Equal(t, int64(3), app.LastBlockHeight()) } @@ -175,17 +188,20 @@ func TestABCI_FinalizeBlock_WithInitialHeight(t *testing.T) { db := dbm.NewMemDB() app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil) - app.InitChain( + _, err := app.InitChain( &abci.RequestInitChain{ InitialHeight: 3, }, ) + require.NoError(t, err) - _, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 4}) + _, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 4}) require.Error(t, err, "invalid height: 4; expected: 3") - app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 3}) - app.Commit() + _, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 3}) + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) require.Equal(t, int64(3), app.LastBlockHeight()) } @@ -200,9 +216,10 @@ func TestABCI_GRPCQuery(t *testing.T) { suite := NewBaseAppSuite(t, grpcQueryOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) req := testdata.SayHelloRequest{Name: "foo"} reqBz, err := req.Marshal() @@ -216,8 +233,10 @@ func TestABCI_GRPCQuery(t *testing.T) { require.Equal(t, sdkerrors.ErrInvalidHeight.ABCICode(), resQuery.Code, resQuery) require.Contains(t, resQuery.Log, "TestABCI_GRPCQuery is not ready; please wait for first block") - suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: suite.baseApp.LastBlockHeight() + 1}) - suite.baseApp.Commit() + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: suite.baseApp.LastBlockHeight() + 1}) + require.NoError(t, err) + _, err = suite.baseApp.Commit() + require.NoError(t, err) reqQuery := abci.RequestQuery{ Data: reqBz, @@ -278,9 +297,10 @@ func TestBaseApp_PrepareCheckState(t *testing.T) { app := baseapp.NewBaseApp(name, logger, db, nil) app.SetParamStore(¶mStore{db: dbm.NewMemDB()}) - app.InitChain(&abci.RequestInitChain{ + _, err := app.InitChain(&abci.RequestInitChain{ ConsensusParams: cp, }) + require.NoError(t, err) wasPrepareCheckStateCalled := false app.SetPrepareCheckStater(func(ctx sdk.Context) { @@ -288,7 +308,8 @@ func TestBaseApp_PrepareCheckState(t *testing.T) { }) app.Seal() - app.Commit() + _, err = app.Commit() + require.NoError(t, err) require.Equal(t, true, wasPrepareCheckStateCalled) } @@ -305,9 +326,10 @@ func TestBaseApp_Precommit(t *testing.T) { app := baseapp.NewBaseApp(name, logger, db, nil) app.SetParamStore(¶mStore{db: dbm.NewMemDB()}) - app.InitChain(&abci.RequestInitChain{ + _, err := app.InitChain(&abci.RequestInitChain{ ConsensusParams: cp, }) + require.NoError(t, err) wasPrecommiterCalled := false app.SetPrecommiter(func(ctx sdk.Context) { @@ -330,9 +352,10 @@ func TestABCI_CheckTx(t *testing.T) { baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImpl{t, capKey1, counterKey}) nTxs := int64(5) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) for i := int64(0); i < nTxs; i++ { tx := newTxCounter(t, suite.txConfig, i, 0) // no messages @@ -352,7 +375,7 @@ func TestABCI_CheckTx(t *testing.T) { require.Equal(t, nTxs, storedCounter) // if a block is committed, CheckTx state should be reset - _, err := suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: 1, Hash: []byte("hash"), }) @@ -373,9 +396,10 @@ func TestABCI_FinalizeBlock_DeliverTx(t *testing.T) { anteOpt := func(bapp *baseapp.BaseApp) { bapp.SetAnteHandler(anteHandlerTxTest(t, capKey1, anteKey)) } suite := NewBaseAppSuite(t, anteOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) deliverKey := []byte("deliver-key") baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey}) @@ -421,9 +445,10 @@ func TestABCI_FinalizeBlock_MultiMsg(t *testing.T) { anteOpt := func(bapp *baseapp.BaseApp) { bapp.SetAnteHandler(anteHandlerTxTest(t, capKey1, anteKey)) } suite := NewBaseAppSuite(t, anteOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) deliverKey := []byte("deliver-key") baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey}) @@ -499,9 +524,10 @@ func TestABCI_Query_SimulateTx(t *testing.T) { } suite := NewBaseAppSuite(t, anteOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImplGasMeterOnly{gasConsumed}) @@ -558,9 +584,10 @@ func TestABCI_InvalidTransaction(t *testing.T) { suite := NewBaseAppSuite(t, anteOpt) baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImplGasMeterOnly{}) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: 1, @@ -689,15 +716,18 @@ func TestABCI_TxGasLimits(t *testing.T) { suite := NewBaseAppSuite(t, anteOpt) baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImplGasMeterOnly{}) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) - suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: 1, }) + require.NoError(t, err) - suite.baseApp.Commit() + _, err = suite.baseApp.Commit() + require.NoError(t, err) testCases := []struct { tx signing.Tx @@ -778,15 +808,17 @@ func TestABCI_MaxBlockGasLimits(t *testing.T) { suite := NewBaseAppSuite(t, anteOpt) baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImplGasMeterOnly{}) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{ Block: &cmtproto.BlockParams{ MaxGas: 100, }, }, }) + require.NoError(t, err) - suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) + require.NoError(t, err) testCases := []struct { tx signing.Tx @@ -811,7 +843,8 @@ func TestABCI_MaxBlockGasLimits(t *testing.T) { tx := tc.tx // reset block gas - suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: suite.baseApp.LastBlockHeight() + 1}) + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: suite.baseApp.LastBlockHeight() + 1}) + require.NoError(t, err) // execute the transaction multiple times for j := 0; j < tc.numDelivers; j++ { @@ -876,13 +909,14 @@ func TestABCI_GasConsumptionBadTx(t *testing.T) { suite := NewBaseAppSuite(t, anteOpt) baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImplGasMeterOnly{}) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{ Block: &cmtproto.BlockParams{ MaxGas: 9, }, }, }) + require.NoError(t, err) tx := newTxCounter(t, suite.txConfig, 5, 0) tx = setFailOnAnte(t, suite.txConfig, tx, true) @@ -914,9 +948,10 @@ func TestABCI_Query(t *testing.T) { suite := NewBaseAppSuite(t, anteOpt) baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImplGasMeterOnly{}) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) // NOTE: "/store/key1" tells us KVStore // and the final "/key" says to use the data as the @@ -1057,13 +1092,14 @@ func TestABCI_GetBlockRetentionHeight(t *testing.T) { tc := tc tc.bapp.SetParamStore(¶mStore{db: dbm.NewMemDB()}) - tc.bapp.InitChain(&abci.RequestInitChain{ + _, err = tc.bapp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{ Evidence: &cmtproto.EvidenceParams{ MaxAgeNumBlocks: tc.maxAgeBlocks, }, }, }) + require.NoError(t, err) t.Run(name, func(t *testing.T) { require.Equal(t, tc.expected, tc.bapp.GetBlockRetentionHeight(tc.commitHeight)) @@ -1125,9 +1161,10 @@ func TestABCI_Proposal_HappyPath(t *testing.T) { baseapptestutil.RegisterKeyValueServer(suite.baseApp.MsgServiceRouter(), MsgKeyValueImpl{}) baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), NoopCounterServerImpl{}) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) tx := newTxCounter(t, suite.txConfig, 0, 1) txBytes, err := suite.txConfig.TxEncoder()(tx) @@ -1169,13 +1206,14 @@ func TestABCI_Proposal_HappyPath(t *testing.T) { require.NoError(t, err) require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status) + // the same txs as in PrepareProposal res, err := suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: suite.baseApp.LastBlockHeight() + 1, - Txs: [][]byte{txBytes}, + Txs: reqProposalTxBytes[:], }) require.NoError(t, err) - require.Equal(t, 1, pool.CountTx()) + require.Equal(t, 0, pool.CountTx()) require.NotEmpty(t, res.TxResults[0].Events) require.True(t, res.TxResults[0].IsOK(), fmt.Sprintf("%v", res)) @@ -1202,10 +1240,11 @@ func TestABCI_Proposal_Read_State_PrepareProposal(t *testing.T) { suite := NewBaseAppSuite(t, setInitChainerOpt, prepareOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ InitialHeight: 1, ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) reqPrepareProposal := abci.RequestPrepareProposal{ MaxTxBytes: 1000, @@ -1224,10 +1263,6 @@ func TestABCI_Proposal_Read_State_PrepareProposal(t *testing.T) { resProcessProposal, err := suite.baseApp.ProcessProposal(&reqProcessProposal) require.NoError(t, err) require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status) - - // suite.baseApp.BeginBlock(abci.RequestBeginBlock{ - // Header: cmtproto.Header{Height: suite.baseApp.LastBlockHeight() + 1}, - // }) } func TestABCI_PrepareProposal_ReachedMaxBytes(t *testing.T) { @@ -1238,9 +1273,10 @@ func TestABCI_PrepareProposal_ReachedMaxBytes(t *testing.T) { } suite := NewBaseAppSuite(t, anteOpt, baseapp.SetMempool(pool)) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) for i := 0; i < 100; i++ { tx2 := newTxCounter(t, suite.txConfig, int64(i), int64(i)) @@ -1265,12 +1301,13 @@ func TestABCI_PrepareProposal_BadEncoding(t *testing.T) { } suite := NewBaseAppSuite(t, anteOpt, baseapp.SetMempool(pool)) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) tx := newTxCounter(t, suite.txConfig, 0, 0) - err := pool.Insert(sdk.Context{}, tx) + err = pool.Insert(sdk.Context{}, tx) require.NoError(t, err) reqPrepareProposal := abci.RequestPrepareProposal{ @@ -1290,9 +1327,10 @@ func TestABCI_PrepareProposal_Failures(t *testing.T) { } suite := NewBaseAppSuite(t, anteOpt, baseapp.SetMempool(pool)) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) tx := newTxCounter(t, suite.txConfig, 0, 0) txBytes, err := suite.txConfig.TxEncoder()(tx) @@ -1330,9 +1368,10 @@ func TestABCI_PrepareProposal_PanicRecovery(t *testing.T) { } suite := NewBaseAppSuite(t, prepareOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) req := abci.RequestPrepareProposal{ MaxTxBytes: 1000, @@ -1354,9 +1393,10 @@ func TestABCI_ProcessProposal_PanicRecovery(t *testing.T) { } suite := NewBaseAppSuite(t, processOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) require.NotPanics(t, func() { res, err := suite.baseApp.ProcessProposal(&abci.RequestProcessProposal{Height: 1}) @@ -1392,9 +1432,10 @@ func TestABCI_Proposal_Reset_State_Between_Calls(t *testing.T) { suite := NewBaseAppSuite(t, prepareOpt, processOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) reqPrepareProposal := abci.RequestPrepareProposal{ MaxTxBytes: 1000, From bb27c529118dc733b77805e2b288b90dd471534e Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 16 Jun 2023 17:59:38 +0200 Subject: [PATCH 02/16] more test fixes --- baseapp/abci_test.go | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index f9c6118db658..ec1ffc932cec 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -337,7 +337,8 @@ func TestBaseApp_Precommit(t *testing.T) { }) app.Seal() - app.Commit() + _, err = app.Commit() + require.NoError(t, err) require.Equal(t, true, wasPrecommiterCalled) } @@ -384,7 +385,8 @@ func TestABCI_CheckTx(t *testing.T) { require.NotNil(t, getCheckStateCtx(suite.baseApp).BlockGasMeter(), "block gas meter should have been set to checkState") require.NotEmpty(t, getCheckStateCtx(suite.baseApp).HeaderHash()) - suite.baseApp.Commit() + _, err = suite.baseApp.Commit() + require.NoError(t, err) checkStateStore = getCheckStateCtx(suite.baseApp).KVStore(capKey1) storedBytes := checkStateStore.Get(counterKey) @@ -436,7 +438,8 @@ func TestABCI_FinalizeBlock_DeliverTx(t *testing.T) { require.Equal(t, sdk.MarkEventsToIndex(counterEvent(sdk.EventTypeMessage, counter).ToABCIEvents(), map[string]struct{}{})[0].Attributes[0], events[2].Attributes[0], "msg handler update counter event") } - suite.baseApp.Commit() + _, err = suite.baseApp.Commit() + require.NoError(t, err) } } @@ -462,10 +465,11 @@ func TestABCI_FinalizeBlock_MultiMsg(t *testing.T) { txBytes, err := suite.txConfig.TxEncoder()(tx) require.NoError(t, err) - suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: 1, Txs: [][]byte{txBytes}, }) + require.NoError(t, err) store := getFinalizeBlockStateCtx(suite.baseApp).KVStore(capKey1) @@ -569,8 +573,10 @@ func TestABCI_Query_SimulateTx(t *testing.T) { require.Equal(t, result.Events, simRes.Result.Events) require.True(t, bytes.Equal(result.Data, simRes.Result.Data)) - suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: count}) - suite.baseApp.Commit() + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: count}) + require.NoError(t, err) + _, err = suite.baseApp.Commit() + require.NoError(t, err) } } @@ -589,9 +595,10 @@ func TestABCI_InvalidTransaction(t *testing.T) { }) require.NoError(t, err) - suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: 1, }) + require.NoError(t, err) // transaction with no messages { @@ -979,7 +986,7 @@ func TestABCI_Query(t *testing.T) { bz, err := suite.txConfig.TxEncoder()(tx) require.NoError(t, err) - suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ + _, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{ Height: 1, Txs: [][]byte{bz}, }) @@ -990,7 +997,8 @@ func TestABCI_Query(t *testing.T) { require.Equal(t, 0, len(res.Value)) // query returns correct value after Commit - suite.baseApp.Commit() + _, err = suite.baseApp.Commit() + require.NoError(t, err) res, err = suite.baseApp.Query(context.TODO(), &query) require.NoError(t, err) @@ -1122,8 +1130,10 @@ func TestPrepareCheckStateCalledWithCheckState(t *testing.T) { wasPrepareCheckStateCalled = true }) - app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) - app.Commit() + _, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) require.Equal(t, true, wasPrepareCheckStateCalled) } @@ -1144,8 +1154,10 @@ func TestPrecommiterCalledWithDeliverState(t *testing.T) { wasPrecommiterCalled = true }) - app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) - app.Commit() + _, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) require.Equal(t, true, wasPrecommiterCalled) } From a7206f06cf73b024ced828e2a421af7b6725af43 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 23 Jun 2023 12:56:33 +0200 Subject: [PATCH 03/16] improve tests --- baseapp/abci_test.go | 67 +++++++++++++++++++++++++++++++++++++++++ baseapp/baseapp_test.go | 26 +++++++++++----- 2 files changed, 85 insertions(+), 8 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index ec1ffc932cec..146d5d579b5e 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -206,6 +206,73 @@ func TestABCI_FinalizeBlock_WithInitialHeight(t *testing.T) { require.Equal(t, int64(3), app.LastBlockHeight()) } +func TestABCI_FinalizeBlock_WithBeginAndEndBlocker(t *testing.T) { + name := t.Name() + db := dbm.NewMemDB() + app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil) + + app.SetBeginBlocker(func(ctx sdk.Context) (sdk.BeginBlock, error) { + return sdk.BeginBlock{ + Events: []abci.Event{ + { + Type: "sometype", + Attributes: []abci.EventAttribute{ + { + Key: "foo", + Value: "bar", + }, + }, + }, + }, + }, nil + }) + + app.SetEndBlocker(func(ctx sdk.Context) (sdk.EndBlock, error) { + return sdk.EndBlock{ + Events: []abci.Event{ + { + Type: "anothertype", + Attributes: []abci.EventAttribute{ + { + Key: "foo", + Value: "bar", + }, + }, + }, + }, + }, nil + }) + + _, err := app.InitChain( + &abci.RequestInitChain{ + InitialHeight: 1, + }, + ) + require.NoError(t, err) + + res, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) + require.NoError(t, err) + + require.Len(t, res.Events, 2) + + require.Equal(t, "sometype", res.Events[0].Type) + require.Equal(t, "foo", res.Events[0].Attributes[0].Key) + require.Equal(t, "bar", res.Events[0].Attributes[0].Value) + require.Equal(t, "mode", res.Events[0].Attributes[1].Key) + require.Equal(t, "BeginBlock", res.Events[0].Attributes[1].Value) + + require.Equal(t, "anothertype", res.Events[1].Type) + require.Equal(t, "foo", res.Events[1].Attributes[0].Key) + require.Equal(t, "bar", res.Events[1].Attributes[0].Value) + require.Equal(t, "mode", res.Events[1].Attributes[1].Key) + require.Equal(t, "EndBlock", res.Events[1].Attributes[1].Value) + + _, err = app.Commit() + require.NoError(t, err) + + require.Equal(t, int64(1), app.LastBlockHeight()) +} + func TestABCI_GRPCQuery(t *testing.T) { grpcQueryOpt := func(bapp *baseapp.BaseApp) { testdata.RegisterQueryServer( diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 44c91901f3fc..5d970aa0ff64 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -20,6 +20,7 @@ import ( "cosmossdk.io/store/snapshots" snapshottypes "cosmossdk.io/store/snapshots/types" storetypes "cosmossdk.io/store/types" + "github.com/cosmos/cosmos-sdk/baseapp" baseapptestutil "github.com/cosmos/cosmos-sdk/baseapp/testutil" "github.com/cosmos/cosmos-sdk/client" @@ -100,9 +101,10 @@ func NewBaseAppSuiteWithSnapshots(t *testing.T, cfg SnapshotsConfig, opts ...fun baseapptestutil.RegisterKeyValueServer(suite.baseApp.MsgServiceRouter(), MsgKeyValueImpl{}) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err = suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) r := rand.New(rand.NewSource(3920758213583)) keyCounter := 0 @@ -455,9 +457,10 @@ func TestCustomRunTxPanicHandler(t *testing.T) { } suite := NewBaseAppSuite(t, anteOpt) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) suite.baseApp.AddRunTxRecoveryHandler(func(recoveryObj interface{}) error { err, ok := recoveryObj.(error) @@ -494,9 +497,10 @@ func TestBaseAppAnteHandler(t *testing.T) { deliverKey := []byte("deliver-key") baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey}) - suite.baseApp.InitChain(&abci.RequestInitChain{ + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ ConsensusParams: &cmtproto.ConsensusParams{}, }) + require.NoError(t, err) // execute a tx that will fail ante handler execution // @@ -566,11 +570,15 @@ func TestABCI_CreateQueryContext(t *testing.T) { name := t.Name() app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil) - app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) - app.Commit() + _, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1}) + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) - app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 2}) - app.Commit() + _, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 2}) + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) testCases := []struct { name string @@ -606,7 +614,9 @@ func TestSetMinGasPrices(t *testing.T) { func TestGetMaximumBlockGas(t *testing.T) { suite := NewBaseAppSuite(t) - suite.baseApp.InitChain(&abci.RequestInitChain{}) + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{}) + require.NoError(t, err) + ctx := suite.baseApp.NewContext(true) suite.baseApp.StoreConsensusParams(ctx, cmtproto.ConsensusParams{Block: &cmtproto.BlockParams{MaxGas: 0}}) From 6006adc813c982d3398748a025e96bcb3f7e7644 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 23 Jun 2023 13:39:18 +0200 Subject: [PATCH 04/16] improve tests + extendVote --- baseapp/abci.go | 5 +++-- baseapp/abci_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index ded22c971933..5b20201ddda6 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -557,7 +557,7 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( // If vote extensions are not enabled, as a safety precaution, we return an // error. cp := app.GetConsensusParams(app.voteExtensionState.ctx) - if cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight <= 0 { + if cp.Abci != nil && req.Height < cp.Abci.VoteExtensionsEnableHeight { return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) } @@ -570,6 +570,7 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( WithHeaderInfo(coreheader.Info{ ChainID: app.chainID, Height: req.Height, + Hash: req.Hash, }) // add a deferred recover handler in case extendVote panics @@ -608,7 +609,7 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r // If vote extensions are not enabled, as a safety precaution, we return an // error. cp := app.GetConsensusParams(app.voteExtensionState.ctx) - if cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight <= 0 { + if cp.Abci != nil && req.Height < cp.Abci.VoteExtensionsEnableHeight { return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to VerifyVoteExtension at height %d", req.Height) } diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 146d5d579b5e..8797e411082d 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "errors" "fmt" + "strconv" "strings" "testing" @@ -273,6 +274,53 @@ func TestABCI_FinalizeBlock_WithBeginAndEndBlocker(t *testing.T) { require.Equal(t, int64(1), app.LastBlockHeight()) } +func TestABCI_ExtendVote(t *testing.T) { + name := t.Name() + db := dbm.NewMemDB() + app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil) + + app.SetExtendVoteHandler(func(ctx sdk.Context, req *abci.RequestExtendVote) (*abci.ResponseExtendVote, error) { + voteExt := []byte("foo") + voteExt = append(voteExt, req.Hash...) + voteExt = append(voteExt, []byte(strconv.FormatInt(req.Height, 10))...) + return &abci.ResponseExtendVote{VoteExtension: voteExt}, nil + }) + + app.SetParamStore(¶mStore{db: dbm.NewMemDB()}) + _, err := app.InitChain( + &abci.RequestInitChain{ + InitialHeight: 1, + ConsensusParams: &cmtproto.ConsensusParams{ + Abci: &cmtproto.ABCIParams{ + VoteExtensionsEnableHeight: 200, + }, + }, + }, + ) + require.NoError(t, err) + + // Votes not enabled yet + _, err = app.ExtendVote(context.Background(), &abci.RequestExtendVote{Height: 123, Hash: []byte("thehash")}) + require.ErrorContains(t, err, "vote extensions are not enabled") + + // First vote on the first enabled height + res, err := app.ExtendVote(context.Background(), &abci.RequestExtendVote{Height: 200, Hash: []byte("thehash")}) + require.NoError(t, err) + require.Len(t, res.VoteExtension, 13) + + res, err = app.ExtendVote(context.Background(), &abci.RequestExtendVote{Height: 1000, Hash: []byte("thehash")}) + require.NoError(t, err) + require.Len(t, res.VoteExtension, 14) + + // Error during vote extension should return an empty vote extension and no error + app.SetExtendVoteHandler(func(ctx sdk.Context, req *abci.RequestExtendVote) (*abci.ResponseExtendVote, error) { + return nil, errors.New("some error") + }) + res, err = app.ExtendVote(context.Background(), &abci.RequestExtendVote{Height: 1000, Hash: []byte("thehash")}) + require.NoError(t, err) + require.Len(t, res.VoteExtension, 0) +} + func TestABCI_GRPCQuery(t *testing.T) { grpcQueryOpt := func(bapp *baseapp.BaseApp) { testdata.RegisterQueryServer( From d947df161302452414de2977029a2d0a56e78293 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 23 Jun 2023 13:56:50 +0200 Subject: [PATCH 05/16] improve tests --- baseapp/abci_test.go | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 8797e411082d..aca3d6677e50 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -280,10 +280,19 @@ func TestABCI_ExtendVote(t *testing.T) { app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil) app.SetExtendVoteHandler(func(ctx sdk.Context, req *abci.RequestExtendVote) (*abci.ResponseExtendVote, error) { - voteExt := []byte("foo") - voteExt = append(voteExt, req.Hash...) - voteExt = append(voteExt, []byte(strconv.FormatInt(req.Height, 10))...) - return &abci.ResponseExtendVote{VoteExtension: voteExt}, nil + voteExt := "foo" + hex.EncodeToString(req.Hash) + strconv.FormatInt(req.Height, 10) + return &abci.ResponseExtendVote{VoteExtension: []byte(voteExt)}, nil + }) + + app.SetVerifyVoteExtensionHandler(func(ctx sdk.Context, req *abci.RequestVerifyVoteExtension) (*abci.ResponseVerifyVoteExtension, error) { + // do some kind of verification here + expectedVoteExt := "foo" + hex.EncodeToString(req.Hash) + strconv.FormatInt(req.Height, 10) + if !bytes.Equal(req.VoteExtension, []byte(expectedVoteExt)) { + fmt.Println(expectedVoteExt) + return &abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_REJECT}, nil + } + + return &abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_ACCEPT}, nil }) app.SetParamStore(¶mStore{db: dbm.NewMemDB()}) @@ -306,11 +315,11 @@ func TestABCI_ExtendVote(t *testing.T) { // First vote on the first enabled height res, err := app.ExtendVote(context.Background(), &abci.RequestExtendVote{Height: 200, Hash: []byte("thehash")}) require.NoError(t, err) - require.Len(t, res.VoteExtension, 13) + require.Len(t, res.VoteExtension, 20) res, err = app.ExtendVote(context.Background(), &abci.RequestExtendVote{Height: 1000, Hash: []byte("thehash")}) require.NoError(t, err) - require.Len(t, res.VoteExtension, 14) + require.Len(t, res.VoteExtension, 21) // Error during vote extension should return an empty vote extension and no error app.SetExtendVoteHandler(func(ctx sdk.Context, req *abci.RequestExtendVote) (*abci.ResponseExtendVote, error) { @@ -319,6 +328,24 @@ func TestABCI_ExtendVote(t *testing.T) { res, err = app.ExtendVote(context.Background(), &abci.RequestExtendVote{Height: 1000, Hash: []byte("thehash")}) require.NoError(t, err) require.Len(t, res.VoteExtension, 0) + + // Verify Vote Extensions + _, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 123, VoteExtension: []byte("1234567")}) + require.ErrorContains(t, err, "vote extensions are not enabled") + + // First vote on the first enabled height + vres, err := app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 200, Hash: []byte("thehash"), VoteExtension: []byte("foo74686568617368200")}) + require.NoError(t, err) + require.Equal(t, abci.ResponseVerifyVoteExtension_ACCEPT, vres.Status) + + vres, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 1000, Hash: []byte("thehash"), VoteExtension: []byte("foo746865686173681000")}) + require.NoError(t, err) + require.Equal(t, abci.ResponseVerifyVoteExtension_ACCEPT, vres.Status) + + // Reject because it's just some random bytes + vres, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 201, Hash: []byte("thehash"), VoteExtension: []byte("12345678")}) + require.NoError(t, err) + require.Equal(t, abci.ResponseVerifyVoteExtension_REJECT, vres.Status) } func TestABCI_GRPCQuery(t *testing.T) { From df207494db9f33d75207903c251f005342cec6bf Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 23 Jun 2023 13:58:57 +0200 Subject: [PATCH 06/16] test --- baseapp/abci_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index aca3d6677e50..88e91849cbe3 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -346,6 +346,14 @@ func TestABCI_ExtendVote(t *testing.T) { vres, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 201, Hash: []byte("thehash"), VoteExtension: []byte("12345678")}) require.NoError(t, err) require.Equal(t, abci.ResponseVerifyVoteExtension_REJECT, vres.Status) + + // Reject because the verification failed (no error) + app.SetVerifyVoteExtensionHandler(func(ctx sdk.Context, req *abci.RequestVerifyVoteExtension) (*abci.ResponseVerifyVoteExtension, error) { + return nil, errors.New("some error") + }) + vres, err = app.VerifyVoteExtension(&abci.RequestVerifyVoteExtension{Height: 201, Hash: []byte("thehash"), VoteExtension: []byte("12345678")}) + require.NoError(t, err) + require.Equal(t, abci.ResponseVerifyVoteExtension_REJECT, vres.Status) } func TestABCI_GRPCQuery(t *testing.T) { From c1117b3e1b4cb0ef3011406b3961cd045e8b482a Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 23 Jun 2023 14:09:21 +0200 Subject: [PATCH 07/16] cl++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 269c13c8c77a..2c65518db659 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#16547](https://github.com/cosmos/cosmos-sdk/pull/16547) Ensure a transaction's gas limit cannot exceed the block gas limit. * (x/auth/types) [#16554](https://github.com/cosmos/cosmos-sdk/pull/16554) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. * (baseapp) [#16613](https://github.com/cosmos/cosmos-sdk/pull/16613) Ensure each message in a transaction has a registered handler, otherwise `CheckTx` will fail. +* (baseapp) [#16596](https://github.com/cosmos/cosmos-sdk/pull/16596) Return error during ExtendVote and VerifyVoteExtension if the request height is earlier than `VoteExtensionsEnableHeight`. ## [v0.50.0-alpha.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-alpha.0) - 2023-06-07 From a90066fc2f19a3c5a96e2a305ebc0ba7090389b8 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Mon, 26 Jun 2023 20:42:51 +0200 Subject: [PATCH 08/16] more testing --- baseapp/abci_test.go | 122 +++++++++++++++++++++++++++++++++++++++++- baseapp/abci_utils.go | 13 ++--- scripts/mockgen.sh | 1 + 3 files changed, 129 insertions(+), 7 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 76f6c7821e7e..df4908a91561 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -11,20 +11,28 @@ import ( "testing" abci "github.com/cometbft/cometbft/abci/types" + cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" dbm "github.com/cosmos/cosmos-db" + protoio "github.com/cosmos/gogoproto/io" "github.com/cosmos/gogoproto/jsonpb" + "github.com/cosmos/gogoproto/proto" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" errorsmod "cosmossdk.io/errors" "cosmossdk.io/log" + "cosmossdk.io/math" pruningtypes "cosmossdk.io/store/pruning/types" "cosmossdk.io/store/snapshots" snapshottypes "cosmossdk.io/store/snapshots/types" storetypes "cosmossdk.io/store/types" + "github.com/cometbft/cometbft/crypto/secp256k1" + "github.com/cosmos/cosmos-sdk/baseapp" baseapptestutil "github.com/cosmos/cosmos-sdk/baseapp/testutil" + "github.com/cosmos/cosmos-sdk/baseapp/testutil/mock" "github.com/cosmos/cosmos-sdk/testutil" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" @@ -287,7 +295,6 @@ func TestABCI_ExtendVote(t *testing.T) { // do some kind of verification here expectedVoteExt := "foo" + hex.EncodeToString(req.Hash) + strconv.FormatInt(req.Height, 10) if !bytes.Equal(req.VoteExtension, []byte(expectedVoteExt)) { - fmt.Println(expectedVoteExt) return &abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_REJECT}, nil } @@ -1589,6 +1596,119 @@ func TestABCI_PrepareProposal_PanicRecovery(t *testing.T) { }) } +func TestABCI_PrepareProposal_VoteExtensions(t *testing.T) { + // set up mocks + ctrl := gomock.NewController(t) + valStore := mock.NewMockValidatorStore(ctrl) + privkey := secp256k1.GenPrivKey() + pubkey := privkey.PubKey() + addr := sdk.AccAddress(pubkey.Address()) + tmPk := cmtprotocrypto.PublicKey{ + Sum: &cmtprotocrypto.PublicKey_Secp256K1{ + Secp256K1: pubkey.Bytes(), + }, + } + + val1 := mock.NewMockValidator(ctrl) + val1.EXPECT().BondedTokens().Return(math.NewInt(667)) + val1.EXPECT().CmtConsPublicKey().Return(tmPk, nil).AnyTimes() + + consAddr := sdk.ConsAddress(addr.String()) + valStore.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr.Bytes()).Return(val1, nil).AnyTimes() + valStore.EXPECT().TotalBondedTokens(gomock.Any()).Return(math.NewInt(1000)).AnyTimes() + + // set up baseapp + prepareOpt := func(bapp *baseapp.BaseApp) { + bapp.SetPrepareProposal(func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { + cp := ctx.ConsensusParams() + extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight + if extsEnabled { + err := baseapp.ValidateVoteExtensions(ctx, valStore, req.Height, bapp.ChainID(), req.LocalLastCommit.Votes, req.LocalLastCommit.Round) + if err != nil { + return nil, err + } + + req.Txs = append(req.Txs, []byte("some-tx-that-does-something-from-votes")) + + } + return &abci.ResponsePrepareProposal{Txs: req.Txs}, nil + }) + } + + suite := NewBaseAppSuite(t, prepareOpt) + + _, err := suite.baseApp.InitChain(&abci.RequestInitChain{ + InitialHeight: 1, + ConsensusParams: &cmtproto.ConsensusParams{ + Abci: &cmtproto.ABCIParams{ + VoteExtensionsEnableHeight: 2, + }, + }, + }) + require.NoError(t, err) + + // first test without vote extensions, no new txs should be added + reqPrepareProposal := abci.RequestPrepareProposal{ + MaxTxBytes: 1000, + Height: 1, // this value can't be 0 + } + resPrepareProposal, err := suite.baseApp.PrepareProposal(&reqPrepareProposal) + require.NoError(t, err) + require.Equal(t, 0, len(resPrepareProposal.Txs)) + + // now we try with vote extensions, a new tx should show up + marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { + var buf bytes.Buffer + if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil { + return nil, err + } + + return buf.Bytes(), nil + } + + ext := []byte("something") + cve := cmtproto.CanonicalVoteExtension{ + Extension: ext, + Height: 2, // the vote extension was signed in the previous height + Round: int64(0), + ChainId: suite.baseApp.ChainID(), + } + + bz, err := marshalDelimitedFn(&cve) + require.NoError(t, err) + + extSig, err := privkey.Sign(bz) + require.NoError(t, err) + + reqPrepareProposal = abci.RequestPrepareProposal{ + MaxTxBytes: 1000, + Height: 3, // this value can't be 0 + LocalLastCommit: abci.ExtendedCommitInfo{ + Round: 0, + Votes: []abci.ExtendedVoteInfo{ + { + Validator: abci.Validator{ + Address: consAddr.Bytes(), + // this is being ignored by our validation function + Power: sdk.TokensToConsensusPower(math.NewInt(1000000), sdk.DefaultPowerReduction), + }, + VoteExtension: ext, + ExtensionSignature: extSig, + }, + }, + }, + } + resPrepareProposal, err = suite.baseApp.PrepareProposal(&reqPrepareProposal) + require.NoError(t, err) + require.Equal(t, 1, len(resPrepareProposal.Txs)) + + // now vote extensions but our sole voter doesn't reach majority + val1.EXPECT().BondedTokens().Return(math.NewInt(666)) + resPrepareProposal, err = suite.baseApp.PrepareProposal(&reqPrepareProposal) + require.NoError(t, err) + require.Equal(t, 0, len(resPrepareProposal.Txs)) +} + func TestABCI_ProcessProposal_PanicRecovery(t *testing.T) { processOpt := func(app *baseapp.BaseApp) { app.SetProcessProposal(func(ctx sdk.Context, rpp *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) { diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 5a5298f0112f..d5e46c7ead45 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -50,7 +50,7 @@ type ( // ValidateVoteExtensions defines a helper function for verifying vote extension // signatures that may be passed or manually injected into a block proposal from -// a proposer in ProcessProposal. It returns an error if any signature is invalid +// a proposer in PrepareProposal. It returns an error if any signature is invalid // or if unexpected vote extensions and/or signatures are found or less than 2/3 // power is received. func ValidateVoteExtensions( @@ -58,10 +58,11 @@ func ValidateVoteExtensions( valStore ValidatorStore, currentHeight int64, chainID string, - extCommit abci.ExtendedCommitInfo, + votes []abci.ExtendedVoteInfo, + round int32, ) error { cp := ctx.ConsensusParams() - extsEnabled := cp.Abci != nil && cp.Abci.VoteExtensionsEnableHeight > 0 + extsEnabled := cp.Abci != nil && currentHeight >= cp.Abci.VoteExtensionsEnableHeight marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { var buf bytes.Buffer @@ -72,8 +73,8 @@ func ValidateVoteExtensions( return buf.Bytes(), nil } - var sumVP math.Int - for _, vote := range extCommit.Votes { + sumVP := math.NewInt(0) + for _, vote := range votes { if !extsEnabled { if len(vote.VoteExtension) > 0 { return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight) @@ -112,7 +113,7 @@ func ValidateVoteExtensions( cve := cmtproto.CanonicalVoteExtension{ Extension: vote.VoteExtension, Height: currentHeight - 1, // the vote extension was signed in the previous height - Round: int64(extCommit.Round), + Round: int64(round), ChainId: chainID, } diff --git a/scripts/mockgen.sh b/scripts/mockgen.sh index a3786acc50d2..a57691b951f7 100755 --- a/scripts/mockgen.sh +++ b/scripts/mockgen.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash mockgen_cmd="mockgen" +$mockgen_cmd -source=baseapp/abci_utils.go -package mock -destination baseapp/testutil/mock/validator_store.go $mockgen_cmd -source=client/account_retriever.go -package mock -destination testutil/mock/account_retriever.go $mockgen_cmd -package mock -destination store/mock/cosmos_cosmos_db_DB.go github.com/cosmos/cosmos-db DB $mockgen_cmd -source=types/module/module.go -package mock -destination testutil/mock/types_module_module.go From 3bd5cd5418cb0be487d1240dadcba90dc63e57f6 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Mon, 26 Jun 2023 20:46:08 +0200 Subject: [PATCH 09/16] add missing mock --- baseapp/testutil/mock/mocks.go | 210 +++++++++++++++++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 baseapp/testutil/mock/mocks.go diff --git a/baseapp/testutil/mock/mocks.go b/baseapp/testutil/mock/mocks.go new file mode 100644 index 000000000000..437ed68f485b --- /dev/null +++ b/baseapp/testutil/mock/mocks.go @@ -0,0 +1,210 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: baseapp/abci_utils.go + +// Package mock is a generated GoMock package. +package mock + +import ( + reflect "reflect" + + math "cosmossdk.io/math" + crypto "github.com/cometbft/cometbft/proto/tendermint/crypto" + baseapp "github.com/cosmos/cosmos-sdk/baseapp" + types "github.com/cosmos/cosmos-sdk/crypto/types" + types0 "github.com/cosmos/cosmos-sdk/types" + gomock "github.com/golang/mock/gomock" +) + +// MockValidator is a mock of Validator interface. +type MockValidator struct { + ctrl *gomock.Controller + recorder *MockValidatorMockRecorder +} + +// MockValidatorMockRecorder is the mock recorder for MockValidator. +type MockValidatorMockRecorder struct { + mock *MockValidator +} + +// NewMockValidator creates a new mock instance. +func NewMockValidator(ctrl *gomock.Controller) *MockValidator { + mock := &MockValidator{ctrl: ctrl} + mock.recorder = &MockValidatorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockValidator) EXPECT() *MockValidatorMockRecorder { + return m.recorder +} + +// BondedTokens mocks base method. +func (m *MockValidator) BondedTokens() math.Int { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BondedTokens") + ret0, _ := ret[0].(math.Int) + return ret0 +} + +// BondedTokens indicates an expected call of BondedTokens. +func (mr *MockValidatorMockRecorder) BondedTokens() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BondedTokens", reflect.TypeOf((*MockValidator)(nil).BondedTokens)) +} + +// CmtConsPublicKey mocks base method. +func (m *MockValidator) CmtConsPublicKey() (crypto.PublicKey, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CmtConsPublicKey") + ret0, _ := ret[0].(crypto.PublicKey) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CmtConsPublicKey indicates an expected call of CmtConsPublicKey. +func (mr *MockValidatorMockRecorder) CmtConsPublicKey() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CmtConsPublicKey", reflect.TypeOf((*MockValidator)(nil).CmtConsPublicKey)) +} + +// MockValidatorStore is a mock of ValidatorStore interface. +type MockValidatorStore struct { + ctrl *gomock.Controller + recorder *MockValidatorStoreMockRecorder +} + +// MockValidatorStoreMockRecorder is the mock recorder for MockValidatorStore. +type MockValidatorStoreMockRecorder struct { + mock *MockValidatorStore +} + +// NewMockValidatorStore creates a new mock instance. +func NewMockValidatorStore(ctrl *gomock.Controller) *MockValidatorStore { + mock := &MockValidatorStore{ctrl: ctrl} + mock.recorder = &MockValidatorStoreMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockValidatorStore) EXPECT() *MockValidatorStoreMockRecorder { + return m.recorder +} + +// GetValidatorByConsAddr mocks base method. +func (m *MockValidatorStore) GetValidatorByConsAddr(arg0 types0.Context, arg1 types.Address) (baseapp.Validator, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetValidatorByConsAddr", arg0, arg1) + ret0, _ := ret[0].(baseapp.Validator) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetValidatorByConsAddr indicates an expected call of GetValidatorByConsAddr. +func (mr *MockValidatorStoreMockRecorder) GetValidatorByConsAddr(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetValidatorByConsAddr", reflect.TypeOf((*MockValidatorStore)(nil).GetValidatorByConsAddr), arg0, arg1) +} + +// TotalBondedTokens mocks base method. +func (m *MockValidatorStore) TotalBondedTokens(ctx types0.Context) math.Int { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TotalBondedTokens", ctx) + ret0, _ := ret[0].(math.Int) + return ret0 +} + +// TotalBondedTokens indicates an expected call of TotalBondedTokens. +func (mr *MockValidatorStoreMockRecorder) TotalBondedTokens(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TotalBondedTokens", reflect.TypeOf((*MockValidatorStore)(nil).TotalBondedTokens), ctx) +} + +// MockGasTx is a mock of GasTx interface. +type MockGasTx struct { + ctrl *gomock.Controller + recorder *MockGasTxMockRecorder +} + +// MockGasTxMockRecorder is the mock recorder for MockGasTx. +type MockGasTxMockRecorder struct { + mock *MockGasTx +} + +// NewMockGasTx creates a new mock instance. +func NewMockGasTx(ctrl *gomock.Controller) *MockGasTx { + mock := &MockGasTx{ctrl: ctrl} + mock.recorder = &MockGasTxMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockGasTx) EXPECT() *MockGasTxMockRecorder { + return m.recorder +} + +// GetGas mocks base method. +func (m *MockGasTx) GetGas() uint64 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGas") + ret0, _ := ret[0].(uint64) + return ret0 +} + +// GetGas indicates an expected call of GetGas. +func (mr *MockGasTxMockRecorder) GetGas() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGas", reflect.TypeOf((*MockGasTx)(nil).GetGas)) +} + +// MockProposalTxVerifier is a mock of ProposalTxVerifier interface. +type MockProposalTxVerifier struct { + ctrl *gomock.Controller + recorder *MockProposalTxVerifierMockRecorder +} + +// MockProposalTxVerifierMockRecorder is the mock recorder for MockProposalTxVerifier. +type MockProposalTxVerifierMockRecorder struct { + mock *MockProposalTxVerifier +} + +// NewMockProposalTxVerifier creates a new mock instance. +func NewMockProposalTxVerifier(ctrl *gomock.Controller) *MockProposalTxVerifier { + mock := &MockProposalTxVerifier{ctrl: ctrl} + mock.recorder = &MockProposalTxVerifierMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockProposalTxVerifier) EXPECT() *MockProposalTxVerifierMockRecorder { + return m.recorder +} + +// PrepareProposalVerifyTx mocks base method. +func (m *MockProposalTxVerifier) PrepareProposalVerifyTx(tx types0.Tx) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PrepareProposalVerifyTx", tx) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// PrepareProposalVerifyTx indicates an expected call of PrepareProposalVerifyTx. +func (mr *MockProposalTxVerifierMockRecorder) PrepareProposalVerifyTx(tx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrepareProposalVerifyTx", reflect.TypeOf((*MockProposalTxVerifier)(nil).PrepareProposalVerifyTx), tx) +} + +// ProcessProposalVerifyTx mocks base method. +func (m *MockProposalTxVerifier) ProcessProposalVerifyTx(txBz []byte) (types0.Tx, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ProcessProposalVerifyTx", txBz) + ret0, _ := ret[0].(types0.Tx) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ProcessProposalVerifyTx indicates an expected call of ProcessProposalVerifyTx. +func (mr *MockProposalTxVerifierMockRecorder) ProcessProposalVerifyTx(txBz interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProcessProposalVerifyTx", reflect.TypeOf((*MockProposalTxVerifier)(nil).ProcessProposalVerifyTx), txBz) +} From 69fe53411c28d7675cf5a060da293f856d650b27 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 27 Jun 2023 11:02:46 +0200 Subject: [PATCH 10/16] lint --- baseapp/abci_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index df4908a91561..ed5904524f54 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -11,6 +11,7 @@ import ( "testing" abci "github.com/cometbft/cometbft/abci/types" + "github.com/cometbft/cometbft/crypto/secp256k1" cmtprotocrypto "github.com/cometbft/cometbft/proto/tendermint/crypto" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" dbm "github.com/cosmos/cosmos-db" @@ -28,8 +29,6 @@ import ( snapshottypes "cosmossdk.io/store/snapshots/types" storetypes "cosmossdk.io/store/types" - "github.com/cometbft/cometbft/crypto/secp256k1" - "github.com/cosmos/cosmos-sdk/baseapp" baseapptestutil "github.com/cosmos/cosmos-sdk/baseapp/testutil" "github.com/cosmos/cosmos-sdk/baseapp/testutil/mock" From dff15560bd026613f6f3917cb772add64b93046a Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 27 Jun 2023 11:14:41 +0200 Subject: [PATCH 11/16] fix check --- baseapp/abci.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index c68ea55cc3b4..379e0cf504de 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -558,7 +558,9 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( // If vote extensions are not enabled, as a safety precaution, we return an // error. cp := app.GetConsensusParams(app.voteExtensionState.ctx) - if cp.Abci != nil && req.Height < cp.Abci.VoteExtensionsEnableHeight { + + extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight + if !extsEnabled { return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) } @@ -610,7 +612,9 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r // If vote extensions are not enabled, as a safety precaution, we return an // error. cp := app.GetConsensusParams(app.voteExtensionState.ctx) - if cp.Abci != nil && req.Height < cp.Abci.VoteExtensionsEnableHeight { + + extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight + if !extsEnabled { return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to VerifyVoteExtension at height %d", req.Height) } From 3751957342ce120dc397c562a9a3b9c89758de7b Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 27 Jun 2023 11:25:59 +0200 Subject: [PATCH 12/16] fix check again --- baseapp/abci.go | 4 ++-- baseapp/abci_test.go | 2 +- baseapp/abci_utils.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 379e0cf504de..f0e015678353 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -559,7 +559,7 @@ func (app *BaseApp) ExtendVote(_ context.Context, req *abci.RequestExtendVote) ( // error. cp := app.GetConsensusParams(app.voteExtensionState.ctx) - extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight + extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0 if !extsEnabled { return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to ExtendVote at height %d", req.Height) } @@ -613,7 +613,7 @@ func (app *BaseApp) VerifyVoteExtension(req *abci.RequestVerifyVoteExtension) (r // error. cp := app.GetConsensusParams(app.voteExtensionState.ctx) - extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight + extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0 if !extsEnabled { return nil, fmt.Errorf("vote extensions are not enabled; unexpected call to VerifyVoteExtension at height %d", req.Height) } diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index ed5904524f54..736e0ec3808e 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -1620,7 +1620,7 @@ func TestABCI_PrepareProposal_VoteExtensions(t *testing.T) { prepareOpt := func(bapp *baseapp.BaseApp) { bapp.SetPrepareProposal(func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { cp := ctx.ConsensusParams() - extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight + extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0 if extsEnabled { err := baseapp.ValidateVoteExtensions(ctx, valStore, req.Height, bapp.ChainID(), req.LocalLastCommit.Votes, req.LocalLastCommit.Round) if err != nil { diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index d5e46c7ead45..4bb7c2a33899 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -62,7 +62,7 @@ func ValidateVoteExtensions( round int32, ) error { cp := ctx.ConsensusParams() - extsEnabled := cp.Abci != nil && currentHeight >= cp.Abci.VoteExtensionsEnableHeight + extsEnabled := cp.Abci != nil && currentHeight >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0 marshalDelimitedFn := func(msg proto.Message) ([]byte, error) { var buf bytes.Buffer From 1abdd3ca5c2fe5fec2e21b31b290adb0001b9877 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 27 Jun 2023 11:35:30 +0200 Subject: [PATCH 13/16] rollback change --- baseapp/abci_test.go | 2 +- baseapp/abci_utils.go | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 736e0ec3808e..f4be0c1497a5 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -1622,7 +1622,7 @@ func TestABCI_PrepareProposal_VoteExtensions(t *testing.T) { cp := ctx.ConsensusParams() extsEnabled := cp.Abci != nil && req.Height >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0 if extsEnabled { - err := baseapp.ValidateVoteExtensions(ctx, valStore, req.Height, bapp.ChainID(), req.LocalLastCommit.Votes, req.LocalLastCommit.Round) + err := baseapp.ValidateVoteExtensions(ctx, valStore, req.Height, bapp.ChainID(), req.LocalLastCommit) if err != nil { return nil, err } diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 4bb7c2a33899..d73fb003b46e 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -58,8 +58,7 @@ func ValidateVoteExtensions( valStore ValidatorStore, currentHeight int64, chainID string, - votes []abci.ExtendedVoteInfo, - round int32, + extCommit abci.ExtendedCommitInfo, ) error { cp := ctx.ConsensusParams() extsEnabled := cp.Abci != nil && currentHeight >= cp.Abci.VoteExtensionsEnableHeight && cp.Abci.VoteExtensionsEnableHeight != 0 @@ -74,7 +73,7 @@ func ValidateVoteExtensions( } sumVP := math.NewInt(0) - for _, vote := range votes { + for _, vote := range extCommit.Votes { if !extsEnabled { if len(vote.VoteExtension) > 0 { return fmt.Errorf("vote extensions disabled; received non-empty vote extension at height %d", currentHeight) @@ -113,7 +112,7 @@ func ValidateVoteExtensions( cve := cmtproto.CanonicalVoteExtension{ Extension: vote.VoteExtension, Height: currentHeight - 1, // the vote extension was signed in the previous height - Round: int64(round), + Round: int64(extCommit.Round), ChainId: chainID, } From 47af806078d4f0154967abd7ace69eecf11e99ec Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 27 Jun 2023 12:10:31 +0200 Subject: [PATCH 14/16] update upgrading file --- UPGRADING.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/UPGRADING.md b/UPGRADING.md index 0ba194b304c1..0700aef048ee 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -29,6 +29,18 @@ Additionally, the SDK is starting its abstraction from CometBFT Go types thoroug * The usage of CometBFT have been replaced to use the Cosmos SDK logger interface (`cosmossdk.io/log.Logger`). * The usage of `github.com/cometbft/cometbft/libs/bytes.HexByte` have been replaced by `[]byte`. +### Enable Vote Extensions + +To enable vote extensions, the consensus param `Abci.VoteExtensionsEnableHeight` must be set to a value greater +than zero. + +For new chains this can be done in the `genesis.json` file. + +For existing chains this can be done in two ways: + +- An upgrade handler that sets this value. +- A governance proposal that changes the consensus param. + ### BaseApp All ABCI methods now accept a pointer to the request and response types defined From 7b5cbb71d3b4dc3c8b5aaaf89201ea9085fd14a7 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 27 Jun 2023 14:24:06 +0200 Subject: [PATCH 15/16] improve upgrading doc --- UPGRADING.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 0700aef048ee..314bedd8bf2f 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -31,15 +31,18 @@ Additionally, the SDK is starting its abstraction from CometBFT Go types thoroug ### Enable Vote Extensions -To enable vote extensions, the consensus param `Abci.VoteExtensionsEnableHeight` must be set to a value greater -than zero. +> This is an optional feature that is disabled by default. -For new chains this can be done in the `genesis.json` file. +Once all the code changes required to implement Vote Extensions are in place, +they can be enabled by setting the consensus param `Abci.VoteExtensionsEnableHeight` +to a value greater than zero. + +In a new chain, this can be done in the `genesis.json` file. For existing chains this can be done in two ways: -- An upgrade handler that sets this value. -- A governance proposal that changes the consensus param. +- During an upgrade the value is set in an upgrade handler. +- A governance proposal that changes the consensus param **after a coordinated upgrade has taken place**. ### BaseApp From a905df9d0c29fc9ee136699c980b67df1179845e Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Fri, 30 Jun 2023 09:14:00 +0200 Subject: [PATCH 16/16] julien suggestions --- UPGRADING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 314bedd8bf2f..c6fb3004feb0 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -29,9 +29,9 @@ Additionally, the SDK is starting its abstraction from CometBFT Go types thoroug * The usage of CometBFT have been replaced to use the Cosmos SDK logger interface (`cosmossdk.io/log.Logger`). * The usage of `github.com/cometbft/cometbft/libs/bytes.HexByte` have been replaced by `[]byte`. -### Enable Vote Extensions +#### Enable Vote Extensions -> This is an optional feature that is disabled by default. +:::tip This is an optional feature that is disabled by default. Once all the code changes required to implement Vote Extensions are in place, they can be enabled by setting the consensus param `Abci.VoteExtensionsEnableHeight`