From 024850c8812e481e441d5d42673b9a33d89ec9ae Mon Sep 17 00:00:00 2001 From: Marko Date: Tue, 9 May 2023 10:53:19 +0200 Subject: [PATCH] refactor(core): use working hash in finalizeblock instead of flush (#16065) --- baseapp/abci.go | 33 ++++++++++++++++++++++++++------- client/grpc_query_test.go | 2 +- testutil/sims/app_helpers.go | 2 +- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index c901a859bf62..abe3271e79b7 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -702,8 +702,7 @@ func (app *BaseApp) FinalizeBlock(_ context.Context, req *abci.RequestFinalizeBl TxResults: txResults, ValidatorUpdates: endBlock.ValidatorUpdates, ConsensusParamUpdates: &cp, - // TODO: Do not commit, but use current IAVL root hash! - AppHash: app.flushCommit().Hash, + AppHash: app.workingHash(), }, nil } @@ -727,6 +726,8 @@ func (app *BaseApp) Commit(_ context.Context, _ *abci.RequestCommit) (*abci.Resp rms.SetCommitHeader(header) } + app.commit() + resp := &abci.ResponseCommit{ RetainHeight: retainHeight, } @@ -778,20 +779,38 @@ func (app *BaseApp) Commit(_ context.Context, _ *abci.RequestCommit) (*abci.Resp return resp, nil } -// flushCommit writes all state transitions that occurred during FinalizeBlock. -// These writes are persisted to the root multi-store (app.cms) and flushed to +// commit commits all state transitions that occurred during FinalizeBlock. +// These writes have been persisted to the root multi-store (app.cms) via workingHash and flushed to // disk. This means when the ABCI client requests Commit(), the application // state transitions are already flushed to disk and as a result, we already have // an application Merkle root. -func (app *BaseApp) flushCommit() storetypes.CommitID { - app.finalizeBlockState.ms.Write() - +// CONTRACT: must be called after workingHash is called. +func (app *BaseApp) commit() storetypes.CommitID { + // call commit to persist data to disk commitID := app.cms.Commit() app.logger.Info("commit synced", "commit", fmt.Sprintf("%X", commitID)) return commitID } +// workingHash gets the apphash that will be finalized in commit. +// These writes will be persisted to the root multi-store (app.cms) and flushed to +// disk in the Commit phase. This means when the ABCI client requests Commit(), the application +// state transitions will be flushed to disk and as a result, but we already have +// an application Merkle root. +func (app *BaseApp) workingHash() []byte { + // Write the FinalizeBlock state into branched storage and commit the MultiStore. + // The write to the FinalizeBlock state writes all state transitions to the root + // MultiStore (app.cms) so when Commit() is called it persists those values. + app.finalizeBlockState.ms.Write() + + // Get the hash of all writes in order to return the apphash to the comet in finalizeBlock. + commitHash := app.cms.WorkingHash() + app.logger.Debug("hash of all writes", "workingHash", fmt.Sprintf("%X", commitHash)) + + return commitHash +} + // halt attempts to gracefully shutdown the node via SIGINT and SIGTERM falling // back on os.Exit if both fail. func (app *BaseApp) halt() { diff --git a/client/grpc_query_test.go b/client/grpc_query_test.go index a4be61b9dc63..ef3afbd54386 100644 --- a/client/grpc_query_test.go +++ b/client/grpc_query_test.go @@ -91,7 +91,7 @@ func (s *IntegrationTestSuite) SetupSuite() { }, ) - // app.Commit(context.TODO(), &abci.RequestCommit{}) // TODO see if this is needed + app.Commit(context.TODO(), &abci.RequestCommit{}) app.FinalizeBlock(context.TODO(), &abci.RequestFinalizeBlock{ Height: app.LastBlockHeight() + 1, Hash: app.LastCommitID().Hash, diff --git a/testutil/sims/app_helpers.go b/testutil/sims/app_helpers.go index 160265d4a1cc..b97c142c5877 100644 --- a/testutil/sims/app_helpers.go +++ b/testutil/sims/app_helpers.go @@ -166,7 +166,7 @@ func SetupWithConfiguration(appConfig depinject.Config, startupConfig StartupCon // commit genesis changes if !startupConfig.AtGenesis { - // app.Commit(context.TODO(), &abci.RequestCommit{}) // TODO figure out if this is needed? + app.Commit(context.TODO(), &abci.RequestCommit{}) _, err := app.FinalizeBlock(context.TODO(), &abci.RequestFinalizeBlock{ Height: app.LastBlockHeight() + 1, NextValidatorsHash: valSet.Hash(),