From 8b262bdd1d3686df05a88eddc64f7fa81e8ed2e6 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 5 Jul 2024 14:33:50 -0400 Subject: [PATCH] fix: invalid `txs_results` returned for legacy ABCI responses (backport #3031) (#3434) close: #3002 This PR fixes the issue reported above. This is not a storage issue in particular, the results are still in storage after an upgrade, but not returned properly by the RPC endpoint. The fix is to make the `/block_results` endpoint in `v0.38` to return properly a legacy ABCI response created with `v0.37`. Once this fix is merged on `v0.38` and a patch release is cut, any node on `v0.38` (e.g. an archive node) that applies the patch release, should have the results returned properly by the RPC `/block_results` endpoint. --- #### PR checklist - [X] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] ~~Updated relevant documentation (`docs/` or `spec/`) and code comments~~ - [X] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
This is an automatic backport of pull request #3031 done by [Mergify](https://mergify.com). --------- Co-authored-by: Andy Nogueira --- .../bug-fixes/3002-invalid-txs-results.md | 3 + rpc/core/blocks.go | 1 + rpc/core/blocks_test.go | 2 + state/compatibility_test.go | 652 ++++++++++++++++++ state/errors.go | 33 +- state/state_test.go | 19 +- state/store.go | 75 +- state/store_test.go | 6 +- 8 files changed, 765 insertions(+), 26 deletions(-) create mode 100644 .changelog/unreleased/bug-fixes/3002-invalid-txs-results.md create mode 100644 state/compatibility_test.go diff --git a/.changelog/unreleased/bug-fixes/3002-invalid-txs-results.md b/.changelog/unreleased/bug-fixes/3002-invalid-txs-results.md new file mode 100644 index 0000000000..ed0980f466 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/3002-invalid-txs-results.md @@ -0,0 +1,3 @@ +- `[rpc]` Fix an issue where a legacy ABCI response, created on `v0.37` or before, is not returned properly in `v0.38` and up +on the `/block_results` RPC endpoint. + ([\#3002](https://github.com/cometbft/cometbft/issues/3002)) diff --git a/rpc/core/blocks.go b/rpc/core/blocks.go index ea013a7c3e..792d82ae34 100644 --- a/rpc/core/blocks.go +++ b/rpc/core/blocks.go @@ -195,6 +195,7 @@ func (env *Environment) BlockResults(_ *rpctypes.Context, heightPtr *int64) (*ct FinalizeBlockEvents: results.Events, ValidatorUpdates: results.ValidatorUpdates, ConsensusParamUpdates: results.ConsensusParamUpdates, + AppHash: results.AppHash, }, nil } diff --git a/rpc/core/blocks_test.go b/rpc/core/blocks_test.go index 4114eaa118..ffc480c888 100644 --- a/rpc/core/blocks_test.go +++ b/rpc/core/blocks_test.go @@ -73,6 +73,7 @@ func TestBlockResults(t *testing.T) { {Code: 0, Data: []byte{0x02}, Log: "ok"}, {Code: 1, Log: "not ok"}, }, + AppHash: make([]byte, 1), } env := &Environment{} @@ -100,6 +101,7 @@ func TestBlockResults(t *testing.T) { FinalizeBlockEvents: results.Events, ValidatorUpdates: results.ValidatorUpdates, ConsensusParamUpdates: results.ConsensusParamUpdates, + AppHash: make([]byte, 1), }}, } diff --git a/state/compatibility_test.go b/state/compatibility_test.go new file mode 100644 index 0000000000..895c39d8ce --- /dev/null +++ b/state/compatibility_test.go @@ -0,0 +1,652 @@ +package state_test + +import ( + "fmt" + "testing" + "time" + + gogo "github.com/cosmos/gogoproto/types" + "github.com/stretchr/testify/require" + + dbm "github.com/cometbft/cometbft-db" + abciv1 "github.com/cometbft/cometbft/api/cometbft/abci/v1" + abciv1beta1 "github.com/cometbft/cometbft/api/cometbft/abci/v1beta1" + abciv1beta2 "github.com/cometbft/cometbft/api/cometbft/abci/v1beta2" + abciv1beta3 "github.com/cometbft/cometbft/api/cometbft/abci/v1beta3" + cryptov1 "github.com/cometbft/cometbft/api/cometbft/crypto/v1" + statev1 "github.com/cometbft/cometbft/api/cometbft/state/v1" + statev1beta2 "github.com/cometbft/cometbft/api/cometbft/state/v1beta2" + statev1beta3 "github.com/cometbft/cometbft/api/cometbft/state/v1beta3" + typesv1 "github.com/cometbft/cometbft/api/cometbft/types/v1" + typesv1beta1 "github.com/cometbft/cometbft/api/cometbft/types/v1beta1" + typesv1beta2 "github.com/cometbft/cometbft/api/cometbft/types/v1beta2" + "github.com/cometbft/cometbft/crypto/ed25519" + sm "github.com/cometbft/cometbft/state" +) + +// Compatibility test across different state proto versions + +func calcABCIResponsesKey(height int64) []byte { + return []byte(fmt.Sprintf("abciResponsesKey:%v", height)) +} + +var lastABCIResponseKey = []byte("lastABCIResponseKey") + +var ( + _ sm.Store = (*MultiStore)(nil) + _ LegacyStore = (*MultiStore)(nil) +) + +// MultiStore represents a state store that implements the Store interface +// and contains additional store and database options. +// +// Fields: +// - Store (sm.Store): The store instance used by the MultiStore. +// - db (dbm.DB): The database instance used by the MultiStore. +// - StoreOptions (sm.StoreOptions): The options for the MultiStore. +type MultiStore struct { + sm.Store + db dbm.DB + sm.StoreOptions +} + +// NewMultiStore initializes a new instance of MultiStore with the provided parameters. +// It sets the store, db, and StoreOptions fields of the MultiStore struct. +// +// Parameters: +// - db (dbm.DB): The database instance to be used by the MultiStore. +// - options (sm.StoreOptions): The store options to be used by the MultiStore. +// - store (sm.Store): The store instance to be used by the MultiStore. +// +// Returns: +// - *MultiStore: A pointer to the newly created MultiStore instance. +func NewMultiStore(db dbm.DB, options sm.StoreOptions, store sm.Store) *MultiStore { + return &MultiStore{ + Store: store, + db: db, + StoreOptions: options, + } +} + +// LegacyStore represents a legacy data store. +// Example usage: +// +// _ LegacyStore = (*MultiStore)(nil) +type LegacyStore interface { + SaveABCIResponses(height int64, abciResponses *statev1beta2.ABCIResponses) error +} + +// SaveABCIResponses saves the ABCIResponses for a given height in the MultiStore. +// It strips out any nil values from the DeliverTxs field, and saves the ABCIResponses to +// disk if the DiscardABCIResponses flag is set to false. It also saves the last ABCI response +// for crash recovery, overwriting the previously saved response. +// +// Parameters: +// - height (int64): The height at which the ABCIResponses are being saved. +// - abciResponses (ABCIResponses): The ABCIResponses to be saved. +// +// Returns: +// - error: An error if there was a problem saving the ABCIResponses. +// +// NOTE: The MultiStore must be properly configured with the StoreOptions and db before calling this method. +func (multi MultiStore) SaveABCIResponses(height int64, abciResponses *statev1beta2.ABCIResponses) error { + var dtxs []*abciv1beta2.ResponseDeliverTx + // strip nil values, + for _, tx := range abciResponses.DeliverTxs { + if tx != nil { + dtxs = append(dtxs, tx) + } + } + abciResponses.DeliverTxs = dtxs + + // If the flag is false then we save the ABCIResponse. This can be used for the /BlockResults + // query or to reindex an event using the command line. + if !multi.StoreOptions.DiscardABCIResponses { + bz, err := abciResponses.Marshal() + if err != nil { + return err + } + if err := multi.db.Set(calcABCIResponsesKey(height), bz); err != nil { + return err + } + } + + // We always save the last ABCI response for crash recovery. + // This overwrites the previous saved ABCI Response. + response := &statev1beta2.ABCIResponsesInfo{ + AbciResponses: abciResponses, + Height: height, + } + bz, err := response.Marshal() + if err != nil { + return err + } + + return multi.db.SetSync(lastABCIResponseKey, bz) +} + +// TestSaveLegacyAndLoadFinalizeBlock tests saving and loading of ABCIResponses +// using the multiStore. It verifies that the loaded ABCIResponses match the +// original ones and that missing fields are correctly handled. +// This test is important for the LoadFinalizeBlockResponse method in the state store. +func TestSaveLegacyAndLoadFinalizeBlock(t *testing.T) { + tearDown, stateDB, _, store := setupTestCaseWithStore(t) + defer tearDown(t) + options := sm.StoreOptions{ + DiscardABCIResponses: false, + } + + height := int64(1) + multiStore := NewMultiStore(stateDB, options, store) + + // try with a complete ABCI Response + v1beta2ABCIResponses := newV1Beta2ABCIResponses() + err := multiStore.SaveABCIResponses(height, &v1beta2ABCIResponses) + require.NoError(t, err) + require.Equal(t, 1, len(v1beta2ABCIResponses.DeliverTxs)) + require.Equal(t, 1, len(v1beta2ABCIResponses.BeginBlock.Events)) + require.Equal(t, 1, len(v1beta2ABCIResponses.EndBlock.Events)) + + finalizeBlockResponse, err := multiStore.LoadFinalizeBlockResponse(height) + require.NoError(t, err) + + // Test for not nil + require.NotNil(t, finalizeBlockResponse.TxResults) + require.NotNil(t, finalizeBlockResponse.Events) + require.NotNil(t, finalizeBlockResponse.ValidatorUpdates) + require.NotNil(t, finalizeBlockResponse.ConsensusParamUpdates) + require.Nil(t, finalizeBlockResponse.AppHash) + + // Test for equality + require.Equal(t, 1, len(finalizeBlockResponse.TxResults)) + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs), len(finalizeBlockResponse.TxResults)) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Code, finalizeBlockResponse.TxResults[0].Code) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Data, finalizeBlockResponse.TxResults[0].Data) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Log, finalizeBlockResponse.TxResults[0].Log) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].GasWanted, finalizeBlockResponse.TxResults[0].GasWanted) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].GasUsed, finalizeBlockResponse.TxResults[0].GasUsed) + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs[0].Events), len(finalizeBlockResponse.TxResults[0].Events)) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Events[0].Type, finalizeBlockResponse.TxResults[0].Events[0].Type) + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes), len(finalizeBlockResponse.TxResults[0].Events[0].Attributes)) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Key, finalizeBlockResponse.TxResults[0].Events[0].Attributes[0].Key) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Value, finalizeBlockResponse.TxResults[0].Events[0].Attributes[0].Value) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Codespace, finalizeBlockResponse.TxResults[0].Codespace) + + require.Equal(t, 2, len(finalizeBlockResponse.Events)) + require.Equal(t, len(v1beta2ABCIResponses.BeginBlock.Events)+len(v1beta2ABCIResponses.EndBlock.Events), len(finalizeBlockResponse.Events)) + + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Type, finalizeBlockResponse.Events[0].Type) + require.Equal(t, len(v1beta2ABCIResponses.BeginBlock.Events[0].Attributes)+1, len(finalizeBlockResponse.Events[0].Attributes)) // +1 for inject 'mode' attribute + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Attributes[0].Key, finalizeBlockResponse.Events[0].Attributes[0].Key) + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Attributes[0].Value, finalizeBlockResponse.Events[0].Attributes[0].Value) + + require.Equal(t, v1beta2ABCIResponses.EndBlock.ConsensusParamUpdates.Block.MaxBytes, finalizeBlockResponse.ConsensusParamUpdates.Block.MaxBytes) + require.Equal(t, v1beta2ABCIResponses.EndBlock.ConsensusParamUpdates.Block.MaxGas, finalizeBlockResponse.ConsensusParamUpdates.Block.MaxGas) + require.Equal(t, v1beta2ABCIResponses.EndBlock.ConsensusParamUpdates.Evidence.MaxAgeNumBlocks, finalizeBlockResponse.ConsensusParamUpdates.Evidence.MaxAgeNumBlocks) + require.Equal(t, v1beta2ABCIResponses.EndBlock.ConsensusParamUpdates.Evidence.MaxAgeDuration, finalizeBlockResponse.ConsensusParamUpdates.Evidence.MaxAgeDuration) + require.Equal(t, v1beta2ABCIResponses.EndBlock.ConsensusParamUpdates.Evidence.MaxBytes, finalizeBlockResponse.ConsensusParamUpdates.Evidence.MaxBytes) + require.Equal(t, v1beta2ABCIResponses.EndBlock.ConsensusParamUpdates.Validator.PubKeyTypes, finalizeBlockResponse.ConsensusParamUpdates.Validator.PubKeyTypes) + require.Equal(t, v1beta2ABCIResponses.EndBlock.ConsensusParamUpdates.Version.App, finalizeBlockResponse.ConsensusParamUpdates.Version.App) + + require.Nil(t, finalizeBlockResponse.ConsensusParamUpdates.Abci) + require.Nil(t, finalizeBlockResponse.ConsensusParamUpdates.Synchrony) + require.Nil(t, finalizeBlockResponse.ConsensusParamUpdates.Feature) + require.Nil(t, finalizeBlockResponse.AppHash) + + require.Equal(t, len(v1beta2ABCIResponses.EndBlock.ValidatorUpdates), len(finalizeBlockResponse.ValidatorUpdates)) + require.Equal(t, v1beta2ABCIResponses.EndBlock.ValidatorUpdates[0].Power, finalizeBlockResponse.ValidatorUpdates[0].Power) + + // skip until an equivalency test is possible + // require.NotNil(t, finalizeBlockResponse.ValidatorUpdates[0].PubKeyBytes) + // require.NotEmpty(t, finalizeBlockResponse.ValidatorUpdates[0].PubKeyType) + // require.Equal(t, v1beta2ABCIResponses.ValidatorUpdates[0].PubKey.GetEd25519(), finalizeBlockResponse.ValidatorUpdates[0].PubKeyBytes) + + // try with an ABCI Response missing fields + height = int64(2) + v1beta2ABCIResponses = newV1Beta2ABCIResponsesWithNullFields() + require.Equal(t, 1, len(v1beta2ABCIResponses.DeliverTxs)) + require.Equal(t, 1, len(v1beta2ABCIResponses.BeginBlock.Events)) + require.Nil(t, v1beta2ABCIResponses.EndBlock) + err = multiStore.SaveABCIResponses(height, &v1beta2ABCIResponses) + require.NoError(t, err) + finalizeBlockResponse, err = multiStore.LoadFinalizeBlockResponse(height) + require.NoError(t, err) + + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs), len(finalizeBlockResponse.TxResults)) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].String(), finalizeBlockResponse.TxResults[0].String()) + require.Equal(t, len(v1beta2ABCIResponses.BeginBlock.Events), len(finalizeBlockResponse.Events)) +} + +// This test un-marshals a v1beta2.ABCIResponses as a statev1.LegacyABCIResponses +// This logic is important for the LoadFinalizeBlockResponse method in the state store +// The conversion should not fail because they are compatible. +// + +func TestStateV1Beta2ABCIResponsesAsStateV1LegacyABCIResponse(t *testing.T) { + v1beta2ABCIResponses := newV1Beta2ABCIResponses() + + v1b2Resp, err := v1beta2ABCIResponses.Marshal() + require.NoError(t, err) + require.NotNil(t, v1b2Resp) + + // un-marshall a v1beta2 ABCI Response as a LegacyABCIResponse + legacyABCIResponses := new(statev1.LegacyABCIResponses) + err = legacyABCIResponses.Unmarshal(v1b2Resp) + require.NoError(t, err) + + // ensure not nil + require.NotNil(t, legacyABCIResponses.DeliverTxs) + require.NotNil(t, legacyABCIResponses.EndBlock) + require.NotNil(t, legacyABCIResponses.BeginBlock) + + // ensure for equality + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs), len(legacyABCIResponses.DeliverTxs)) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Code, legacyABCIResponses.DeliverTxs[0].Code) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Data, legacyABCIResponses.DeliverTxs[0].Data) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Log, legacyABCIResponses.DeliverTxs[0].Log) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].GasWanted, legacyABCIResponses.DeliverTxs[0].GasWanted) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].GasUsed, legacyABCIResponses.DeliverTxs[0].GasUsed) + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs[0].Events), len(legacyABCIResponses.DeliverTxs[0].Events)) + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes), len(legacyABCIResponses.DeliverTxs[0].Events[0].Attributes)) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Key, legacyABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Key) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Value, legacyABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Value) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Codespace, legacyABCIResponses.DeliverTxs[0].Codespace) + + require.Equal(t, len(v1beta2ABCIResponses.BeginBlock.Events), len(legacyABCIResponses.BeginBlock.Events)) + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Type, legacyABCIResponses.BeginBlock.Events[0].Type) + require.Equal(t, len(v1beta2ABCIResponses.BeginBlock.Events[0].Attributes), len(legacyABCIResponses.BeginBlock.Events[0].Attributes)) + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Attributes[0].Key, legacyABCIResponses.BeginBlock.Events[0].Attributes[0].Key) + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Attributes[0].Value, legacyABCIResponses.BeginBlock.Events[0].Attributes[0].Value) +} + +// This test unmarshals a v1beta2.ABCIResponses as a v1beta3.ResponseFinalizeBlock +// This logic is important for the LoadFinalizeBlockResponse method in the state store +// The conversion should fail because they are not compatible. +func TestStateV1Beta2ABCIResponsesAsV1Beta3ResponseFinalizeBlock(t *testing.T) { + v1beta2ABCIResponses := newV1Beta2ABCIResponses() + data, err := v1beta2ABCIResponses.Marshal() + require.NoError(t, err) + require.NotNil(t, data) + + // This cannot work since they have different schemas, a wrong wireType error is generated + responseFinalizeBlock := new(abciv1beta3.ResponseFinalizeBlock) + err = responseFinalizeBlock.Unmarshal(data) + require.Error(t, err) + require.ErrorContains(t, err, "unexpected EOF") +} + +// This test unmarshal a v1beta2.ABCIResponses as a v1.FinalizeBlockResponse +// This logic is important for the LoadFinalizeBlockResponse method in the state store +// The conversion should fail because they are not compatible. +func TestStateV1Beta2ABCIResponsesAsV1FinalizeBlockResponse(t *testing.T) { + v1beta2ABCIResponses := newV1Beta2ABCIResponses() + data, err := v1beta2ABCIResponses.Marshal() + require.NoError(t, err) + require.NotNil(t, data) + + // This cannot work since they have different schemas, a wrong wireType error is generated + finalizeBlockResponse := new(abciv1.FinalizeBlockResponse) + err = finalizeBlockResponse.Unmarshal(data) + require.Error(t, err) + require.ErrorContains(t, err, "unexpected EOF") +} + +// This test unmarshal a v1beta2.ABCIResponses as a v1beta3.ResponseFinalizeBlock +// This logic is important for the LoadFinalizeBlockResponse method in the state store +// The conversion doesn't fail because no error is return, but they are NOT compatible. +func TestStateV1Beta2ABCIResponsesWithNullAsV1Beta3ResponseFinalizeBlock(t *testing.T) { + v1beta2ABCIResponsesWithNull := newV1Beta2ABCIResponsesWithNullFields() + data, err := v1beta2ABCIResponsesWithNull.Marshal() + require.NoError(t, err) + require.NotNil(t, data) + + // This should not work since they have different schemas + // but an error is not returned, so it deserializes an ABCIResponse + // on top of a FinalizeBlockResponse giving the false impression they are the same + // but because it doesn't error out, the fields in finalizeBlockResponse will have + // their zero-value (e.g. nil, 0, "") + finalizeBlockResponse := new(abciv1beta3.ResponseFinalizeBlock) + err = finalizeBlockResponse.Unmarshal(data) + require.NoError(t, err) + require.Nil(t, finalizeBlockResponse.AppHash) + require.Nil(t, finalizeBlockResponse.TxResults) +} + +// This test unmarshal a v1beta2.ABCIResponses as a statev1beta3.LegacyABCIResponses +// This logic is important for the LoadFinalizeBlockResponse method in the state store +// The conversion should work because they are compatible. +// + +func TestStateV1Beta2ABCIResponsesAsStateV1Beta3LegacyABCIResponse(t *testing.T) { + v1beta2ABCIResponses := newV1Beta2ABCIResponses() + + data, err := v1beta2ABCIResponses.Marshal() + require.NoError(t, err) + require.NotNil(t, data) + + // This works because they are equivalent protos and the fields are populated + legacyABCIResponses := new(statev1beta3.LegacyABCIResponses) + err = legacyABCIResponses.Unmarshal(data) + require.NoError(t, err) + + // ensure not nil + require.NotNil(t, legacyABCIResponses.DeliverTxs) + require.NotNil(t, legacyABCIResponses.EndBlock) + require.NotNil(t, legacyABCIResponses.BeginBlock) + + // ensure for equality + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs), len(legacyABCIResponses.DeliverTxs)) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Code, legacyABCIResponses.DeliverTxs[0].Code) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Data, legacyABCIResponses.DeliverTxs[0].Data) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Log, legacyABCIResponses.DeliverTxs[0].Log) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].GasWanted, legacyABCIResponses.DeliverTxs[0].GasWanted) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].GasUsed, legacyABCIResponses.DeliverTxs[0].GasUsed) + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs[0].Events), len(legacyABCIResponses.DeliverTxs[0].Events)) + require.Equal(t, len(v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes), len(legacyABCIResponses.DeliverTxs[0].Events[0].Attributes)) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Key, legacyABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Key) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Value, legacyABCIResponses.DeliverTxs[0].Events[0].Attributes[0].Value) + require.Equal(t, v1beta2ABCIResponses.DeliverTxs[0].Codespace, legacyABCIResponses.DeliverTxs[0].Codespace) + + require.Equal(t, len(v1beta2ABCIResponses.BeginBlock.Events), len(legacyABCIResponses.BeginBlock.Events)) + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events, legacyABCIResponses.BeginBlock.Events) + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Type, legacyABCIResponses.BeginBlock.Events[0].Type) + require.Equal(t, len(v1beta2ABCIResponses.BeginBlock.Events[0].Attributes), len(legacyABCIResponses.BeginBlock.Events[0].Attributes)) + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Attributes[0].Key, legacyABCIResponses.BeginBlock.Events[0].Attributes[0].Key) + require.Equal(t, v1beta2ABCIResponses.BeginBlock.Events[0].Attributes[0].Value, legacyABCIResponses.BeginBlock.Events[0].Attributes[0].Value) +} + +// This test unmarshal a v1beta2.ABCIResponsesWithNullFields as a statev1beta3.LegacyABCIResponses +// This logic is important for the LoadFinalizeBlockResponse method in the state store +// The conversion should work because they are compatible even if fields to be converted are null. +func TestStateV1Beta2ABCIResponsesWithNullAsStateV1Beta3LegacyABCIResponse(t *testing.T) { + v1beta2ABCIResponsesWithNull := newV1Beta2ABCIResponsesWithNullFields() + data, err := v1beta2ABCIResponsesWithNull.Marshal() + require.NoError(t, err) + require.NotNil(t, data) + + // This works because they are equivalent protos and the fields are populated + // even if a field is null, it will be converted properly + legacyResponseWithNull := new(statev1beta3.LegacyABCIResponses) + err = legacyResponseWithNull.Unmarshal(data) + require.NoError(t, err) + require.NotNil(t, legacyResponseWithNull.DeliverTxs) + require.Nil(t, legacyResponseWithNull.EndBlock) + require.NotNil(t, legacyResponseWithNull.BeginBlock) + + require.Equal(t, len(v1beta2ABCIResponsesWithNull.BeginBlock.Events), len(legacyResponseWithNull.BeginBlock.Events)) + require.Equal(t, v1beta2ABCIResponsesWithNull.BeginBlock.Events, legacyResponseWithNull.BeginBlock.Events) + require.Equal(t, v1beta2ABCIResponsesWithNull.BeginBlock.Events[0].Type, legacyResponseWithNull.BeginBlock.Events[0].Type) + require.Equal(t, len(v1beta2ABCIResponsesWithNull.BeginBlock.Events[0].Attributes), len(legacyResponseWithNull.BeginBlock.Events[0].Attributes)) + require.Equal(t, v1beta2ABCIResponsesWithNull.BeginBlock.Events[0].Attributes[0].Key, legacyResponseWithNull.BeginBlock.Events[0].Attributes[0].Key) + require.Equal(t, v1beta2ABCIResponsesWithNull.BeginBlock.Events[0].Attributes[0].Value, legacyResponseWithNull.BeginBlock.Events[0].Attributes[0].Value) + + require.Equal(t, len(v1beta2ABCIResponsesWithNull.DeliverTxs), len(legacyResponseWithNull.DeliverTxs)) + require.Equal(t, v1beta2ABCIResponsesWithNull.DeliverTxs[0].Code, legacyResponseWithNull.DeliverTxs[0].Code) + require.Equal(t, v1beta2ABCIResponsesWithNull.DeliverTxs[0].Data, legacyResponseWithNull.DeliverTxs[0].Data) + require.Equal(t, v1beta2ABCIResponsesWithNull.DeliverTxs[0].Log, legacyResponseWithNull.DeliverTxs[0].Log) + require.Equal(t, v1beta2ABCIResponsesWithNull.DeliverTxs[0].GasWanted, legacyResponseWithNull.DeliverTxs[0].GasWanted) + require.Equal(t, v1beta2ABCIResponsesWithNull.DeliverTxs[0].GasUsed, legacyResponseWithNull.DeliverTxs[0].GasUsed) + require.Equal(t, len(v1beta2ABCIResponsesWithNull.DeliverTxs[0].Events), len(legacyResponseWithNull.DeliverTxs[0].Events)) + require.Equal(t, len(v1beta2ABCIResponsesWithNull.DeliverTxs[0].Events[0].Attributes), len(legacyResponseWithNull.DeliverTxs[0].Events[0].Attributes)) + require.Equal(t, v1beta2ABCIResponsesWithNull.DeliverTxs[0].Events[0].Attributes[0].Key, legacyResponseWithNull.DeliverTxs[0].Events[0].Attributes[0].Key) + require.Equal(t, v1beta2ABCIResponsesWithNull.DeliverTxs[0].Events[0].Attributes[0].Value, legacyResponseWithNull.DeliverTxs[0].Events[0].Attributes[0].Value) + require.Equal(t, v1beta2ABCIResponsesWithNull.DeliverTxs[0].Codespace, legacyResponseWithNull.DeliverTxs[0].Codespace) +} + +// This test unmarshal a v1beta3.ResponseFinalizeBlock as a abciv1.FinalizeBlockResponse +// This logic is important for the LoadFinalizeBlockResponse method in the state store +// The conversion should work because they are compatible. +func TestStateV1Beta3ResponsesFinalizeBlockAsV1FinalizeBlockResponse(t *testing.T) { + v1beta3ResponseFinalizeBlock := newV1Beta3ResponsesFinalizeBlock() + + data, err := v1beta3ResponseFinalizeBlock.Marshal() + require.NoError(t, err) + require.NotNil(t, data) + + // This works because they are equivalent protos and the fields are populated + finalizeBlockResponse := new(abciv1.FinalizeBlockResponse) + err = finalizeBlockResponse.Unmarshal(data) + require.NoError(t, err) + + // Test for not nil + require.NotNil(t, finalizeBlockResponse.TxResults) + require.NotNil(t, finalizeBlockResponse.Events) + require.NotNil(t, finalizeBlockResponse.ValidatorUpdates) + require.NotNil(t, finalizeBlockResponse.ConsensusParamUpdates) + require.NotNil(t, finalizeBlockResponse.AppHash) + + // Test for equality + require.Equal(t, len(v1beta3ResponseFinalizeBlock.TxResults), len(finalizeBlockResponse.TxResults)) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].Code, finalizeBlockResponse.TxResults[0].Code) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].Data, finalizeBlockResponse.TxResults[0].Data) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].Log, finalizeBlockResponse.TxResults[0].Log) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].GasWanted, finalizeBlockResponse.TxResults[0].GasWanted) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].GasUsed, finalizeBlockResponse.TxResults[0].GasUsed) + require.Equal(t, len(v1beta3ResponseFinalizeBlock.TxResults[0].Events), len(finalizeBlockResponse.TxResults[0].Events)) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].Events[0].Type, finalizeBlockResponse.TxResults[0].Events[0].Type) + require.Equal(t, len(v1beta3ResponseFinalizeBlock.TxResults[0].Events[0].Attributes), len(finalizeBlockResponse.TxResults[0].Events[0].Attributes)) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].Events[0].Attributes[0].Key, finalizeBlockResponse.TxResults[0].Events[0].Attributes[0].Key) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].Events[0].Attributes[0].Value, finalizeBlockResponse.TxResults[0].Events[0].Attributes[0].Value) + require.Equal(t, v1beta3ResponseFinalizeBlock.TxResults[0].Codespace, finalizeBlockResponse.TxResults[0].Codespace) + + require.Equal(t, len(v1beta3ResponseFinalizeBlock.Events), len(finalizeBlockResponse.Events)) + require.Equal(t, v1beta3ResponseFinalizeBlock.Events[0].Type, finalizeBlockResponse.Events[0].Type) + require.Equal(t, len(v1beta3ResponseFinalizeBlock.Events[0].Attributes), len(finalizeBlockResponse.Events[0].Attributes)) + require.Equal(t, v1beta3ResponseFinalizeBlock.Events[0].Attributes[0].Key, finalizeBlockResponse.Events[0].Attributes[0].Key) + require.Equal(t, v1beta3ResponseFinalizeBlock.Events[0].Attributes[0].Value, finalizeBlockResponse.Events[0].Attributes[0].Value) + + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates, finalizeBlockResponse.ConsensusParamUpdates) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Block.MaxBytes, finalizeBlockResponse.ConsensusParamUpdates.Block.MaxBytes) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Block.MaxGas, finalizeBlockResponse.ConsensusParamUpdates.Block.MaxGas) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Evidence.MaxAgeNumBlocks, finalizeBlockResponse.ConsensusParamUpdates.Evidence.MaxAgeNumBlocks) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Evidence.MaxAgeDuration, finalizeBlockResponse.ConsensusParamUpdates.Evidence.MaxAgeDuration) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Evidence.MaxBytes, finalizeBlockResponse.ConsensusParamUpdates.Evidence.MaxBytes) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Validator.PubKeyTypes, finalizeBlockResponse.ConsensusParamUpdates.Validator.PubKeyTypes) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Version.App, finalizeBlockResponse.ConsensusParamUpdates.Version.App) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Synchrony.Precision, finalizeBlockResponse.ConsensusParamUpdates.Synchrony.Precision) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Synchrony.MessageDelay, finalizeBlockResponse.ConsensusParamUpdates.Synchrony.MessageDelay) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Feature.VoteExtensionsEnableHeight.Value, finalizeBlockResponse.ConsensusParamUpdates.Feature.VoteExtensionsEnableHeight.Value) + require.Equal(t, v1beta3ResponseFinalizeBlock.ConsensusParamUpdates.Feature.PbtsEnableHeight.Value, finalizeBlockResponse.ConsensusParamUpdates.Feature.PbtsEnableHeight.Value) + + require.Equal(t, v1beta3ResponseFinalizeBlock.AppHash, finalizeBlockResponse.AppHash) + + require.Equal(t, len(v1beta3ResponseFinalizeBlock.ValidatorUpdates), len(finalizeBlockResponse.ValidatorUpdates)) + require.Equal(t, v1beta3ResponseFinalizeBlock.ValidatorUpdates[0].Power, finalizeBlockResponse.ValidatorUpdates[0].Power) + + // skip until an equivalency test is possible + // require.NotNil(t, finalizeBlockResponse.ValidatorUpdates[0].PubKeyBytes) + // require.NotEmpty(t, finalizeBlockResponse.ValidatorUpdates[0].PubKeyType) + // require.Equal(t, v1beta3ResponseFinalizeBlock.ValidatorUpdates[0].PubKey.GetEd25519(), finalizeBlockResponse.ValidatorUpdates[0].PubKeyBytes) +} + +// Generate a v1beta2 ABCIResponses with data for all fields. +func newV1Beta2ABCIResponses() statev1beta2.ABCIResponses { + eventAttr := abciv1beta2.EventAttribute{ + Key: "key", + Value: "value", + } + + deliverTxEvent := abciv1beta2.Event{ + Type: "deliver_tx_event", + Attributes: []abciv1beta2.EventAttribute{eventAttr}, + } + + responseDeliverTx := abciv1beta2.ResponseDeliverTx{ + Code: abciv1beta1.CodeTypeOK, + Data: []byte("result tx data"), + Log: "tx committed successfully", + Info: "tx processing info", + Events: []abciv1beta2.Event{deliverTxEvent}, + } + + validatorUpdates := []abciv1beta1.ValidatorUpdate{{ + PubKey: cryptov1.PublicKey{Sum: &cryptov1.PublicKey_Ed25519{Ed25519: make([]byte, 1)}}, + Power: int64(10), + }} + + consensusParams := &typesv1beta2.ConsensusParams{ + Block: &typesv1beta2.BlockParams{ + MaxBytes: int64(100000), + MaxGas: int64(10000), + }, + Evidence: &typesv1beta1.EvidenceParams{ + MaxAgeNumBlocks: int64(10), + MaxAgeDuration: time.Duration(1000), + MaxBytes: int64(10000), + }, + Validator: &typesv1beta1.ValidatorParams{ + PubKeyTypes: []string{"ed25519"}, + }, + Version: &typesv1beta1.VersionParams{ + App: uint64(10), + }, + } + + endBlockEvent := abciv1beta2.Event{ + Type: "end_block_event", + Attributes: []abciv1beta2.EventAttribute{eventAttr}, + } + + beginBlockEvent := abciv1beta2.Event{ + Type: "begin_block_event", + Attributes: []abciv1beta2.EventAttribute{eventAttr}, + } + + // v1beta2 ABCI Responses + v1beta2ABCIResponses := statev1beta2.ABCIResponses{ + BeginBlock: &abciv1beta2.ResponseBeginBlock{ + Events: []abciv1beta2.Event{beginBlockEvent}, + }, + DeliverTxs: []*abciv1beta2.ResponseDeliverTx{ + &responseDeliverTx, + }, + EndBlock: &abciv1beta2.ResponseEndBlock{ + ValidatorUpdates: validatorUpdates, + ConsensusParamUpdates: consensusParams, + Events: []abciv1beta2.Event{endBlockEvent}, + }, + } + return v1beta2ABCIResponses +} + +// Generate a v1beta2 ABCIResponses with fields missing data (nil). +func newV1Beta2ABCIResponsesWithNullFields() statev1beta2.ABCIResponses { + eventAttr := abciv1beta2.EventAttribute{ + Key: "key", + Value: "value", + } + + deliverTxEvent := abciv1beta2.Event{ + Type: "deliver_tx_event", + Attributes: []abciv1beta2.EventAttribute{eventAttr}, + } + + responseDeliverTx := abciv1beta2.ResponseDeliverTx{ + Code: abciv1beta1.CodeTypeOK, + Events: []abciv1beta2.Event{deliverTxEvent}, + } + + beginBlockEvent := abciv1beta2.Event{ + Type: "begin_block_event", + Attributes: []abciv1beta2.EventAttribute{eventAttr}, + } + + // v1beta2 ABCI Responses + v1beta2ABCIResponses := statev1beta2.ABCIResponses{ + BeginBlock: &abciv1beta2.ResponseBeginBlock{ + Events: []abciv1beta2.Event{beginBlockEvent}, + }, + DeliverTxs: []*abciv1beta2.ResponseDeliverTx{ + &responseDeliverTx, + }, + } + return v1beta2ABCIResponses +} + +// Generate a v1beta3 Response Finalize Block with data for all fields. +func newV1Beta3ResponsesFinalizeBlock() abciv1beta3.ResponseFinalizeBlock { + eventAttr := abciv1beta2.EventAttribute{ + Key: "key", + Value: "value", + } + + txEvent := abciv1beta2.Event{ + Type: "tx_event", + Attributes: []abciv1beta2.EventAttribute{eventAttr}, + } + + oneEvent := abciv1beta2.Event{ + Type: "one_event", + Attributes: []abciv1beta2.EventAttribute{eventAttr}, + } + + twoEvent := abciv1beta2.Event{ + Type: "two_event", + Attributes: []abciv1beta2.EventAttribute{eventAttr}, + } + + events := make([]abciv1beta2.Event, 0) + events = append(events, txEvent) + events = append(events, oneEvent) + events = append(events, twoEvent) + + txResults := []*abciv1beta3.ExecTxResult{{ + Code: 0, + Data: []byte("result tx data"), + Log: "tx committed successfully", + Info: "tx processing info", + GasWanted: 15, + GasUsed: 10, + Events: []abciv1beta2.Event{txEvent}, + Codespace: "01", + }} + + validatorUpdates := []abciv1beta1.ValidatorUpdate{{ + PubKey: cryptov1.PublicKey{Sum: &cryptov1.PublicKey_Ed25519{Ed25519: make([]byte, ed25519.PubKeySize)}}, + Power: int64(10), + }} + + consensusParams := &typesv1.ConsensusParams{ + Block: &typesv1.BlockParams{ + MaxBytes: int64(100000), + MaxGas: int64(10000), + }, + Evidence: &typesv1.EvidenceParams{ + MaxAgeNumBlocks: int64(10), + MaxAgeDuration: time.Duration(1000), + MaxBytes: int64(10000), + }, + Validator: &typesv1.ValidatorParams{ + PubKeyTypes: []string{ed25519.KeyType}, + }, + Version: &typesv1.VersionParams{ + App: uint64(10), + }, + Synchrony: &typesv1.SynchronyParams{ + Precision: durationPtr(time.Second * 2), + MessageDelay: durationPtr(time.Second * 4), + }, + Feature: &typesv1.FeatureParams{ + VoteExtensionsEnableHeight: &gogo.Int64Value{ + Value: 10, + }, + PbtsEnableHeight: &gogo.Int64Value{ + Value: 10, + }, + }, + } + + // v1beta3 FinalizeBlock Response + v1beta3FinalizeBlock := abciv1beta3.ResponseFinalizeBlock{ + Events: events, + TxResults: txResults, + ValidatorUpdates: validatorUpdates, + ConsensusParamUpdates: consensusParams, + AppHash: make([]byte, 32), + } + return v1beta3FinalizeBlock +} + +func durationPtr(t time.Duration) *time.Duration { + return &t +} diff --git a/state/errors.go b/state/errors.go index bbad8141af..40131641bf 100644 --- a/state/errors.go +++ b/state/errors.go @@ -5,6 +5,12 @@ import ( "fmt" ) +var ( + ErrFinalizeBlockResponsesNotPersisted = errors.New("node is not persisting finalize block responses") + ErrPrunerCannotLowerRetainHeight = errors.New("cannot set a height lower than previously requested - heights might have already been pruned") + ErrInvalidRetainHeight = errors.New("retain height cannot be less or equal than 0") +) + type ( ErrInvalidBlock error ErrProxyAppConn error @@ -74,6 +80,15 @@ type ( ErrCannotLoadState struct { Err error } + + ErrABCIResponseResponseUnmarshalForHeight struct { + Height int64 + } + + ErrABCIResponseCorruptedOrSpecChangeForHeight struct { + Err error + Height int64 + } ) func (e ErrUnknownBlock) Error() string { @@ -126,6 +141,18 @@ func (e ErrNoABCIResponsesForHeight) Error() string { return fmt.Sprintf("could not find results for height #%d", e.Height) } +func (e ErrABCIResponseResponseUnmarshalForHeight) Error() string { + return fmt.Sprintf("could not decode results for height %d", e.Height) +} + +func (e ErrABCIResponseCorruptedOrSpecChangeForHeight) Error() string { + return fmt.Sprintf("failed to unmarshall FinalizeBlockResponse (also tried as legacy ABCI response) for height %d", e.Height) +} + +func (e ErrABCIResponseCorruptedOrSpecChangeForHeight) Unwrap() error { + return e.Err +} + func (e ErrPrunerFailedToGetRetainHeight) Error() string { return fmt.Sprintf("pruner failed to get existing %s retain height: %s", e.Which, e.Err.Error()) } @@ -158,12 +185,6 @@ func (e ErrFailedToPruneStates) Unwrap() error { return e.Err } -var ( - ErrFinalizeBlockResponsesNotPersisted = errors.New("node is not persisting finalize block responses") - ErrPrunerCannotLowerRetainHeight = errors.New("cannot set a height lower than previously requested - heights might have already been pruned") - ErrInvalidRetainHeight = errors.New("retain height cannot be less or equal than 0") -) - func (e ErrCannotLoadState) Error() string { return fmt.Sprintf("cannot load state: %v", e.Err) } diff --git a/state/state_test.go b/state/state_test.go index dc37f8f007..2d8a7f16e3 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -23,6 +23,13 @@ import ( // setupTestCase does setup common to all test cases. func setupTestCase(t *testing.T) (func(t *testing.T), dbm.DB, sm.State) { + t.Helper() + tearDown, stateDB, state, _ := setupTestCaseWithStore(t) + return tearDown, stateDB, state +} + +// setupTestCase does setup common to all test cases. +func setupTestCaseWithStore(t *testing.T) (func(t *testing.T), dbm.DB, sm.State, sm.Store) { t.Helper() config := test.ResetTestRoot("state_") dbType := dbm.BackendType(config.DBBackend) @@ -41,7 +48,7 @@ func setupTestCase(t *testing.T) (func(t *testing.T), dbm.DB, sm.State) { os.RemoveAll(config.RootDir) } - return tearDown, stateDB, state + return tearDown, stateDB, state, stateStore } // TestStateCopy tests the correct copying behavior of State. @@ -121,6 +128,8 @@ func TestFinalizeBlockResponsesSaveLoad1(t *testing.T) { abci.NewValidatorUpdate(ed25519.GenPrivKey().PubKey(), 10), } + abciResponses.AppHash = make([]byte, 1) + err := stateStore.SaveFinalizeBlockResponse(block.Height, abciResponses) require.NoError(t, err) loadedABCIResponses, err := stateStore.LoadFinalizeBlockResponse(block.Height) @@ -524,7 +533,7 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) { _, updatedVal2 := updatedState3.NextValidators.GetByAddress(val2PubKey.Address()) // 2. Scale - // old prios: v1(10):-38, v2(1):39 + // old prios: cryptov1(10):-38, v2(1):39 wantVal1Prio = prevVal1.ProposerPriority wantVal2Prio = prevVal2.ProposerPriority // scale to diffMax = 22 = 2 * tvp, diff=39-(-38)=77 @@ -533,14 +542,14 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) { dist := wantVal2Prio - wantVal1Prio // ratio := (dist + 2*totalPower - 1) / 2*totalPower = 98/22 = 4 ratio := (dist + 2*totalPower - 1) / (2 * totalPower) - // v1(10):-38/4, v2(1):39/4 + // cryptov1(10):-38/4, v2(1):39/4 wantVal1Prio /= ratio // -9 wantVal2Prio /= ratio // 9 // 3. Center - noop // 4. IncrementProposerPriority() -> - // v1(10):-9+10, v2(1):9+1 -> v2 proposer so subsract tvp(11) - // v1(10):1, v2(1):-1 + // cryptov1(10):-9+10, v2(1):9+1 -> v2 proposer so subsract tvp(11) + // cryptov1(10):1, v2(1):-1 wantVal2Prio += updatedVal2.VotingPower // 10 -> prop wantVal1Prio += updatedVal1.VotingPower // 1 wantVal2Prio -= totalPower // -1 diff --git a/state/store.go b/state/store.go index 1802246024..7b14920d13 100644 --- a/state/store.go +++ b/state/store.go @@ -122,7 +122,7 @@ type Store interface { LoadValidators(height int64) (*types.ValidatorSet, error) // LoadFinalizeBlockResponse loads the abciResponse for a given height LoadFinalizeBlockResponse(height int64) (*abci.FinalizeBlockResponse, error) - // LoadLastABCIResponse loads the last abciResponse for a given height + // LoadLastFinalizeBlockResponse loads the last abciResponse for a given height LoadLastFinalizeBlockResponse(height int64) (*abci.FinalizeBlockResponse, error) // LoadConsensusParams loads the consensus params for a given height LoadConsensusParams(height int64) (types.ConsensusParams, error) @@ -652,14 +652,22 @@ func (store dbStore) LoadFinalizeBlockResponse(height int64) (*abci.FinalizeBloc resp := new(abci.FinalizeBlockResponse) err = resp.Unmarshal(buf) - if err != nil { + // Check for an error or if the resp.AppHash is nil if so + // this means the unmarshalling should be a LegacyABCIResponses + // Depending on a source message content (serialized as ABCIResponses) + // there are instances where it can be deserialized as a FinalizeBlockResponse + // without causing an error. But the values will not be deserialized properly + // and, it will contain zero values, and one of them is an AppHash == nil + // This can be verified in the /state/compatibility_test.go file + if err != nil || resp.AppHash == nil { // The data might be of the legacy ABCI response type, so // we try to unmarshal that legacyResp := new(cmtstate.LegacyABCIResponses) - rerr := legacyResp.Unmarshal(buf) - if rerr != nil { - cmtos.Exit(fmt.Sprintf(`LoadFinalizeBlockResponse: Data has been corrupted or its spec has - changed: %v\n`, err)) + if err := legacyResp.Unmarshal(buf); err != nil { + // only return an error, this method is only invoked through the `/block_results` not for state logic and + // some tests, so no need to exit cometbft if there's an error, just return it. + store.Logger.Error("failed in LoadFinalizeBlockResponse", "error", ErrABCIResponseCorruptedOrSpecChangeForHeight{Height: height, Err: err}) + return nil, ErrABCIResponseCorruptedOrSpecChangeForHeight{Height: height, Err: err} } // The state store contains the old format. Migrate to // the new FinalizeBlockResponse format. Note that the @@ -669,10 +677,11 @@ func (store dbStore) LoadFinalizeBlockResponse(height int64) (*abci.FinalizeBloc // TODO: ensure that buf is completely read. + // Otherwise return the FinalizeBlockResponse return resp, nil } -// LoadLastFinalizeBlockResponses loads the FinalizeBlockResponses from the most recent height. +// LoadLastFinalizeBlockResponse loads the FinalizeBlockResponses from the most recent height. // The height parameter is used to ensure that the response corresponds to the latest height. // If not, an error is returned. // @@ -1077,14 +1086,52 @@ func min(a int64, b int64) int64 { // responseFinalizeBlockFromLegacy is a convenience function that takes the old abci responses and morphs // it to the finalize block response. Note that the app hash is missing. func responseFinalizeBlockFromLegacy(legacyResp *cmtstate.LegacyABCIResponses) *abci.FinalizeBlockResponse { - return &abci.FinalizeBlockResponse{ - TxResults: legacyResp.DeliverTxs, - ValidatorUpdates: legacyResp.EndBlock.ValidatorUpdates, - ConsensusParamUpdates: legacyResp.EndBlock.ConsensusParamUpdates, - Events: append(legacyResp.BeginBlock.Events, legacyResp.EndBlock.Events...), - // NOTE: AppHash is missing in the response but will - // be caught and filled in consensus/replay.go + var response abci.FinalizeBlockResponse + events := make([]abci.Event, 0) + + if legacyResp.DeliverTxs != nil { + response.TxResults = legacyResp.DeliverTxs + } + + // Check for begin block and end block and only append events or assign values if they are not nil + if legacyResp.BeginBlock != nil { + if legacyResp.BeginBlock.Events != nil { + // Add BeginBlock attribute to BeginBlock events + for idx := range legacyResp.BeginBlock.Events { + legacyResp.BeginBlock.Events[idx].Attributes = append(legacyResp.BeginBlock.Events[idx].Attributes, abci.EventAttribute{ + Key: "mode", + Value: "BeginBlock", + Index: false, + }) + } + events = append(events, legacyResp.BeginBlock.Events...) + } + } + if legacyResp.EndBlock != nil { + if legacyResp.EndBlock.ValidatorUpdates != nil { + response.ValidatorUpdates = legacyResp.EndBlock.ValidatorUpdates + } + if legacyResp.EndBlock.ConsensusParamUpdates != nil { + response.ConsensusParamUpdates = legacyResp.EndBlock.ConsensusParamUpdates + } + if legacyResp.EndBlock.Events != nil { + // Add EndBlock attribute to BeginBlock events + for idx := range legacyResp.EndBlock.Events { + legacyResp.EndBlock.Events[idx].Attributes = append(legacyResp.EndBlock.Events[idx].Attributes, abci.EventAttribute{ + Key: "mode", + Value: "EndBlock", + Index: false, + }) + } + events = append(events, legacyResp.EndBlock.Events...) + } } + + response.Events = events + + // NOTE: AppHash is missing in the response but will + // be caught and filled in consensus/replay.go + return &response } // ----- Util. diff --git a/state/store_test.go b/state/store_test.go index ae872263ca..63b2e24a46 100644 --- a/state/store_test.go +++ b/state/store_test.go @@ -180,6 +180,7 @@ func TestPruneStates(t *testing.T) { {Data: []byte{2}}, {Data: []byte{3}}, }, + AppHash: make([]byte, 1), }) require.NoError(t, err) } @@ -375,6 +376,7 @@ func TestABCIResPruningStandalone(t *testing.T) { TxResults: []*abci.ExecTxResult{ {Code: 32, Data: []byte("Hello"), Log: "Huh?"}, }, + AppHash: make([]byte, 1), } _, bs, txIndexer, blockIndexer, callbackF, stateStore := makeStateAndBlockStoreAndIndexers() defer callbackF() @@ -468,6 +470,7 @@ func TestFinalizeBlockResponsePruning(t *testing.T) { TxResults: []*abci.ExecTxResult{ {Code: 32, Data: []byte("Hello"), Log: "Huh?"}, }, + AppHash: make([]byte, 1), } state, bs, txIndexer, blockIndexer, callbackF, stateStore := makeStateAndBlockStoreAndIndexers() defer callbackF() @@ -526,6 +529,7 @@ func TestLastFinalizeBlockResponses(t *testing.T) { TxResults: []*abci.ExecTxResult{ {Code: 32, Data: []byte("Hello"), Log: "Huh?"}, }, + AppHash: make([]byte, 1), } stateDB = dbm.NewMemDB() @@ -622,7 +626,7 @@ func TestFinalizeBlockRecoveryUsingLegacyABCIResponses(t *testing.T) { resp, err := stateStore.LoadLastFinalizeBlockResponse(height) require.NoError(t, err) require.Equal(t, resp.ConsensusParamUpdates, &cp) - require.Equal(t, resp.Events, legacyResp.LegacyAbciResponses.BeginBlock.Events) + require.Equal(t, len(resp.Events), len(legacyResp.LegacyAbciResponses.BeginBlock.Events)) require.Equal(t, resp.TxResults[0], legacyResp.LegacyAbciResponses.DeliverTxs[0]) }