Skip to content

Commit

Permalink
perf: exclude pruning from tendermint update client in ante handler e…
Browse files Browse the repository at this point in the history
…xecution (#6278)

* performance: exit early on recvpacket to exclude app callbacks

* perf: skip pruning on check tx and recheck tx

* change fmt.Errorf to errors.New

* fix: check execMode simulate in conditional

* revert: recv packet changes

* fix: account for simulation in pruning check

* fix: reuse checkTx ctx

* chore: add changelog

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
(cherry picked from commit 67b23cd)

# Conflicts:
#	modules/light-clients/07-tendermint/update_test.go
  • Loading branch information
colin-axner authored and mergify[bot] committed May 20, 2024
1 parent f6fe145 commit 010bfc6
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (apps/27-interchain-accounts, apps/tranfer, apps/29-fee) [\#6253](https://github.com/cosmos/ibc-go/pull/6253) Allow channel handshake to succeed if fee middleware is wired up on one side, but not the other.
* (apps/27-interchain-accounts) [\#6251](https://github.com/cosmos/ibc-go/pull/6251) Use `UNORDERED` as the default ordering for new ICA channels.
* (apps/transfer) [\#6268](https://github.com/cosmos/ibc-go/pull/6268) Use memo strings instead of JSON keys in `AllowedPacketData` of transfer authorization.
* (core/ante) [\#6278](https://github.com/cosmos/ibc-go/pull/6278) Performance: Exclude pruning from tendermint client updates in ante handler executions.

### Features

Expand Down
6 changes: 5 additions & 1 deletion modules/light-clients/07-tendermint/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,11 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client
return []exported.Height{}
}

cs.pruneOldestConsensusState(ctx, cdc, clientStore)
// performance: do not prune in checkTx
// simulation must prune for accurate gas estimation
if (!ctx.IsCheckTx() && !ctx.IsReCheckTx()) || ctx.ExecMode() == sdk.ExecModeSimulate {
cs.pruneOldestConsensusState(ctx, cdc, clientStore)
}

// check for duplicate update
if _, found := GetConsensusState(clientStore, cdc, header.GetHeight()); found {
Expand Down
78 changes: 78 additions & 0 deletions modules/light-clients/07-tendermint/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ import (

storetypes "cosmossdk.io/store/types"

<<<<<<< HEAD

Check failure on line 8 in modules/light-clients/07-tendermint/update_test.go

View workflow job for this annotation

GitHub Actions / build (arm64)

missing import path

Check failure on line 8 in modules/light-clients/07-tendermint/update_test.go

View workflow job for this annotation

GitHub Actions / build (amd64)

missing import path

Check failure on line 8 in modules/light-clients/07-tendermint/update_test.go

View workflow job for this annotation

GitHub Actions / build (arm)

missing import path

Check failure on line 8 in modules/light-clients/07-tendermint/update_test.go

View workflow job for this annotation

GitHub Actions / lint

missing import path (typecheck)

Check failure on line 8 in modules/light-clients/07-tendermint/update_test.go

View workflow job for this annotation

GitHub Actions / lint

missing import path (typecheck)
tmtypes "github.com/cometbft/cometbft/types"
=======
sdk "github.com/cosmos/cosmos-sdk/types"

cmttypes "github.com/cometbft/cometbft/types"
>>>>>>> 67b23cd8 (perf: exclude pruning from tendermint update client in ante handler execution (#6278))

clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types"
Expand Down Expand Up @@ -523,6 +529,78 @@ func (suite *TendermintTestSuite) TestUpdateState() {
}
}

func (suite *TendermintTestSuite) TestUpdateStateCheckTx() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
path.SetupClients()

createClientMessage := func() exported.ClientMessage {
trustedHeight, ok := path.EndpointA.GetClientLatestHeight().(clienttypes.Height)
suite.Require().True(ok)
header, err := path.EndpointB.Chain.IBCClientHeader(path.EndpointB.Chain.LatestCommittedHeader, trustedHeight)
suite.Require().NoError(err)
return header
}

// get the first height as it will be pruned first.
var pruneHeight exported.Height
getFirstHeightCb := func(height exported.Height) bool {
pruneHeight = height
return true
}
ctx := path.EndpointA.Chain.GetContext()
clientStore := path.EndpointA.Chain.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID)
ibctm.IterateConsensusStateAscending(clientStore, getFirstHeightCb)

// Increment the time by a week
suite.coordinator.IncrementTimeBy(7 * 24 * time.Hour)

lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.Route(path.EndpointA.ClientID)
suite.Require().True(found)

ctx = path.EndpointA.Chain.GetContext().WithIsCheckTx(true)
lightClientModule.UpdateState(ctx, path.EndpointA.ClientID, createClientMessage())

// Increment the time by another week, then update the client.
// This will cause the first two consensus states to become expired.
suite.coordinator.IncrementTimeBy(7 * 24 * time.Hour)
ctx = path.EndpointA.Chain.GetContext().WithIsCheckTx(true)
lightClientModule.UpdateState(ctx, path.EndpointA.ClientID, createClientMessage())

assertPrune := func(pruned bool) {
// check consensus states and associated metadata
consState, ok := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight)
suite.Require().Equal(!pruned, ok)

processTime, ok := ibctm.GetProcessedTime(clientStore, pruneHeight)
suite.Require().Equal(!pruned, ok)

processHeight, ok := ibctm.GetProcessedHeight(clientStore, pruneHeight)
suite.Require().Equal(!pruned, ok)

consKey := ibctm.GetIterationKey(clientStore, pruneHeight)

if pruned {
suite.Require().Nil(consState, "expired consensus state not pruned")
suite.Require().Empty(processTime, "processed time metadata not pruned")
suite.Require().Nil(processHeight, "processed height metadata not pruned")
suite.Require().Nil(consKey, "iteration key not pruned")
} else {
suite.Require().NotNil(consState, "expired consensus state pruned")
suite.Require().NotEqual(uint64(0), processTime, "processed time metadata pruned")
suite.Require().NotNil(processHeight, "processed height metadata pruned")
suite.Require().NotNil(consKey, "iteration key pruned")
}
}

assertPrune(false)

// simulation mode must prune to calculate gas correctly
ctx = ctx.WithExecMode(sdk.ExecModeSimulate)
lightClientModule.UpdateState(ctx, path.EndpointA.ClientID, createClientMessage())

assertPrune(true)
}

func (suite *TendermintTestSuite) TestPruneConsensusState() {
// create path and setup clients
path := ibctesting.NewPath(suite.chainA, suite.chainB)
Expand Down

0 comments on commit 010bfc6

Please sign in to comment.