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.

Sign up for GitHub

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

Time Monotonicity Enforcement #141

Merged
merged 34 commits into from
May 10, 2021
Merged

Time Monotonicity Enforcement #141

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
993f5f2
implement update client fix and start changing tests
AdityaSripal Mar 19, 2021
1ef071c
fix bug and write identical case test
AdityaSripal Mar 19, 2021
d432451
write misbehaviour detection tests
AdityaSripal Mar 19, 2021
593e5a5
add misbehaviour events to UpdateClient
AdityaSripal Mar 19, 2021
38f6115
fix client keeper and write tests
AdityaSripal Mar 19, 2021
4a56b57
add Freeze to ClientState interface
AdityaSripal Mar 25, 2021
77f4c2f
Merge branch 'main' into aditya/update-client-fix
colin-axner Mar 26, 2021
84adabc
Merge branch 'aditya/update-client-fix' of github.com:cosmos/ibc-go-g…
AdityaSripal Mar 26, 2021
a29d312
add cache context and fix events
AdityaSripal Mar 26, 2021
b6a4635
Update modules/light-clients/07-tendermint/types/update.go
colin-axner Mar 30, 2021
9edfaf6
address colin comments
AdityaSripal Mar 30, 2021
dedd6d5
Merge branch 'aditya/update-client-fix' of github.com:cosmos/ibc-go-g…
AdityaSripal Mar 30, 2021
7172807
freeze entire client on misbehaviour
AdityaSripal Mar 30, 2021
b3e50b1
add time misbehaviour and tests
AdityaSripal Mar 30, 2021
117d7d1
Merge branch 'main' into alderfly-ibc-fix
AdityaSripal Mar 30, 2021
937364d
enforce trusted height less than current height in header.ValidateBasic
AdityaSripal Mar 31, 2021
3ec116d
Merge branch 'alderfly-ibc-fix' of github.com:cosmos/ibc-go-ghsa-fw94…
AdityaSripal Mar 31, 2021
5c793ae
cleanup and tests
AdityaSripal Mar 31, 2021
906a413
fix print statement
AdityaSripal Apr 26, 2021
f62cca7
fix merge
AdityaSripal Apr 26, 2021
155e461
enforce monotonicity in update
AdityaSripal Apr 26, 2021
326e5fb
add docs and remove unnecessary interface function
AdityaSripal Apr 26, 2021
2eaa2c9
first round of review comments
AdityaSripal Apr 28, 2021
f63ae67
CHANGELOG
AdityaSripal Apr 28, 2021
c3ac1eb
update updateclient test
AdityaSripal Apr 29, 2021
75bf94a
bump tendermint to 0.34.10
colin-axner Apr 30, 2021
e2a70fb
Merge branch 'main' into alderfly-ibc-fix
colin-axner Apr 30, 2021
eab22f0
remove caching and specific frozen height
AdityaSripal Apr 30, 2021
979435a
Merge branch 'alderfly-ibc-fix' of github.com:cosmos/ibc-go into alde…
AdityaSripal Apr 30, 2021
c7f8bd2
document in go code
AdityaSripal Apr 30, 2021
76e932a
DRY FrozenHeight
AdityaSripal May 3, 2021
cbbb715
fix merge conflicts
colin-axner May 5, 2021
2f90b44
fix build
colin-axner May 5, 2021
b288be9
fix minor merge conflicts
colin-axner May 10, 2021
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
238 changes: 104 additions & 134 deletions modules/core/02-client/keeper/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,216 +42,186 @@ func (suite *KeeperTestSuite) TestCreateClient() {
}

func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
// Must create header creation functions since suite.header gets recreated on each test case
createFutureUpdateFn := func(s *KeeperTestSuite) *ibctmtypes.Header {
heightPlus3 := clienttypes.NewHeight(suite.header.GetHeight().GetRevisionNumber(), suite.header.GetHeight().GetRevisionHeight()+3)
height := suite.header.GetHeight().(clienttypes.Height)
var (
path *ibctesting.Path
updateHeader *ibctmtypes.Header
)

return suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus3.RevisionHeight), height, suite.header.Header.Time.Add(time.Hour),
suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal})
// Must create header creation functions since suite.header gets recreated on each test case
createFutureUpdateFn := func(trustedHeight clienttypes.Height) *ibctmtypes.Header {
header, err := suite.chainA.ConstructUpdateTMClientHeaderWithTrustedHeight(path.EndpointB.Chain, path.EndpointA.ClientID, trustedHeight)
suite.Require().NoError(err)
return header
}
createPastUpdateFn := func(s *KeeperTestSuite) *ibctmtypes.Header {
heightMinus2 := clienttypes.NewHeight(suite.header.GetHeight().GetRevisionNumber(), suite.header.GetHeight().GetRevisionHeight()-2)
heightMinus4 := clienttypes.NewHeight(suite.header.GetHeight().GetRevisionNumber(), suite.header.GetHeight().GetRevisionHeight()-4)
createPastUpdateFn := func(fillHeight, trustedHeight clienttypes.Height) *ibctmtypes.Header {
consState, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, trustedHeight)
suite.Require().True(found)

return suite.chainA.CreateTMClientHeader(testChainID, int64(heightMinus2.RevisionHeight), heightMinus4, suite.header.Header.Time,
suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal})
return suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(fillHeight.RevisionHeight), trustedHeight, consState.(*ibctmtypes.ConsensusState).Timestamp.Add(time.Second*5),
suite.chainB.Vals, suite.chainB.Vals, suite.chainB.Signers)
}
var (
updateHeader *ibctmtypes.Header
clientState *ibctmtypes.ClientState
clientID string
err error
)

cases := []struct {
name string
malleate func() error
expPass bool
freeze bool // only true if update freezes the client to a new frozen height. false if client is already frozen and header is valid past update
name string
malleate func()
expPass bool
expFreeze bool
}{
{"valid update", func() error {
clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState)
{"valid update", func() {
clientState := path.EndpointA.GetClientState().(*ibctmtypes.ClientState)
trustHeight := clientState.GetLatestHeight().(types.Height)

// store intermediate consensus state to check that trustedHeight does not need to be highest consensus state before header height
incrementedClientHeight := testClientHeight.Increment().(types.Height)
intermediateConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.now.Add(time.Minute),
NextValidatorsHash: suite.valSetHash,
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, incrementedClientHeight, intermediateConsState)
path.EndpointA.UpdateClient()

clientState.LatestHeight = incrementedClientHeight
suite.keeper.SetClientState(suite.ctx, clientID, clientState)

updateHeader = createFutureUpdateFn(suite)
return err
updateHeader = createFutureUpdateFn(trustHeight)
}, true, false},
{"valid past update", func() error {
clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState)
suite.Require().NoError(err)
{"valid past update", func() {
clientState := path.EndpointA.GetClientState()
trustedHeight := clientState.GetLatestHeight().(types.Height)

height1 := types.NewHeight(0, 1)
currHeight := suite.chainB.CurrentHeader.Height
fillHeight := types.NewHeight(clientState.GetLatestHeight().GetRevisionNumber(), uint64(currHeight))

// store previous consensus state
prevConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.past,
NextValidatorsHash: suite.valSetHash,
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, height1, prevConsState)
// commit a couple blocks to allow client to fill in gaps
suite.coordinator.CommitBlock(suite.chainB) // this height is not filled in yet
suite.coordinator.CommitBlock(suite.chainB) // this height is filled in by the update below

height2 := types.NewHeight(0, 2)
path.EndpointA.UpdateClient()

// store intermediate consensus state to check that trustedHeight does not need to be hightest consensus state before header height
intermediateConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.past.Add(time.Minute),
NextValidatorsHash: suite.valSetHash,
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, height2, intermediateConsState)
// ensure fill height not set
_, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, fillHeight)
suite.Require().False(found)

// updateHeader will fill in consensus state between prevConsState and suite.consState
// clientState should not be updated
updateHeader = createPastUpdateFn(suite)
return nil
updateHeader = createPastUpdateFn(fillHeight, trustedHeight)
}, true, false},
{"valid duplicate update", func() error {
clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState)
suite.Require().NoError(err)
{"valid duplicate update", func() {
clientID := path.EndpointA.ClientID

height1 := types.NewHeight(0, 1)

// store previous consensus state
prevConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.past,
NextValidatorsHash: suite.valSetHash,
NextValidatorsHash: suite.chainB.Vals.Hash(),
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, height1, prevConsState)

height2 := types.NewHeight(0, 2)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, height1, prevConsState)

// store intermediate consensus state to check that trustedHeight does not need to be hightest consensus state before header height
intermediateConsState := &ibctmtypes.ConsensusState{
height5 := types.NewHeight(0, 5)
// store next consensus state to check that trustedHeight does not need to be hightest consensus state before header height
nextConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.past.Add(time.Minute),
NextValidatorsHash: suite.valSetHash,
NextValidatorsHash: suite.chainB.Vals.Hash(),
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, height2, intermediateConsState)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, height5, nextConsState)

height3 := types.NewHeight(0, 3)
// updateHeader will fill in consensus state between prevConsState and suite.consState
// clientState should not be updated
updateHeader = createPastUpdateFn(suite)
updateHeader = createPastUpdateFn(height3, height1)
// set updateHeader's consensus state in store to create duplicate UpdateClient scenario
suite.keeper.SetClientConsensusState(suite.ctx, clientID, updateHeader.GetHeight(), updateHeader.ConsensusState())
return nil
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, updateHeader.GetHeight(), updateHeader.ConsensusState())
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
}, true, false},
{"misbehaviour detection: conflicting header", func() error {
clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState)
suite.Require().NoError(err)
{"misbehaviour detection: conflicting header", func() {
clientID := path.EndpointA.ClientID

height1 := types.NewHeight(0, 1)

// store previous consensus state
prevConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.past,
NextValidatorsHash: suite.valSetHash,
NextValidatorsHash: suite.chainB.Vals.Hash(),
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, height1, prevConsState)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, height1, prevConsState)

height2 := types.NewHeight(0, 2)

// store intermediate consensus state to check that trustedHeight does not need to be hightest consensus state before header height
intermediateConsState := &ibctmtypes.ConsensusState{
height5 := types.NewHeight(0, 5)
// store next consensus state to check that trustedHeight does not need to be hightest consensus state before header height
nextConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.past.Add(time.Minute),
NextValidatorsHash: suite.valSetHash,
NextValidatorsHash: suite.chainB.Vals.Hash(),
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, height2, intermediateConsState)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, height5, nextConsState)

height3 := types.NewHeight(0, 3)
// updateHeader will fill in consensus state between prevConsState and suite.consState
// clientState should not be updated
updateHeader = createPastUpdateFn(suite)
updateHeader = createPastUpdateFn(height3, height1)
// set conflicting consensus state in store to create misbehaviour scenario
conflictConsState := updateHeader.ConsensusState()
conflictConsState.Root = commitmenttypes.NewMerkleRoot([]byte("conflicting apphash"))
suite.keeper.SetClientConsensusState(suite.ctx, clientID, updateHeader.GetHeight(), conflictConsState)
return nil
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, updateHeader.GetHeight(), conflictConsState)
}, true, true},
{"misbehaviour detection: monotonic time violation", func() error {
clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState)
{"misbehaviour detection: monotonic time violation", func() {
clientState := path.EndpointA.GetClientState().(*ibctmtypes.ClientState)
clientID := path.EndpointA.ClientID
trustedHeight := clientState.GetLatestHeight().(types.Height)

// store intermediate consensus state at a time greater than updateHeader time
// this will break time monotonicity
incrementedClientHeight := testClientHeight.Increment().(types.Height)
incrementedClientHeight := clientState.GetLatestHeight().Increment().(types.Height)
intermediateConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.header.Header.Time.Add(2 * time.Hour),
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
NextValidatorsHash: suite.valSetHash,
NextValidatorsHash: suite.chainB.Vals.Hash(),
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, incrementedClientHeight, intermediateConsState)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, incrementedClientHeight, intermediateConsState)
// set iteration key
clientStore := suite.keeper.ClientStore(suite.ctx, clientID)
ibctmtypes.SetIterationKey(clientStore, incrementedClientHeight)

clientState.LatestHeight = incrementedClientHeight
suite.keeper.SetClientState(suite.ctx, clientID, clientState)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), clientID, clientState)

updateHeader = createFutureUpdateFn(suite)
return err
updateHeader = createFutureUpdateFn(trustedHeight)
}, true, true},
{"client state not found", func() error {
updateHeader = createFutureUpdateFn(suite)
{"client state not found", func() {
updateHeader = createFutureUpdateFn(path.EndpointA.GetClientState().GetLatestHeight().(types.Height))

return nil
path.EndpointA.ClientID = ibctesting.InvalidID
}, false, false},
{"consensus state not found", func() error {
clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
suite.keeper.SetClientState(suite.ctx, testClientID, clientState)
updateHeader = createFutureUpdateFn(suite)

return nil
{"consensus state not found", func() {
clientState := path.EndpointA.GetClientState()
tmClient, ok := clientState.(*ibctmtypes.ClientState)
suite.Require().True(ok)
tmClient.LatestHeight = tmClient.LatestHeight.Increment().(types.Height)

suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientState)
updateHeader = createFutureUpdateFn(clientState.GetLatestHeight().(types.Height))
}, false, false},
{"frozen client before update", func() error {
clientState = &ibctmtypes.ClientState{FrozenHeight: types.NewHeight(0, 1), LatestHeight: testClientHeight}
suite.keeper.SetClientState(suite.ctx, testClientID, clientState)
updateHeader = createFutureUpdateFn(suite)

return nil
{"client is not active", func() {
clientState := path.EndpointA.GetClientState().(*ibctmtypes.ClientState)
clientState.FrozenHeight = types.NewHeight(0, 1)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientState)
updateHeader = createFutureUpdateFn(clientState.GetLatestHeight().(types.Height))
}, false, false},
{"invalid header", func() error {
clientState = ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false)
_, err := suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState)
suite.Require().NoError(err)
updateHeader = createPastUpdateFn(suite)

return nil
{"invalid header", func() {
updateHeader = createFutureUpdateFn(path.EndpointA.GetClientState().GetLatestHeight().(types.Height))
updateHeader.TrustedHeight = updateHeader.TrustedHeight.Increment().(types.Height)
}, false, false},
}

for i, tc := range cases {
for _, tc := range cases {
tc := tc
i := i
suite.Run(fmt.Sprintf("Case %s", tc.name), func() {
suite.SetupTest()
clientID = testClientID // must be explicitly changed
path = ibctesting.NewPath(suite.chainA, suite.chainB)
suite.coordinator.SetupClients(path)

err := tc.malleate()
suite.Require().NoError(err)
tc.malleate()

suite.ctx = suite.ctx.WithBlockTime(updateHeader.Header.Time.Add(time.Minute))
var clientState exported.ClientState
if tc.expPass {
clientState = path.EndpointA.GetClientState()
}

err = suite.keeper.UpdateClient(suite.ctx, clientID, updateHeader)
err := suite.chainA.App.GetIBCKeeper().ClientKeeper.UpdateClient(suite.chainA.GetContext(), path.EndpointA.ClientID, updateHeader)

if tc.expPass {
suite.Require().NoError(err, err)

newClientState, found := suite.keeper.GetClientState(suite.ctx, clientID)
suite.Require().True(found, "valid test case %d failed: %s", i, tc.name)
newClientState := path.EndpointA.GetClientState()

// If the update freezes the client, check that client was frozen to update header's height.
// Otherwise check that consensus state is stored as expected.
if tc.freeze {
if tc.expFreeze {
suite.Require().True(newClientState.IsFrozen(), "client did not freeze after conflicting header was submitted to UpdateClient")
suite.Require().Equal(newClientState.GetFrozenHeight(), updateHeader.GetHeight(), "client frozen at wrong height")
} else {
Expand All @@ -261,8 +231,8 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
NextValidatorsHash: updateHeader.Header.NextValidatorsHash,
}

consensusState, found := suite.keeper.GetClientConsensusState(suite.ctx, clientID, updateHeader.GetHeight())
suite.Require().True(found, "valid test case %d failed: %s", i, tc.name)
consensusState, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, updateHeader.GetHeight())
suite.Require().True(found)

// Determine if clientState should be updated or not
if updateHeader.GetHeight().GT(clientState.GetLatestHeight()) {
Expand All @@ -273,11 +243,11 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
suite.Require().Equal(clientState.GetLatestHeight(), newClientState.GetLatestHeight(), "client state height updated for past header")
}

suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name)
suite.Require().NoError(err)
suite.Require().Equal(expConsensusState, consensusState, "consensus state should have been updated on case %s", tc.name)
}
} else {
suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name)
suite.Require().Error(err)
}
})
}
Expand Down Expand Up @@ -536,7 +506,7 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() {
// store intermediate consensus state to check that trustedHeight does not need to be highest consensus state before header height
intermediateConsState := &ibctmtypes.ConsensusState{
Timestamp: suite.now.Add(time.Minute),
NextValidatorsHash: suite.valSetHash,
NextValidatorsHash: suite.chainB.Vals.Hash(),
}
suite.keeper.SetClientConsensusState(suite.ctx, clientID, heightPlus3, intermediateConsState)

Expand Down
Loading