Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow scaling of trusting period for client upgrades #8185

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions modules/light-clients/07-tendermint/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package tendermint

import (
"fmt"
"time"

errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
upgradetypes "cosmossdk.io/x/upgrade/types"

Expand All @@ -17,8 +19,10 @@ import (
)

// VerifyUpgradeAndUpdateState checks if the upgraded client has been committed by the current client
// It will zero out all client-specific fields (e.g. TrustingPeriod) and verify all data
// in client state that must be the same across all valid Tendermint clients for the new chain.
// It will zero out all client-specific fields and verify all data in client state that must
// be the same across all valid Tendermint clients for the new chain.
// Note, if there is a decrease in the UnbondingPeriod, then the TrustingPeriod, despite being a client-specific field
// is scaled down by the same ratio.
// VerifyUpgrade will return an error if:
// - the upgradedClient is not a Tendermint ClientState
// - the latest height of the client state does not have the same revision number or has a greater
Expand Down Expand Up @@ -93,12 +97,17 @@ func (cs ClientState) VerifyUpgradeAndUpdateState(
return errorsmod.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.GetKeyPath())
}

trustingPeriod := cs.TrustingPeriod
if tmUpgradeClient.UnbondingPeriod < cs.UnbondingPeriod {
trustingPeriod = calculateNewTrustingPeriod(trustingPeriod, cs.UnbondingPeriod, tmUpgradeClient.UnbondingPeriod)
}

// Construct new client state and consensus state
// Relayer chosen client parameters are ignored.
// All chain-chosen parameters come from committed client, all client-chosen parameters
// come from current client.
newClientState := NewClientState(
tmUpgradeClient.ChainId, cs.TrustLevel, cs.TrustingPeriod, tmUpgradeClient.UnbondingPeriod,
tmUpgradeClient.ChainId, cs.TrustLevel, trustingPeriod, tmUpgradeClient.UnbondingPeriod,
cs.MaxClockDrift, tmUpgradeClient.LatestHeight, tmUpgradeClient.ProofSpecs, tmUpgradeClient.UpgradePath,
)

Expand Down Expand Up @@ -165,3 +174,15 @@ func constructUpgradeConsStateMerklePath(upgradePath []string, lastHeight export

return commitmenttypes.NewMerklePath(consStateKey...)
}

// calculateNewTrustingPeriod converts the provided durations to decimal representation to avoid floating-point precision issues
// and calculates the new trusting period, decreasing it by the ratio between the original and new unbonding period.
func calculateNewTrustingPeriod(trustingPeriod, originalUnbonding, newUnbonding time.Duration) time.Duration {
origUnbondingDec := sdkmath.LegacyNewDec(originalUnbonding.Nanoseconds())
newUnbondingDec := sdkmath.LegacyNewDec(newUnbonding.Nanoseconds())
trustingPeriodDec := sdkmath.LegacyNewDec(trustingPeriod.Nanoseconds())

// compute new trusting period: trustingPeriod * newUnbonding / originalUnbonding
newTrustingPeriodDec := trustingPeriodDec.Mul(newUnbondingDec).Quo(origUnbondingDec)
return time.Duration(newTrustingPeriodDec.TruncateInt64())
}
98 changes: 98 additions & 0 deletions modules/light-clients/07-tendermint/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package tendermint_test

import (
"errors"
"time"

sdkmath "cosmossdk.io/math"
upgradetypes "cosmossdk.io/x/upgrade/types"

clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types"
Expand Down Expand Up @@ -86,6 +88,38 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() {
},
expErr: nil,
},
{
name: "successful upgrade with new unbonding period",
setup: func() {
newUnbondingPeriod := time.Hour * 24 * 7 * 2
upgradedClient = ibctm.NewClientState(suite.chainB.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, newUnbondingPeriod, maxClockDrift, clienttypes.NewHeight(clienttypes.ParseChainID(suite.chainB.ChainID), upgradedClient.(*ibctm.ClientState).LatestHeight.GetRevisionHeight()+10), commitmenttypes.GetSDKSpecs(), upgradePath)
upgradedClient = upgradedClient.(*ibctm.ClientState).ZeroCustomFields()
upgradedClientBz, err = clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClient)
suite.Require().NoError(err)

// upgrade Height is at next block
lastHeight = clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1))

// zero custom fields and store in upgrade store
suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) //nolint:errcheck // ignore error for test
suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) //nolint:errcheck // ignore error for test

// commit upgrade store changes and update clients

suite.coordinator.CommitBlock(suite.chainB)
err := path.EndpointA.UpdateClient()
suite.Require().NoError(err)

cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID)
suite.Require().True(found)
tmCs, ok := cs.(*ibctm.ClientState)
suite.Require().True(ok)

upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), tmCs.LatestHeight.GetRevisionHeight())
upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), tmCs.LatestHeight.GetRevisionHeight())
},
expErr: nil,
},
{
name: "unsuccessful upgrade: upgrade path not set",
setup: func() {
Expand Down Expand Up @@ -599,3 +633,67 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() {
})
}
}

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

clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
suite.Require().True(ok)

newUnbondingPeriod := time.Hour * 24 * 7 * 2 // update the unbonding period to two weeks
upgradeClient := ibctm.NewClientState(clientState.ChainId, ibctm.DefaultTrustLevel, trustingPeriod, newUnbondingPeriod, maxClockDrift, clienttypes.NewHeight(1, clientState.LatestHeight.GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath)

upgradedClientBz, err := clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), upgradeClient.ZeroCustomFields())
suite.Require().NoError(err)

upgradedConsState := &ibctm.ConsensusState{NextValidatorsHash: []byte("nextValsHash")} // mocked consensus state
upgradedConsStateBz, err := clienttypes.MarshalConsensusState(suite.chainA.App.AppCodec(), upgradedConsState)
suite.Require().NoError(err)

// zero custom fields and store in chainB upgrade store
upgradeHeight := clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1)) // upgrade is at next block height
err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz)
suite.Require().NoError(err)
err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz)
suite.Require().NoError(err)

// commit upgrade store changes on chainB and update client on chainA
suite.coordinator.CommitBlock(suite.chainB)

err = path.EndpointA.UpdateClient()
suite.Require().NoError(err)

upgradedClientProof, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(upgradeHeight.GetRevisionHeight())), uint64(suite.chainB.LatestCommittedHeader.Header.Height))
upgradedConsensusStateProof, _ := suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(upgradeHeight.GetRevisionHeight())), uint64(suite.chainB.LatestCommittedHeader.Header.Height))

tmClientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
suite.Require().True(ok)

clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID)
err = tmClientState.VerifyUpgradeAndUpdateState(
suite.chainA.GetContext(),
suite.cdc,
clientStore,
upgradeClient,
upgradedConsState,
upgradedClientProof,
upgradedConsensusStateProof,
)
suite.Require().NoError(err)

upgradedClient, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
suite.Require().True(ok)

// assert the unbonding period and the trusting period have been updated correctly
suite.Require().Equal(newUnbondingPeriod, upgradedClient.UnbondingPeriod)

// expected trusting period = trustingPeriod * newUnbonding / originalUnbonding (224 hours = 9 days and 8 hours)
origUnbondingDec := sdkmath.LegacyNewDec(ubdPeriod.Nanoseconds())
newUnbondingDec := sdkmath.LegacyNewDec(newUnbondingPeriod.Nanoseconds())
trustingPeriodDec := sdkmath.LegacyNewDec(trustingPeriod.Nanoseconds())

expTrustingPeriod := trustingPeriodDec.Mul(newUnbondingDec).Quo(origUnbondingDec)
suite.Require().Equal(time.Duration(expTrustingPeriod.TruncateInt64()), upgradedClient.TrustingPeriod)
}