From 996ec859ba3f6e00fdbd6c5e263353af53d71363 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 8 Sep 2020 12:34:06 -0700 Subject: [PATCH 01/24] Add MinRetainBlocks config opt --- server/config/config.go | 15 +++++++++++++++ server/config/toml.go | 13 +++++++++++++ 2 files changed, 28 insertions(+) diff --git a/server/config/config.go b/server/config/config.go index 3baac8164453..22f6d093c0cf 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -43,6 +43,19 @@ type BaseConfig struct { // Note: Commitment of state will be attempted on the corresponding block. HaltTime uint64 `mapstructure:"halt-time"` + // MinRetainBlocks defines the minimum block height offset from the current + // block being committed, such that all blocks past this offset are pruned + // from Tendermint. It is used as part of the process of determining the + // ResponseCommit.RetainHeight value during ABCI Commit. + // + // This configuration value is only responsible for pruning Tendermint blocks. + // It has no bearing on application state pruning which is determined by the + // "pruning-*" configurations. + // + // Note: This configuration can and should be used in conjunction with other + // parameters to determine the correct minimum value of ResponseCommit.RetainHeight. + MinRetainBlocks uint64 `mapstructure:"min-retain-blocks"` + // InterBlockCache enables inter-block caching. InterBlockCache bool `mapstructure:"inter-block-cache"` @@ -150,6 +163,7 @@ func DefaultConfig() *Config { PruningKeepRecent: "0", PruningKeepEvery: "0", PruningInterval: "0", + MinRetainBlocks: 0, IndexEvents: make([]string, 0), }, Telemetry: telemetry.Config{ @@ -197,6 +211,7 @@ func GetConfig(v *viper.Viper) Config { HaltHeight: v.GetUint64("halt-height"), HaltTime: v.GetUint64("halt-time"), IndexEvents: v.GetStringSlice("index-events"), + MinRetainBlocks: v.GetUint64("min-retain-blocks"), }, Telemetry: telemetry.Config{ ServiceName: v.GetString("telemetry.service-name"), diff --git a/server/config/toml.go b/server/config/toml.go index 41497dc497c5..8f1876c81778 100644 --- a/server/config/toml.go +++ b/server/config/toml.go @@ -44,6 +44,19 @@ halt-height = {{ .BaseConfig.HaltHeight }} # Note: Commitment of state will be attempted on the corresponding block. halt-time = {{ .BaseConfig.HaltTime }} +# MinRetainBlocks defines the minimum block height offset from the current +# block being committed, such that all blocks past this offset are pruned +# from Tendermint. It is used as part of the process of determining the +# ResponseCommit.RetainHeight value during ABCI Commit. +# +# This configuration value is only responsible for pruning Tendermint blocks. +# It has no bearing on application state pruning which is determined by the +# "pruning-*" configurations. +# +# Note: This configuration can and should be used in conjunction with other +# parameters to determine the correct minimum value of ResponseCommit.RetainHeight. +min-retain-blocks = {{ .BaseConfig.MinRetainBlocks }} + # InterBlockCache enables inter-block caching. inter-block-cache = {{ .BaseConfig.InterBlockCache }} From d181cedf6d1e1e7501c4fc19c383c67d22bda20a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 8 Sep 2020 12:37:36 -0700 Subject: [PATCH 02/24] Add CLI flag hook --- server/start.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/start.go b/server/start.go index f98aab0ed926..8cdbc93c7af3 100644 --- a/server/start.go +++ b/server/start.go @@ -48,6 +48,7 @@ const ( FlagPruningKeepEvery = "pruning-keep-every" FlagPruningInterval = "pruning-interval" FlagIndexEvents = "index-events" + FlagMinRetainBlocks = "min-retain-blocks" ) // GRPC-related flags. @@ -135,6 +136,7 @@ which accepts a path for the resulting pprof file. cmd.Flags().Uint64(FlagPruningKeepEvery, 0, "Offset heights to keep on disk after 'keep-every' (ignored if pruning is not 'custom')") cmd.Flags().Uint64(FlagPruningInterval, 0, "Height interval at which pruned heights are removed from disk (ignored if pruning is not 'custom')") cmd.Flags().Uint(FlagInvCheckPeriod, 0, "Assert registered invariants every N blocks") + cmd.Flags().Uint64(FlagMinRetainBlocks, 0, "Minimum block height offset during ABCI commit to prune Tendermint blocks") cmd.Flags().Bool(flagGRPCEnable, true, "Define if the gRPC server should be enabled") cmd.Flags().String(flagGRPCAddress, config.DefaultGRPCAddress, "the gRPC server address to listen on") From 4e3bb9e04ee9a40fa8691526c49fe8415b250778 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 8 Sep 2020 12:39:03 -0700 Subject: [PATCH 03/24] update config doc --- server/config/config.go | 3 ++- server/config/toml.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/server/config/config.go b/server/config/config.go index 22f6d093c0cf..548ef8e9da0a 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -46,7 +46,8 @@ type BaseConfig struct { // MinRetainBlocks defines the minimum block height offset from the current // block being committed, such that all blocks past this offset are pruned // from Tendermint. It is used as part of the process of determining the - // ResponseCommit.RetainHeight value during ABCI Commit. + // ResponseCommit.RetainHeight value during ABCI Commit. A value of 0 indicates + // that no blocks should be pruned. // // This configuration value is only responsible for pruning Tendermint blocks. // It has no bearing on application state pruning which is determined by the diff --git a/server/config/toml.go b/server/config/toml.go index 8f1876c81778..fa37f3b4c8be 100644 --- a/server/config/toml.go +++ b/server/config/toml.go @@ -47,7 +47,8 @@ halt-time = {{ .BaseConfig.HaltTime }} # MinRetainBlocks defines the minimum block height offset from the current # block being committed, such that all blocks past this offset are pruned # from Tendermint. It is used as part of the process of determining the -# ResponseCommit.RetainHeight value during ABCI Commit. +# ResponseCommit.RetainHeight value during ABCI Commit. A value of 0 indicates +# that no blocks should be pruned. # # This configuration value is only responsible for pruning Tendermint blocks. # It has no bearing on application state pruning which is determined by the From b5f6d5b039072855d92017d416ee04cf6650594e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 8 Sep 2020 16:50:25 -0700 Subject: [PATCH 04/24] update baseapp opts --- baseapp/baseapp.go | 14 ++++++++++++++ baseapp/options.go | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 4fbe61177dd5..6fede113faad 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -101,6 +101,16 @@ type BaseApp struct { // nolint: maligned // minimum block time (in Unix seconds) at which to halt the chain and gracefully shutdown haltTime uint64 + // minRetainBlocksdefines the minimum block height offset from the current + // block being committed, such that all blocks past this offset are pruned + // from Tendermint. It is used as part of the process of determining the + // ResponseCommit.RetainHeight value during ABCI Commit. A value of 0 indicates + // that no blocks should be pruned. + // + // Note: This configuration can and should be used in conjunction with other + // parameters to determine the correct minimum value of ResponseCommit.RetainHeight. + minRetainBlocks uint64 + // application's version string appVersion string @@ -298,6 +308,10 @@ func (app *BaseApp) setHaltTime(haltTime uint64) { app.haltTime = haltTime } +func (app *BaseApp) setMinRetainBlocks(minRetainBlocks uint64) { + app.minRetainBlocks = minRetainBlocks +} + func (app *BaseApp) setInterBlockCache(cache sdk.MultiStorePersistentCache) { app.interBlockCache = cache } diff --git a/baseapp/options.go b/baseapp/options.go index 22483415fec7..026433ae5bed 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -39,6 +39,13 @@ func SetHaltTime(haltTime uint64) func(*BaseApp) { return func(bap *BaseApp) { bap.setHaltTime(haltTime) } } +// SetMinRetainBlocks returns a BaseApp option function that sets the minimum +// block retention height value when determining which heights to prune during +// ABCI Commit. +func SetMinRetainBlocks(minRetainBlocks uint64) func(*BaseApp) { + return func(bapp *BaseApp) { bapp.setMinRetainBlocks(minRetainBlocks) } +} + // SetTrace will turn on or off trace flag func SetTrace(trace bool) func(*BaseApp) { return func(app *BaseApp) { app.setTrace(trace) } From 7436c83e9e8b7d1e0af189ca1b83437aa4c0a7bd Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 8 Sep 2020 16:50:32 -0700 Subject: [PATCH 05/24] update simapp --- simapp/simd/cmd/root.go | 1 + 1 file changed, 1 insertion(+) diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go index 55d235941012..b01b1704566c 100644 --- a/simapp/simd/cmd/root.go +++ b/simapp/simd/cmd/root.go @@ -196,6 +196,7 @@ func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts serverty baseapp.SetMinGasPrices(cast.ToString(appOpts.Get(server.FlagMinGasPrices))), baseapp.SetHaltHeight(cast.ToUint64(appOpts.Get(server.FlagHaltHeight))), baseapp.SetHaltTime(cast.ToUint64(appOpts.Get(server.FlagHaltTime))), + baseapp.SetMinRetainBlocks(cast.ToUint64(appOpts.Get(server.FlagMinRetainBlocks))), baseapp.SetInterBlockCache(cache), baseapp.SetTrace(cast.ToBool(appOpts.Get(server.FlagTrace))), baseapp.SetIndexEvents(cast.ToStringSlice(appOpts.Get(server.FlagIndexEvents))), From a918473d87fc09237c52b08596d9f37a41c4186a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 8 Sep 2020 17:16:22 -0700 Subject: [PATCH 06/24] Implement GetBlockRentionHeight --- baseapp/abci.go | 53 ++++++++++++++++++++++++++++++++++++++++++++ store/types/store.go | 4 +++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 4a5b6335028e..8436e7672c2e 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -561,6 +561,59 @@ func (app *BaseApp) createQueryContext(height int64, prove bool) (sdk.Context, e return ctx, nil } +// GetBlockRentionHeight returns the height for which all blocks below this height +// are pruned from Tendermint. Given a commitment height and a non-zero local +// minRetainBlocks configuration, the retentionHeight is the smallest height that +// satisfies: +// +// - Unbonding (safety threshold) time: The block interval in which validators +// can be economically punished for misbehavior. Blocks in this interval must be +// auditable e.g. by the light client. +// +// - Logical store snapshot interval: The block interval at which the underlying +// logical store database is persisted to disk, e.g. every 10000 heights. Blocks +// since the last IAVL snapshot must be available for replay on application restart. +// +// - State sync snapshots: Blocks since the oldest available snapshot must be +// available for state sync nodes to catch up (oldest because a node may be +// restoring an old snapshot while a new snapshot was taken). +// +// - Local (minRetainBlocks) config: Archive nodes may want to retain more or +// all blocks, e.g. via a local config option min-retain-blocks. There may also +// be a need to vary retention for other nodes, e.g. sentry nodes which do not +// need historical blocks. +func (app *BaseApp) GetBlockRentionHeight(commitHeight uint64) uint64 { + min := func(x, y uint64) uint64 { + if x < y { + return x + } + return y + } + + // Define the number of blocks needed to protect against misbehaving validators + // which allows light clients to operate safely. Note, we piggy back of the + // evidence parameters instead of computing an estimated nubmer of blocks based + // on the unbonding period and block commitment time as the two should be + // equivalent. + blockSafetyThreshold := app.GetConsensusParams(app.deliverState.ctx).Evidence.MaxAgeNumBlocks + + // Define the state pruning interval, i.e. the block interval at which the + // underlying logical database is persisted to disk. + statePruningInterval := app.cms.GetPruning().Interval + + // Define retentionHeight as the minimum value that satisfies all constraints. + // All blocks past (commitHeight-retentionHeight) are pruned from Tendermint. + retentionHeight := min(app.minRetainBlocks, uint64(blockSafetyThreshold)) + retentionHeight = min(retentionHeight, statePruningInterval) + retentionHeight = min(retentionHeight, app.snapshotInterval) + + if retentionHeight == 0 || (retentionHeight >= commitHeight) { + return 0 + } + + return commitHeight - retentionHeight +} + func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery { if len(path) >= 2 { switch path[1] { diff --git a/store/types/store.go b/store/types/store.go index 2da36a5bd3ec..4e33b88f2ec5 100644 --- a/store/types/store.go +++ b/store/types/store.go @@ -4,10 +4,11 @@ import ( "fmt" "io" - snapshottypes "github.com/cosmos/cosmos-sdk/snapshots/types" abci "github.com/tendermint/tendermint/abci/types" dbm "github.com/tendermint/tm-db" + snapshottypes "github.com/cosmos/cosmos-sdk/snapshots/types" + "github.com/cosmos/cosmos-sdk/types/kv" ) @@ -23,6 +24,7 @@ type Committer interface { // TODO: Deprecate after 0.38.5 SetPruning(PruningOptions) + GetPruning() PruningOptions } // Stores of MultiStore must implement CommitStore. From ecec1da2e98486905073d1df6537dd3db9ea3aef Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Sep 2020 10:20:29 -0700 Subject: [PATCH 07/24] Modify GetBlockRentionHeight --- baseapp/abci.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 8436e7672c2e..70a49364ead2 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -590,28 +590,42 @@ func (app *BaseApp) GetBlockRentionHeight(commitHeight uint64) uint64 { return y } + // Define retentionHeight as the minimum value that satisfies all non-zero + // constraints. All blocks below (commitHeight-retentionHeight) are pruned + // from Tendermint. + var retentionHeight uint64 + // Define the number of blocks needed to protect against misbehaving validators // which allows light clients to operate safely. Note, we piggy back of the // evidence parameters instead of computing an estimated nubmer of blocks based // on the unbonding period and block commitment time as the two should be // equivalent. blockSafetyThreshold := app.GetConsensusParams(app.deliverState.ctx).Evidence.MaxAgeNumBlocks + if blockSafetyThreshold > 0 { + retentionHeight = commitHeight - uint64(blockSafetyThreshold) + } // Define the state pruning interval, i.e. the block interval at which the // underlying logical database is persisted to disk. statePruningInterval := app.cms.GetPruning().Interval + if statePruningInterval > 0 { + retentionHeight = min(retentionHeight, commitHeight-statePruningInterval) + } - // Define retentionHeight as the minimum value that satisfies all constraints. - // All blocks past (commitHeight-retentionHeight) are pruned from Tendermint. - retentionHeight := min(app.minRetainBlocks, uint64(blockSafetyThreshold)) - retentionHeight = min(retentionHeight, statePruningInterval) - retentionHeight = min(retentionHeight, app.snapshotInterval) + if app.snapshotInterval > 0 && app.snapshotKeepRecent > 0 { + retentionHeight = min(retentionHeight, commitHeight-(app.snapshotInterval*uint64(app.snapshotKeepRecent))) + } + + if app.minRetainBlocks > 0 { + retentionHeight = min(retentionHeight, commitHeight-app.minRetainBlocks) + } - if retentionHeight == 0 || (retentionHeight >= commitHeight) { + if retentionHeight <= 0 { + // prune nothing in the case of a non-positive height return 0 } - return commitHeight - retentionHeight + return retentionHeight } func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery { From 277e7cca70de228757eec6271d7ce34b69c34d4e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Sep 2020 12:12:47 -0700 Subject: [PATCH 08/24] Modify GetBlockRentionHeight --- baseapp/abci.go | 21 +++++++++++-- baseapp/abci_test.go | 61 ++++++++++++++++++++++++++++++++++++ store/iavl/store.go | 6 ++++ store/mem/store.go | 1 + store/rootmulti/dbadapter.go | 1 + store/transient/store.go | 5 ++- 6 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 baseapp/abci_test.go diff --git a/baseapp/abci.go b/baseapp/abci.go index 70a49364ead2..932e715f4f49 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -609,15 +609,30 @@ func (app *BaseApp) GetBlockRentionHeight(commitHeight uint64) uint64 { // underlying logical database is persisted to disk. statePruningInterval := app.cms.GetPruning().Interval if statePruningInterval > 0 { - retentionHeight = min(retentionHeight, commitHeight-statePruningInterval) + v := commitHeight - statePruningInterval + if retentionHeight == 0 { + retentionHeight = v + } else { + retentionHeight = min(retentionHeight, v) + } } if app.snapshotInterval > 0 && app.snapshotKeepRecent > 0 { - retentionHeight = min(retentionHeight, commitHeight-(app.snapshotInterval*uint64(app.snapshotKeepRecent))) + v := commitHeight - (app.snapshotInterval * uint64(app.snapshotKeepRecent)) + if retentionHeight == 0 { + retentionHeight = v + } else { + retentionHeight = min(retentionHeight, v) + } } if app.minRetainBlocks > 0 { - retentionHeight = min(retentionHeight, commitHeight-app.minRetainBlocks) + v := commitHeight - app.minRetainBlocks + if retentionHeight == 0 { + retentionHeight = v + } else { + retentionHeight = min(retentionHeight, v) + } } if retentionHeight <= 0 { diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go new file mode 100644 index 000000000000..998453c639ac --- /dev/null +++ b/baseapp/abci_test.go @@ -0,0 +1,61 @@ +package baseapp + +import ( + "testing" + + "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" + tmprototypes "github.com/tendermint/tendermint/proto/tendermint/types" + dbm "github.com/tendermint/tm-db" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +func TestGetBlockRentionHeight(t *testing.T) { + logger := defaultLogger() + db := dbm.NewMemDB() + name := t.Name() + + testCases := map[string]struct { + bapp *BaseApp + maxAgeBlocks int64 + commitHeight uint64 + expected uint64 + }{ + "no pruning": { + bapp: NewBaseApp(name, logger, db, nil), + maxAgeBlocks: 0, + commitHeight: 499000, + expected: 0, + }, + "pruning unbonding time only": { + bapp: NewBaseApp(name, logger, db, nil), + maxAgeBlocks: 362880, + commitHeight: 499000, + expected: 136120, + }, + "pruning iavl snapshot only": { + bapp: NewBaseApp(name, logger, db, nil, SetPruning(sdk.PruningOptions{Interval: 10000})), + maxAgeBlocks: 0, + commitHeight: 499000, + expected: 489000, + }, + } + + for name, tc := range testCases { + tc := tc + + tc.bapp.SetParamStore(¶mStore{db: dbm.NewMemDB()}) + tc.bapp.InitChain(abci.RequestInitChain{ + ConsensusParams: &abci.ConsensusParams{ + Evidence: &tmprototypes.EvidenceParams{ + MaxAgeNumBlocks: tc.maxAgeBlocks, + }, + }, + }) + + t.Run(name, func(t *testing.T) { + require.Equal(t, tc.expected, tc.bapp.GetBlockRentionHeight(tc.commitHeight)) + }) + } +} diff --git a/store/iavl/store.go b/store/iavl/store.go index 4e5f6c1faf73..3d6bedabdbbb 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -125,6 +125,12 @@ func (st *Store) SetPruning(_ types.PruningOptions) { panic("cannot set pruning options on an initialized IAVL store") } +// SetPruning panics as pruning options should be provided at initialization +// since IAVl accepts pruning options directly. +func (st *Store) GetPruning() types.PruningOptions { + panic("cannot get pruning options on an initialized IAVL store") +} + // VersionExists returns whether or not a given version is stored. func (st *Store) VersionExists(version int64) bool { return st.tree.VersionExists(version) diff --git a/store/mem/store.go b/store/mem/store.go index 156b17061ccb..92525d715d60 100644 --- a/store/mem/store.go +++ b/store/mem/store.go @@ -49,4 +49,5 @@ func (s Store) CacheWrapWithTrace(w io.Writer, tc types.TraceContext) types.Cach func (s *Store) Commit() (id types.CommitID) { return } func (s *Store) SetPruning(pruning types.PruningOptions) {} +func (s *Store) GetPruning() types.PruningOptions { return types.PruningOptions{} } func (s Store) LastCommitID() (id types.CommitID) { return } diff --git a/store/rootmulti/dbadapter.go b/store/rootmulti/dbadapter.go index cda6e62ba721..debdd5c6f64b 100644 --- a/store/rootmulti/dbadapter.go +++ b/store/rootmulti/dbadapter.go @@ -31,3 +31,4 @@ func (cdsa commitDBStoreAdapter) LastCommitID() types.CommitID { } func (cdsa commitDBStoreAdapter) SetPruning(_ types.PruningOptions) {} +func (cdsa commitDBStoreAdapter) GetPruning() types.PruningOptions { return types.PruningOptions{} } diff --git a/store/transient/store.go b/store/transient/store.go index e40e79ad6edd..b272427b114a 100644 --- a/store/transient/store.go +++ b/store/transient/store.go @@ -27,9 +27,8 @@ func (ts *Store) Commit() (id types.CommitID) { return } -// Implements CommitStore -func (ts *Store) SetPruning(pruning types.PruningOptions) { -} +func (ts *Store) SetPruning(_ types.PruningOptions) {} +func (ts *Store) GetPruning() types.PruningOptions { return types.PruningOptions{} } // Implements CommitStore func (ts *Store) LastCommitID() (id types.CommitID) { From 5a2ce1215043cfb5da6edc7a8303588f6bb465e9 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Sep 2020 12:18:16 -0700 Subject: [PATCH 09/24] update tests --- baseapp/abci_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 998453c639ac..4da6c07aa67a 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -40,6 +40,29 @@ func TestGetBlockRentionHeight(t *testing.T) { commitHeight: 499000, expected: 489000, }, + "pruning state sync snapshot only": { + bapp: NewBaseApp(name, logger, db, nil, SetSnapshotInterval(50000), SetSnapshotKeepRecent(3)), + maxAgeBlocks: 0, + commitHeight: 499000, + expected: 349000, + }, + "pruning min retention only": { + bapp: NewBaseApp(name, logger, db, nil, SetMinRetainBlocks(400000)), + maxAgeBlocks: 0, + commitHeight: 499000, + expected: 99000, + }, + "pruning all conditions": { + bapp: NewBaseApp( + name, logger, db, nil, + SetPruning(sdk.PruningOptions{Interval: 10000}), + SetMinRetainBlocks(400000), + SetSnapshotInterval(50000), SetSnapshotKeepRecent(3), + ), + maxAgeBlocks: 362880, + commitHeight: 499000, + expected: 99000, + }, } for name, tc := range testCases { From a019645ac7c8c1800ec231fb469d8ec4cca6635f Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Sep 2020 12:20:00 -0700 Subject: [PATCH 10/24] Update ABCI Commit --- baseapp/abci.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 932e715f4f49..acae1dee8ae4 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -317,7 +317,8 @@ func (app *BaseApp) Commit() (res abci.ResponseCommit) { } return abci.ResponseCommit{ - Data: commitID.Hash, + Data: commitID.Hash, + RetainHeight: int64(app.GetBlockRentionHeight(uint64(header.Height))), } } From d0fe2649ac0db49a329d92d42282c0508786a1d9 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Sep 2020 13:00:27 -0700 Subject: [PATCH 11/24] use int64 --- baseapp/abci.go | 21 +++++++++++---------- baseapp/abci_test.go | 4 ++-- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index acae1dee8ae4..2c02a5450ada 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -277,6 +277,7 @@ func (app *BaseApp) Commit() (res abci.ResponseCommit) { defer telemetry.MeasureSince(time.Now(), "abci", "commit") header := app.deliverState.ctx.BlockHeader() + retainHeight := app.GetBlockRentionHeight(header.Height) // Write the DeliverTx state which is cache-wrapped and commit the MultiStore. // The write to the DeliverTx state writes all state transitions to the root @@ -318,7 +319,7 @@ func (app *BaseApp) Commit() (res abci.ResponseCommit) { return abci.ResponseCommit{ Data: commitID.Hash, - RetainHeight: int64(app.GetBlockRentionHeight(uint64(header.Height))), + RetainHeight: retainHeight, } } @@ -583,8 +584,8 @@ func (app *BaseApp) createQueryContext(height int64, prove bool) (sdk.Context, e // all blocks, e.g. via a local config option min-retain-blocks. There may also // be a need to vary retention for other nodes, e.g. sentry nodes which do not // need historical blocks. -func (app *BaseApp) GetBlockRentionHeight(commitHeight uint64) uint64 { - min := func(x, y uint64) uint64 { +func (app *BaseApp) GetBlockRentionHeight(commitHeight int64) int64 { + min := func(x, y int64) int64 { if x < y { return x } @@ -594,21 +595,21 @@ func (app *BaseApp) GetBlockRentionHeight(commitHeight uint64) uint64 { // Define retentionHeight as the minimum value that satisfies all non-zero // constraints. All blocks below (commitHeight-retentionHeight) are pruned // from Tendermint. - var retentionHeight uint64 + var retentionHeight int64 // Define the number of blocks needed to protect against misbehaving validators // which allows light clients to operate safely. Note, we piggy back of the // evidence parameters instead of computing an estimated nubmer of blocks based // on the unbonding period and block commitment time as the two should be // equivalent. - blockSafetyThreshold := app.GetConsensusParams(app.deliverState.ctx).Evidence.MaxAgeNumBlocks - if blockSafetyThreshold > 0 { - retentionHeight = commitHeight - uint64(blockSafetyThreshold) + cp := app.GetConsensusParams(app.deliverState.ctx) + if cp != nil && cp.Evidence != nil && cp.Evidence.MaxAgeNumBlocks > 0 { + retentionHeight = commitHeight - cp.Evidence.MaxAgeNumBlocks } // Define the state pruning interval, i.e. the block interval at which the // underlying logical database is persisted to disk. - statePruningInterval := app.cms.GetPruning().Interval + statePruningInterval := int64(app.cms.GetPruning().Interval) if statePruningInterval > 0 { v := commitHeight - statePruningInterval if retentionHeight == 0 { @@ -619,7 +620,7 @@ func (app *BaseApp) GetBlockRentionHeight(commitHeight uint64) uint64 { } if app.snapshotInterval > 0 && app.snapshotKeepRecent > 0 { - v := commitHeight - (app.snapshotInterval * uint64(app.snapshotKeepRecent)) + v := commitHeight - int64((app.snapshotInterval * uint64(app.snapshotKeepRecent))) if retentionHeight == 0 { retentionHeight = v } else { @@ -628,7 +629,7 @@ func (app *BaseApp) GetBlockRentionHeight(commitHeight uint64) uint64 { } if app.minRetainBlocks > 0 { - v := commitHeight - app.minRetainBlocks + v := commitHeight - int64(app.minRetainBlocks) if retentionHeight == 0 { retentionHeight = v } else { diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 4da6c07aa67a..5d986488b3e2 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -19,8 +19,8 @@ func TestGetBlockRentionHeight(t *testing.T) { testCases := map[string]struct { bapp *BaseApp maxAgeBlocks int64 - commitHeight uint64 - expected uint64 + commitHeight int64 + expected int64 }{ "no pruning": { bapp: NewBaseApp(name, logger, db, nil), From a2d008e3040b75f04684093c3d616a7d15feadad Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Sep 2020 13:03:15 -0700 Subject: [PATCH 12/24] cl++ --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d621cd60e482..76ec57e994f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,7 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa ### Features +* [\#7265](https://github.com/cosmos/cosmos-sdk/pull/7265) Support Tendermint block pruning through a new `min-retain-blocks` configuration that can be set in either `app.toml` or via the CLI. This parameter is used in conjunction with other criteria to determine the height at which Tendermint should prune blocks. * (vesting) [\#7209](https://github.com/cosmos/cosmos-sdk/pull/7209) Create new `MsgCreateVestingAccount` message type along with CLI handler that allows for the creation of delayed and continuous vesting types. * (events) [\#7121](https://github.com/cosmos/cosmos-sdk/pull/7121) The application now drives what events are indexed by Tendermint via the `index-events` configuration in `app.toml`, which is a list of events taking the form `{eventType}.{attributeKey}`. * [\#6089](https://github.com/cosmos/cosmos-sdk/pull/6089) Transactions can now have a `TimeoutHeight` set which allows the transaction to be rejected if it's committed at a height greater than the timeout. From cddfc90da1e5c3b534fb1c345a881dbea2b8349b Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Sep 2020 13:10:57 -0700 Subject: [PATCH 13/24] add zero check --- baseapp/abci.go | 17 ++++++++++------- baseapp/abci_test.go | 33 ++++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 2c02a5450ada..65b5594f3ba8 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -585,6 +585,11 @@ func (app *BaseApp) createQueryContext(height int64, prove bool) (sdk.Context, e // be a need to vary retention for other nodes, e.g. sentry nodes which do not // need historical blocks. func (app *BaseApp) GetBlockRentionHeight(commitHeight int64) int64 { + // pruning is disabled if minRetainBlocks is zero + if app.minRetainBlocks == 0 { + return 0 + } + min := func(x, y int64) int64 { if x < y { return x @@ -628,13 +633,11 @@ func (app *BaseApp) GetBlockRentionHeight(commitHeight int64) int64 { } } - if app.minRetainBlocks > 0 { - v := commitHeight - int64(app.minRetainBlocks) - if retentionHeight == 0 { - retentionHeight = v - } else { - retentionHeight = min(retentionHeight, v) - } + v := commitHeight - int64(app.minRetainBlocks) + if retentionHeight == 0 { + retentionHeight = v + } else { + retentionHeight = min(retentionHeight, v) } if retentionHeight <= 0 { diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 5d986488b3e2..742b03ad1760 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -22,32 +22,44 @@ func TestGetBlockRentionHeight(t *testing.T) { commitHeight int64 expected int64 }{ - "no pruning": { + "defaults": { bapp: NewBaseApp(name, logger, db, nil), maxAgeBlocks: 0, commitHeight: 499000, expected: 0, }, "pruning unbonding time only": { - bapp: NewBaseApp(name, logger, db, nil), + bapp: NewBaseApp(name, logger, db, nil, SetMinRetainBlocks(1)), maxAgeBlocks: 362880, commitHeight: 499000, expected: 136120, }, "pruning iavl snapshot only": { - bapp: NewBaseApp(name, logger, db, nil, SetPruning(sdk.PruningOptions{Interval: 10000})), + bapp: NewBaseApp( + name, logger, db, nil, + SetPruning(sdk.PruningOptions{Interval: 10000}), + SetMinRetainBlocks(1), + ), maxAgeBlocks: 0, commitHeight: 499000, expected: 489000, }, "pruning state sync snapshot only": { - bapp: NewBaseApp(name, logger, db, nil, SetSnapshotInterval(50000), SetSnapshotKeepRecent(3)), + bapp: NewBaseApp( + name, logger, db, nil, + SetSnapshotInterval(50000), + SetSnapshotKeepRecent(3), + SetMinRetainBlocks(1), + ), maxAgeBlocks: 0, commitHeight: 499000, expected: 349000, }, "pruning min retention only": { - bapp: NewBaseApp(name, logger, db, nil, SetMinRetainBlocks(400000)), + bapp: NewBaseApp( + name, logger, db, nil, + SetMinRetainBlocks(400000), + ), maxAgeBlocks: 0, commitHeight: 499000, expected: 99000, @@ -63,6 +75,17 @@ func TestGetBlockRentionHeight(t *testing.T) { commitHeight: 499000, expected: 99000, }, + "disable pruning": { + bapp: NewBaseApp( + name, logger, db, nil, + SetPruning(sdk.PruningOptions{Interval: 10000}), + SetMinRetainBlocks(0), + SetSnapshotInterval(50000), SetSnapshotKeepRecent(3), + ), + maxAgeBlocks: 362880, + commitHeight: 499000, + expected: 0, + }, } for name, tc := range testCases { From 0d488399b4c51ce70ad5c1e12850bc0b6671e8e3 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Sep 2020 20:13:50 -0700 Subject: [PATCH 14/24] fix build --- server/mock/store.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/mock/store.go b/server/mock/store.go index 9e0e05e46722..d731767f192a 100644 --- a/server/mock/store.go +++ b/server/mock/store.go @@ -55,6 +55,10 @@ func (ms multiStore) SetPruning(opts sdk.PruningOptions) { panic("not implemented") } +func (ms multiStore) GetPruning() sdk.PruningOptions { + panic("not implemented") +} + func (ms multiStore) GetCommitKVStore(key sdk.StoreKey) sdk.CommitKVStore { panic("not implemented") } From 53f84789907c2171b2eee0af32bc8750dacb25e0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 10 Sep 2020 13:19:36 -0400 Subject: [PATCH 15/24] update godocs --- store/mem/store.go | 8 ++++++-- store/rootmulti/dbadapter.go | 5 ++++- store/transient/store.go | 5 ++++- store/types/store.go | 1 - 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/store/mem/store.go b/store/mem/store.go index 92525d715d60..4657a01140be 100644 --- a/store/mem/store.go +++ b/store/mem/store.go @@ -49,5 +49,9 @@ func (s Store) CacheWrapWithTrace(w io.Writer, tc types.TraceContext) types.Cach func (s *Store) Commit() (id types.CommitID) { return } func (s *Store) SetPruning(pruning types.PruningOptions) {} -func (s *Store) GetPruning() types.PruningOptions { return types.PruningOptions{} } -func (s Store) LastCommitID() (id types.CommitID) { return } + +// GetPruning is a no-op as pruning options cannot be directly set on this store. +// They must be set on the root commit multi-store. +func (s *Store) GetPruning() types.PruningOptions { return types.PruningOptions{} } + +func (s Store) LastCommitID() (id types.CommitID) { return } diff --git a/store/rootmulti/dbadapter.go b/store/rootmulti/dbadapter.go index debdd5c6f64b..4d6e5afeb875 100644 --- a/store/rootmulti/dbadapter.go +++ b/store/rootmulti/dbadapter.go @@ -31,4 +31,7 @@ func (cdsa commitDBStoreAdapter) LastCommitID() types.CommitID { } func (cdsa commitDBStoreAdapter) SetPruning(_ types.PruningOptions) {} -func (cdsa commitDBStoreAdapter) GetPruning() types.PruningOptions { return types.PruningOptions{} } + +// GetPruning is a no-op as pruning options cannot be directly set on this store. +// They must be set on the root commit multi-store. +func (cdsa commitDBStoreAdapter) GetPruning() types.PruningOptions { return types.PruningOptions{} } diff --git a/store/transient/store.go b/store/transient/store.go index b272427b114a..572ab55f7697 100644 --- a/store/transient/store.go +++ b/store/transient/store.go @@ -28,7 +28,10 @@ func (ts *Store) Commit() (id types.CommitID) { } func (ts *Store) SetPruning(_ types.PruningOptions) {} -func (ts *Store) GetPruning() types.PruningOptions { return types.PruningOptions{} } + +// GetPruning is a no-op as pruning options cannot be directly set on this store. +// They must be set on the root commit multi-store. +func (ts *Store) GetPruning() types.PruningOptions { return types.PruningOptions{} } // Implements CommitStore func (ts *Store) LastCommitID() (id types.CommitID) { diff --git a/store/types/store.go b/store/types/store.go index 4e33b88f2ec5..94ec7f06fdd9 100644 --- a/store/types/store.go +++ b/store/types/store.go @@ -22,7 +22,6 @@ type Committer interface { Commit() CommitID LastCommitID() CommitID - // TODO: Deprecate after 0.38.5 SetPruning(PruningOptions) GetPruning() PruningOptions } From a2a9ab6908dacaaf88ce8aec6bf2edb983fe9f3e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 10 Sep 2020 13:20:15 -0400 Subject: [PATCH 16/24] lint++ --- store/types/store.go | 1 - 1 file changed, 1 deletion(-) diff --git a/store/types/store.go b/store/types/store.go index 94ec7f06fdd9..989ae94e8c42 100644 --- a/store/types/store.go +++ b/store/types/store.go @@ -8,7 +8,6 @@ import ( dbm "github.com/tendermint/tm-db" snapshottypes "github.com/cosmos/cosmos-sdk/snapshots/types" - "github.com/cosmos/cosmos-sdk/types/kv" ) From 451fcb332ecb8e42a5ec693fcdf31f8f0247fa03 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 11 Sep 2020 05:31:17 -0700 Subject: [PATCH 17/24] Update baseapp/abci.go Co-authored-by: Erik Grinaker --- baseapp/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 65b5594f3ba8..1d5795100c00 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -584,7 +584,7 @@ func (app *BaseApp) createQueryContext(height int64, prove bool) (sdk.Context, e // all blocks, e.g. via a local config option min-retain-blocks. There may also // be a need to vary retention for other nodes, e.g. sentry nodes which do not // need historical blocks. -func (app *BaseApp) GetBlockRentionHeight(commitHeight int64) int64 { +func (app *BaseApp) GetBlockRetentionHeight(commitHeight int64) int64 { // pruning is disabled if minRetainBlocks is zero if app.minRetainBlocks == 0 { return 0 From 9e766c1e9bbe56e88a8ad51a992d94c69a207ad8 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 11 Sep 2020 05:31:40 -0700 Subject: [PATCH 18/24] Update baseapp/baseapp.go Co-authored-by: Erik Grinaker --- baseapp/baseapp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 6fede113faad..b2e86a0fe12c 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -101,7 +101,7 @@ type BaseApp struct { // nolint: maligned // minimum block time (in Unix seconds) at which to halt the chain and gracefully shutdown haltTime uint64 - // minRetainBlocksdefines the minimum block height offset from the current + // minRetainBlocks defines the minimum block height offset from the current // block being committed, such that all blocks past this offset are pruned // from Tendermint. It is used as part of the process of determining the // ResponseCommit.RetainHeight value during ABCI Commit. A value of 0 indicates From f3f91706808573ccd2067a1e39054d78128c90b0 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Fri, 11 Sep 2020 05:32:12 -0700 Subject: [PATCH 19/24] Update server/config/config.go Co-authored-by: Erik Grinaker --- server/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/config/config.go b/server/config/config.go index 548ef8e9da0a..71d83b862244 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -44,7 +44,7 @@ type BaseConfig struct { HaltTime uint64 `mapstructure:"halt-time"` // MinRetainBlocks defines the minimum block height offset from the current - // block being committed, such that all blocks past this offset are pruned + // block being committed, such that blocks past this offset may be pruned // from Tendermint. It is used as part of the process of determining the // ResponseCommit.RetainHeight value during ABCI Commit. A value of 0 indicates // that no blocks should be pruned. From 7a94b117ca27c6bd38603ca783de2bde057b6d36 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 11 Sep 2020 09:59:01 -0400 Subject: [PATCH 20/24] fix build & refactor min logic --- baseapp/abci.go | 34 ++++++++++++++-------------------- baseapp/abci_test.go | 2 +- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 1d5795100c00..51a7c99572cf 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -277,7 +277,7 @@ func (app *BaseApp) Commit() (res abci.ResponseCommit) { defer telemetry.MeasureSince(time.Now(), "abci", "commit") header := app.deliverState.ctx.BlockHeader() - retainHeight := app.GetBlockRentionHeight(header.Height) + retainHeight := app.GetBlockRetentionHeight(header.Height) // Write the DeliverTx state which is cache-wrapped and commit the MultiStore. // The write to the DeliverTx state writes all state transitions to the root @@ -563,7 +563,7 @@ func (app *BaseApp) createQueryContext(height int64, prove bool) (sdk.Context, e return ctx, nil } -// GetBlockRentionHeight returns the height for which all blocks below this height +// GetBlockRetentionHeight returns the height for which all blocks below this height // are pruned from Tendermint. Given a commitment height and a non-zero local // minRetainBlocks configuration, the retentionHeight is the smallest height that // satisfies: @@ -590,11 +590,17 @@ func (app *BaseApp) GetBlockRetentionHeight(commitHeight int64) int64 { return 0 } - min := func(x, y int64) int64 { - if x < y { + minNonZero := func(x, y int64) int64 { + switch { + case x == 0: + return y + case y == 0: return x + case x < y: + return x + default: + return y } - return y } // Define retentionHeight as the minimum value that satisfies all non-zero @@ -617,28 +623,16 @@ func (app *BaseApp) GetBlockRetentionHeight(commitHeight int64) int64 { statePruningInterval := int64(app.cms.GetPruning().Interval) if statePruningInterval > 0 { v := commitHeight - statePruningInterval - if retentionHeight == 0 { - retentionHeight = v - } else { - retentionHeight = min(retentionHeight, v) - } + retentionHeight = minNonZero(retentionHeight, v) } if app.snapshotInterval > 0 && app.snapshotKeepRecent > 0 { v := commitHeight - int64((app.snapshotInterval * uint64(app.snapshotKeepRecent))) - if retentionHeight == 0 { - retentionHeight = v - } else { - retentionHeight = min(retentionHeight, v) - } + retentionHeight = minNonZero(retentionHeight, v) } v := commitHeight - int64(app.minRetainBlocks) - if retentionHeight == 0 { - retentionHeight = v - } else { - retentionHeight = min(retentionHeight, v) - } + retentionHeight = minNonZero(retentionHeight, v) if retentionHeight <= 0 { // prune nothing in the case of a non-positive height diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 742b03ad1760..578357f049cc 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -101,7 +101,7 @@ func TestGetBlockRentionHeight(t *testing.T) { }) t.Run(name, func(t *testing.T) { - require.Equal(t, tc.expected, tc.bapp.GetBlockRentionHeight(tc.commitHeight)) + require.Equal(t, tc.expected, tc.bapp.GetBlockRetentionHeight(tc.commitHeight)) }) } } From 2c3d62ab1f302ca221795c7fa6903b47e5f33859 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 11 Sep 2020 10:03:15 -0400 Subject: [PATCH 21/24] godoc++ --- server/config/config.go | 6 ++++-- server/config/toml.go | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/server/config/config.go b/server/config/config.go index 71d83b862244..72b8ed73bb3f 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -53,8 +53,10 @@ type BaseConfig struct { // It has no bearing on application state pruning which is determined by the // "pruning-*" configurations. // - // Note: This configuration can and should be used in conjunction with other - // parameters to determine the correct minimum value of ResponseCommit.RetainHeight. + // Note: Tendermint block pruning is dependant on this parameter in conunction + // with the unbonding (safety threshold) period, state pruning and state sync + // snapshot parameters to determine the correct minimum value of + // ResponseCommit.RetainHeight. MinRetainBlocks uint64 `mapstructure:"min-retain-blocks"` // InterBlockCache enables inter-block caching. diff --git a/server/config/toml.go b/server/config/toml.go index fa37f3b4c8be..b042da74293d 100644 --- a/server/config/toml.go +++ b/server/config/toml.go @@ -54,8 +54,10 @@ halt-time = {{ .BaseConfig.HaltTime }} # It has no bearing on application state pruning which is determined by the # "pruning-*" configurations. # -# Note: This configuration can and should be used in conjunction with other -# parameters to determine the correct minimum value of ResponseCommit.RetainHeight. +# Note: Tendermint block pruning is dependant on this parameter in conunction +# with the unbonding (safety threshold) period, state pruning and state sync +# snapshot parameters to determine the correct minimum value of +# ResponseCommit.RetainHeight. min-retain-blocks = {{ .BaseConfig.MinRetainBlocks }} # InterBlockCache enables inter-block caching. From 94c2a0360364078a2b19d1c5ca91e24a00ae30fc Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 11 Sep 2020 14:57:34 -0400 Subject: [PATCH 22/24] adjustments --- baseapp/abci.go | 22 ++++++++++++++++------ baseapp/abci_test.go | 19 +++++++++++++++---- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 51a7c99572cf..8a163506b34f 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -618,12 +618,22 @@ func (app *BaseApp) GetBlockRetentionHeight(commitHeight int64) int64 { retentionHeight = commitHeight - cp.Evidence.MaxAgeNumBlocks } - // Define the state pruning interval, i.e. the block interval at which the - // underlying logical database is persisted to disk. - statePruningInterval := int64(app.cms.GetPruning().Interval) - if statePruningInterval > 0 { - v := commitHeight - statePruningInterval - retentionHeight = minNonZero(retentionHeight, v) + // Define the state pruning offset, i.e. the block offset at which the + // underlying logical database is persisted to disk. Note, we only consider + // this case when the offset is greater than the current height, otherwise it + // would yield 0 and + statePruningOffset := int64(app.cms.GetPruning().KeepEvery) + if statePruningOffset > 0 { + if commitHeight > statePruningOffset { + v := commitHeight - (commitHeight % statePruningOffset) + retentionHeight = minNonZero(retentionHeight, v) + } else { + // Hitting this case means we have persisting enabled but have yet to reach + // a height in which we persist state, so we return zero regardless of other + // conditions. Otherwise, we could end up pruning blocks without having + // any state committed to disk. + return 0 + } } if app.snapshotInterval > 0 && app.snapshotKeepRecent > 0 { diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 578357f049cc..33735140e29f 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -37,12 +37,12 @@ func TestGetBlockRentionHeight(t *testing.T) { "pruning iavl snapshot only": { bapp: NewBaseApp( name, logger, db, nil, - SetPruning(sdk.PruningOptions{Interval: 10000}), + SetPruning(sdk.PruningOptions{KeepEvery: 10000}), SetMinRetainBlocks(1), ), maxAgeBlocks: 0, commitHeight: 499000, - expected: 489000, + expected: 490000, }, "pruning state sync snapshot only": { bapp: NewBaseApp( @@ -67,7 +67,7 @@ func TestGetBlockRentionHeight(t *testing.T) { "pruning all conditions": { bapp: NewBaseApp( name, logger, db, nil, - SetPruning(sdk.PruningOptions{Interval: 10000}), + SetPruning(sdk.PruningOptions{KeepEvery: 10000}), SetMinRetainBlocks(400000), SetSnapshotInterval(50000), SetSnapshotKeepRecent(3), ), @@ -75,10 +75,21 @@ func TestGetBlockRentionHeight(t *testing.T) { commitHeight: 499000, expected: 99000, }, + "no pruning due to no persisted state": { + bapp: NewBaseApp( + name, logger, db, nil, + SetPruning(sdk.PruningOptions{KeepEvery: 10000}), + SetMinRetainBlocks(400000), + SetSnapshotInterval(50000), SetSnapshotKeepRecent(3), + ), + maxAgeBlocks: 362880, + commitHeight: 10000, + expected: 0, + }, "disable pruning": { bapp: NewBaseApp( name, logger, db, nil, - SetPruning(sdk.PruningOptions{Interval: 10000}), + SetPruning(sdk.PruningOptions{KeepEvery: 10000}), SetMinRetainBlocks(0), SetSnapshotInterval(50000), SetSnapshotKeepRecent(3), ), From b3821ef92bc26fe071341120d3b66680eaee8c3c Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 11 Sep 2020 14:58:22 -0400 Subject: [PATCH 23/24] godoc++ --- baseapp/abci.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 8a163506b34f..f50083acd146 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -619,9 +619,7 @@ func (app *BaseApp) GetBlockRetentionHeight(commitHeight int64) int64 { } // Define the state pruning offset, i.e. the block offset at which the - // underlying logical database is persisted to disk. Note, we only consider - // this case when the offset is greater than the current height, otherwise it - // would yield 0 and + // underlying logical database is persisted to disk. statePruningOffset := int64(app.cms.GetPruning().KeepEvery) if statePruningOffset > 0 { if commitHeight > statePruningOffset { From 21c54297a4a385705f82aaff82ed68b5e084c10e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 11 Sep 2020 14:59:44 -0400 Subject: [PATCH 24/24] godoc++ --- baseapp/baseapp.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index b2e86a0fe12c..23a9d1dd8d56 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -107,8 +107,10 @@ type BaseApp struct { // nolint: maligned // ResponseCommit.RetainHeight value during ABCI Commit. A value of 0 indicates // that no blocks should be pruned. // - // Note: This configuration can and should be used in conjunction with other - // parameters to determine the correct minimum value of ResponseCommit.RetainHeight. + // Note: Tendermint block pruning is dependant on this parameter in conunction + // with the unbonding (safety threshold) period, state pruning and state sync + // snapshot parameters to determine the correct minimum value of + // ResponseCommit.RetainHeight. minRetainBlocks uint64 // application's version string