Skip to content

Commit

Permalink
fix: remove setting of finalizeBlockState in FinalizeBlock + fix init…
Browse files Browse the repository at this point in the history 
…ialHeight + add ProcessProposal in tests/sims (#16794)

Co-authored-by: Aleksandr Bezobchuk <[email protected]>
(cherry picked from commit 0fd6227)
  • Loading branch information
@facundomedica @mergify
facundomedica authored and mergify[bot] committed Jul 6, 2023
1 parent c919775 commit 2c564bc
Show file tree
Hide file tree
Showing 19 changed files with 206 additions and 66 deletions.
54 changes: 21 additions & 33 deletions baseapp/abci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,15 @@ func (app *BaseApp) InitChain(req *abci.RequestInitChain) (*abci.ResponseInitCha
// On a new chain, we consider the init chain block height as 0, even though
// req.InitialHeight is 1 by default.
initHeader := cmtproto.Header{ChainID: req.ChainId, Time: req.Time}
app.initialHeight = req.InitialHeight

app.logger.Info("InitChain", "initialHeight", req.InitialHeight, "chainID", req.ChainId)

// Set the initial height, which will be used to determine if we are proposing
// or processing the first block or not.
app.initialHeight = req.InitialHeight
if app.initialHeight == 0 { // If initial height is 0, set it to 1
app.initialHeight = 1
}

// if req.InitialHeight is > 1, then we set the initial version on all stores
if req.InitialHeight > 1 {
Expand Down Expand Up @@ -675,46 +677,32 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons
AppHash: app.LastCommitID().Hash,
}

// Initialize the FinalizeBlock state. If this is the first block, it should
// already be initialized in InitChain. Otherwise app.finalizeBlockState will be
// nil, since it is reset on Commit.
if app.finalizeBlockState == nil {
app.setState(execModeFinalize, header)
} else {
// In the first block, app.finalizeBlockState.ctx will already be initialized
// by InitChain. Context is now updated with Header information.
app.finalizeBlockState.ctx = app.finalizeBlockState.ctx.
WithBlockHeader(header).
WithBlockHeight(req.Height).
WithHeaderInfo(coreheader.Info{
ChainID: app.chainID,
Height: req.Height,
Time: req.Time,
Hash: req.Hash,
AppHash: app.LastCommitID().Hash,
})
}

gasMeter := app.getBlockGasMeter(app.finalizeBlockState.ctx)

// app.finalizeBlockState.ctx will already be initialized
// by InitChain or by ProcessProposal. Context is now updated with Header information.
app.finalizeBlockState.ctx = app.finalizeBlockState.ctx.
WithBlockGasMeter(gasMeter).
WithBlockHeader(header).
WithHeaderHash(req.Hash).
WithConsensusParams(app.GetConsensusParams(app.finalizeBlockState.ctx)).
WithVoteInfos(req.DecidedLastCommit.Votes).
WithExecMode(sdk.ExecModeFinalize).
WithHeaderInfo(coreheader.Info{
ChainID: app.chainID,
Height: req.Height,
Time: req.Time,
Hash: req.Hash,
AppHash: app.LastCommitID().Hash,
}).WithCometInfo(cometInfo{
Misbehavior: req.Misbehavior,
ValidatorsHash: req.NextValidatorsHash,
ProposerAddress: req.ProposerAddress,
LastCommit: req.DecidedLastCommit,
})
}).
WithConsensusParams(app.GetConsensusParams(app.finalizeBlockState.ctx)).
WithVoteInfos(req.DecidedLastCommit.Votes).
WithExecMode(sdk.ExecModeFinalize).
WithCometInfo(cometInfo{
Misbehavior: req.Misbehavior,
ValidatorsHash: req.NextValidatorsHash,
ProposerAddress: req.ProposerAddress,
LastCommit: req.DecidedLastCommit,
})

gasMeter := app.getBlockGasMeter(app.finalizeBlockState.ctx)

app.finalizeBlockState.ctx = app.finalizeBlockState.ctx.
WithBlockGasMeter(gasMeter)

if app.checkState != nil {
app.checkState.ctx = app.checkState.ctx.
Expand Down
28 changes: 26 additions & 2 deletions baseapp/abci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ func TestABCI_InitChain(t *testing.T) {
require.Equal(t, value, resQ.Value)

// commit and ensure we can still query
_, err = app.ProcessProposal(&abci.RequestProcessProposal{Height: app.LastBlockHeight() + 1})
require.NoError(t, err)
_, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: app.LastBlockHeight() + 1})
require.NoError(t, err)
_, err = app.Commit()
Expand Down Expand Up @@ -578,6 +580,12 @@ func TestABCI_FinalizeBlock_DeliverTx(t *testing.T) {
txs = append(txs, txBytes)
}

_, err := suite.baseApp.ProcessProposal(&abci.RequestProcessProposal{
Height: int64(blockN) + 1,
Txs: txs,
})
require.NoError(t, err)

res, err := suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{
Height: int64(blockN) + 1,
Txs: txs,
Expand Down Expand Up @@ -729,6 +737,8 @@ 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))

_, err = suite.baseApp.ProcessProposal(&abci.RequestProcessProposal{Height: count})
require.NoError(t, err)
_, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: count})
require.NoError(t, err)
_, err = suite.baseApp.Commit()
Expand Down Expand Up @@ -897,6 +907,10 @@ func TestABCI_TxGasLimits(t *testing.T) {
})
require.NoError(t, err)

_, err = suite.baseApp.ProcessProposal(&abci.RequestProcessProposal{
Height: 1,
})
require.NoError(t, err)
_, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{
Height: 1,
})
Expand Down Expand Up @@ -934,6 +948,12 @@ func TestABCI_TxGasLimits(t *testing.T) {
}

// Deliver the txs
_, err = suite.baseApp.ProcessProposal(&abci.RequestProcessProposal{
Height: 2,
Txs: txs,
})
require.NoError(t, err)

res, err := suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{
Height: 2,
Txs: txs,
Expand Down Expand Up @@ -1299,7 +1319,9 @@ func TestPrepareCheckStateCalledWithCheckState(t *testing.T) {
wasPrepareCheckStateCalled = true
})

_, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
_, err := app.ProcessProposal(&abci.RequestProcessProposal{Height: 1})
require.NoError(t, err)
_, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
require.NoError(t, err)
_, err = app.Commit()
require.NoError(t, err)
Expand All @@ -1323,7 +1345,9 @@ func TestPrecommiterCalledWithDeliverState(t *testing.T) {
wasPrecommiterCalled = true
})

_, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
_, err := app.ProcessProposal(&abci.RequestProcessProposal{Height: 1})
require.NoError(t, err)
_, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
require.NoError(t, err)
_, err = app.Commit()
require.NoError(t, err)
Expand Down
26 changes: 24 additions & 2 deletions baseapp/baseapp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,13 @@ func NewBaseAppSuiteWithSnapshots(t *testing.T, cfg SnapshotsConfig, opts ...fun
txs = append(txs, txBytes)
}

_, err := suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{
_, err := suite.baseApp.ProcessProposal(&abci.RequestProcessProposal{
Height: height,
Txs: txs,
})
require.NoError(t, err)

_, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{
Height: height,
Txs: txs,
})
Expand Down Expand Up @@ -188,13 +194,17 @@ func TestLoadVersion(t *testing.T) {
require.Equal(t, emptyCommitID, lastID)

// execute a block, collect commit ID
_, err = app.ProcessProposal(&abci.RequestProcessProposal{Height: 1})
require.NoError(t, err)
res, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
require.NoError(t, err)
commitID1 := storetypes.CommitID{Version: 1, Hash: res.AppHash}
_, err = app.Commit()
require.NoError(t, err)

// execute a block, collect commit ID
_, err = app.ProcessProposal(&abci.RequestProcessProposal{Height: 2})
require.NoError(t, err)
res, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 2})
require.NoError(t, err)
commitID2 := storetypes.CommitID{Version: 2, Hash: res.AppHash}
Expand All @@ -218,6 +228,8 @@ func TestLoadVersion(t *testing.T) {

testLoadVersionHelper(t, app, int64(1), commitID1)

_, err = app.ProcessProposal(&abci.RequestProcessProposal{Height: 2})
require.NoError(t, err)
_, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 2})
require.NoError(t, err)
_, err = app.Commit()
Expand Down Expand Up @@ -304,6 +316,8 @@ func TestSetLoader(t *testing.T) {
require.Nil(t, err)

// "execute" one block
_, err = app.ProcessProposal(&abci.RequestProcessProposal{Height: 2})
require.NoError(t, err)
res, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 2})
require.NoError(t, err)
require.NotNil(t, res.AppHash)
Expand Down Expand Up @@ -353,6 +367,8 @@ func TestLoadVersionInvalid(t *testing.T) {
err = app.LoadVersion(-1)
require.Error(t, err)

_, err = app.ProcessProposal(&abci.RequestProcessProposal{Height: 1})
require.NoError(t, err)
res, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
require.NoError(t, err)
commitID1 := storetypes.CommitID{Version: 1, Hash: res.AppHash}
Expand Down Expand Up @@ -572,11 +588,15 @@ func TestABCI_CreateQueryContext(t *testing.T) {
name := t.Name()
app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil)

_, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
_, err := app.ProcessProposal(&abci.RequestProcessProposal{Height: 1})
require.NoError(t, err)
_, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
require.NoError(t, err)
_, err = app.Commit()
require.NoError(t, err)

_, err = app.ProcessProposal(&abci.RequestProcessProposal{Height: 2})
require.NoError(t, err)
_, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 2})
require.NoError(t, err)
_, err = app.Commit()
Expand Down Expand Up @@ -662,6 +682,8 @@ func TestLoadVersionPruning(t *testing.T) {
// Commit seven blocks, of which 7 (latest) is kept in addition to 6, 5
// (keep recent) and 3 (keep every).
for i := int64(1); i <= 7; i++ {
_, err := app.ProcessProposal(&abci.RequestProcessProposal{Height: i})
require.NoError(t, err)
res, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: i})
require.NoError(t, err)
_, err = app.Commit()
Expand Down
2 changes: 2 additions & 0 deletions baseapp/msg_service_router_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ func TestMsgService(t *testing.T) {
app.MsgServiceRouter(),
testdata.MsgServerImpl{},
)
_, err = app.ProcessProposal(&abci.RequestProcessProposal{Height: 1})
require.NoError(t, err)
_, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1})
require.NoError(t, err)

Expand Down
3 changes: 2 additions & 1 deletion baseapp/streaming_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestABCI_MultiListener_StateChanges(t *testing.T) {
var expectedChangeSet []*storetypes.StoreKVPair

// create final block context state
_, err := suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: int64(blockN) + 1, Txs: txs})
_, err := suite.baseApp.ProcessProposal(&abci.RequestProcessProposal{Height: int64(blockN) + 1, Txs: txs})
require.NoError(t, err)

for i := 0; i < txPerHeight; i++ {
Expand Down Expand Up @@ -133,6 +133,7 @@ func Test_Ctx_with_StreamingManager(t *testing.T) {

for blockN := 0; blockN < nBlocks; blockN++ {

suite.baseApp.ProcessProposal(&abci.RequestProcessProposal{Height: int64(blockN) + 1})
suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: int64(blockN) + 1})

ctx := getFinalizeBlockStateCtx(suite.baseApp)
Expand Down
9 changes: 8 additions & 1 deletion server/mock/app_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestInitApp(t *testing.T) {
require.Equal(t, []byte("bar"), qres.Value)
}

func TestDeliverTx(t *testing.T) {
func TestFinalizeBlock(t *testing.T) {
app := SetupApp(t)

key := "my-special-key"
Expand All @@ -72,6 +72,13 @@ func TestDeliverTx(t *testing.T) {
tx := NewTx(key, value, randomAccounts[0].Address)
txBytes := tx.GetSignBytes()

_, err := app.ProcessProposal(&abci.RequestProcessProposal{
Hash: []byte("apphash"),
Height: 1,
Txs: [][]byte{txBytes},
})
require.NoError(t, err)

res, err := app.FinalizeBlock(&abci.RequestFinalizeBlock{
Hash: []byte("apphash"),
Height: 1,
Expand Down
1 change: 0 additions & 1 deletion simapp/test_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ func NewSimappWithCustomOptions(t *testing.T, isCheckTx bool, options SetupOptio
Address: acc.GetAddress().String(),
Coins: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100000000000000))),
}

app := NewSimApp(options.Logger, options.DB, nil, true, options.AppOpts)
genesisState := app.DefaultGenesis()
genesisState, err = simtestutil.GenesisStateWithValSet(app.AppCodec(), genesisState, valSet, []authtypes.GenesisAccount{acc}, balance)
Expand Down
4 changes: 4 additions & 0 deletions tests/e2e/server/export_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ func TestExportCmd_Height(t *testing.T) {

// Fast forward to block `tc.fastForward`.
for i := int64(2); i <= tc.fastForward; i++ {
app.ProcessProposal(&abci.RequestProcessProposal{
Height: i,
})

app.FinalizeBlock(&abci.RequestFinalizeBlock{
Height: i,
})
Expand Down
13 changes: 7 additions & 6 deletions tests/integration/distribution/keeper/msg_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
res, err := f.app.RunMsg(
tc.msg,
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticCommit(),
)
Expand Down Expand Up @@ -432,7 +433,7 @@ func TestMsgSetWithdrawAddress(t *testing.T) {
tc.preRun()
res, err := f.app.RunMsg(
tc.msg,
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticCommit(),
)
if tc.expErr {
Expand Down Expand Up @@ -528,7 +529,7 @@ func TestMsgWithdrawValidatorCommission(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
res, err := f.app.RunMsg(
tc.msg,
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticCommit(),
)
if tc.expErr {
Expand Down Expand Up @@ -630,7 +631,7 @@ func TestMsgFundCommunityPool(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
res, err := f.app.RunMsg(
tc.msg,
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticCommit(),
)
if tc.expErr {
Expand Down Expand Up @@ -758,7 +759,7 @@ func TestMsgUpdateParams(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
res, err := f.app.RunMsg(
tc.msg,
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticCommit(),
)
if tc.expErr {
Expand Down Expand Up @@ -837,7 +838,7 @@ func TestMsgCommunityPoolSpend(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
res, err := f.app.RunMsg(
tc.msg,
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticCommit(),
)
if tc.expErr {
Expand Down Expand Up @@ -939,7 +940,7 @@ func TestMsgDepositValidatorRewardsPool(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
res, err := f.app.RunMsg(
tc.msg,
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticCommit(),
)
if tc.expErr {
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/slashing/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func TestUnJailNotBonded(t *testing.T) {
}
_, err = f.app.RunMsg(
&msgUnjail,
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticCommit(),
)
assert.ErrorContains(t, err, "cannot be unjailed")
Expand All @@ -205,7 +205,7 @@ func TestUnJailNotBonded(t *testing.T) {
// verify we can immediately unjail
_, err = f.app.RunMsg(
&msgUnjail,
integration.WithAutomaticFinalizeBlock(),
integration.WithAutomaticProcessProposal(),
integration.WithAutomaticCommit(),
)
assert.NilError(t, err)
Expand Down
Loading

0 comments on commit 2c564bc

Please sign in to comment.