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

Jump to bottom

Switch mock proofs to real proofs; debug #5711

Merged
merged 26 commits into from
Mar 7, 2020
Merged

Switch mock proofs to real proofs; debug #5711

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
61a3e08
Add key, path, value to mock proofs
cwgoes Feb 27, 2020
a1e6bc6
Also alter mock types (why are there duplicates)
cwgoes Feb 27, 2020
d7dab89
Remove mock proofs from handshake_test.go
cwgoes Feb 27, 2020
70afe8d
Use actual proofs
cwgoes Feb 27, 2020
850e6c3
Try to fix historical info, no luck
cwgoes Feb 27, 2020
3e42663
Have test-cases produce consensus heights
cwgoes Feb 27, 2020
32ee4a9
Fix consensus height / proof height difference in verifyClientConsens…
cwgoes Feb 27, 2020
69cf142
Bug fixes contd.
cwgoes Mar 2, 2020
c38d60a
Fix some identifier issues
cwgoes Mar 2, 2020
c81d691
`TestConnOpenConfirm` now works
cwgoes Mar 2, 2020
587ca1f
further on proof
AdityaSripal Mar 3, 2020
c4ecf34
fix debugger print statement
AdityaSripal Mar 3, 2020
e8d346f
IT PASSES
AdityaSripal Mar 3, 2020
046a8bf
revert identifier changes
AdityaSripal Mar 3, 2020
2def886
refactor query proof to generate proofs from either chain
AdityaSripal Mar 3, 2020
c1e5cfd
fix ack and confirm
AdityaSripal Mar 4, 2020
f3db3c3
Remove temporary break
cwgoes Mar 4, 2020
c293d99
fix connection and channel verify tests
AdityaSripal Mar 5, 2020
d53b452
Merge branch 'cwgoes/debug-proofs' of https://github.com/cosmos/cosmo…
AdityaSripal Mar 5, 2020
a681130
fix everything but verify client consensusstate
AdityaSripal Mar 5, 2020
9418881
fix all verify tests
AdityaSripal Mar 5, 2020
445286e
fix conflicts
fedekunze Mar 5, 2020
412a77b
fix ics07 tests
fedekunze Mar 5, 2020
b1a4631
fix handshake tests
AdityaSripal Mar 6, 2020
486ff78
fix packet tests
AdityaSripal Mar 7, 2020
7ebc311
fix timeout tests
AdityaSripal Mar 7, 2020
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
2 changes: 1 addition & 1 deletion simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func NewSimApp(
// During begin block slashing happens after distr.BeginBlocker so that
// there is nothing left over in the validator fee pool, so as to keep the
// CanWithdrawInvariant invariant.
app.mm.SetOrderBeginBlockers(upgrade.ModuleName, mint.ModuleName, distr.ModuleName, slashing.ModuleName, evidence.ModuleName)
app.mm.SetOrderBeginBlockers(upgrade.ModuleName, mint.ModuleName, distr.ModuleName, slashing.ModuleName, evidence.ModuleName, staking.ModuleName)
app.mm.SetOrderEndBlockers(crisis.ModuleName, gov.ModuleName, staking.ModuleName)

// NOTE: The genutils moodule must occur after staking so that pools are
Expand Down
3 changes: 3 additions & 0 deletions x/ibc/02-client/exported/exported.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ type ClientState interface {

VerifyClientConsensusState(
cdc *codec.Codec,
root commitmentexported.Root,
height uint64,
counterpartyClientIdentifier string,
consensusHeight uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
consensusState ConsensusState,
Expand Down
5 changes: 3 additions & 2 deletions x/ibc/02-client/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ func (k Keeper) GetSelfConsensusState(ctx sdk.Context, height uint64) (exported.
valSet := stakingtypes.Validators(histInfo.Valset)

consensusState := ibctmtypes.ConsensusState{
Height: height,
Timestamp: ctx.BlockTime(),
Height: height,
// FIXME: Currently commented out due to time normalisation issues.
//Timestamp: histInfo.Header.Time,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to get fixed in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to figure it out, some sort of normalisation (maybe in Tendermint?).

We should either fix it or open an issue to track it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What feature/guarantee/check are we missing by not having this timestamp?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5762

Root: commitmenttypes.NewMerkleRoot(histInfo.Header.AppHash),
ValidatorSet: tmtypes.NewValidatorSet(valSet.ToTmValidators()),
}
Expand Down
4 changes: 2 additions & 2 deletions x/ibc/03-connection/keeper/handshake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (k Keeper) ConnOpenTry(

// Check that ChainA stored the correct ConsensusState of chainB at the given consensusHeight
if err := k.VerifyClientConsensusState(
ctx, connection, consensusHeight, proofConsensus, expectedConsensusState,
ctx, connection, proofHeight, consensusHeight, proofConsensus, expectedConsensusState,
); err != nil {
return err
}
Expand Down Expand Up @@ -177,7 +177,7 @@ func (k Keeper) ConnOpenAck(

// Ensure that ChainB has stored the correct ConsensusState for chainA at the consensusHeight
if err := k.VerifyClientConsensusState(
ctx, connection, consensusHeight, proofConsensus, expectedConsensusState,
ctx, connection, proofHeight, consensusHeight, proofConsensus, expectedConsensusState,
); err != nil {
return err
}
Expand Down
158 changes: 69 additions & 89 deletions x/ibc/03-connection/keeper/handshake_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

connection "github.com/cosmos/cosmos-sdk/x/ibc/03-connection"
"github.com/cosmos/cosmos-sdk/x/ibc/03-connection/exported"
commitmentexported "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/exported"
ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types"
)

Expand Down Expand Up @@ -47,66 +46,62 @@ func (suite *KeeperTestSuite) TestConnOpenInit() {
// TestConnOpenTry - Chain B (ID #2) calls ConnOpenTry to verify the state of
// connection on Chain A (ID #1) is INIT
func (suite *KeeperTestSuite) TestConnOpenTry() {
// counterparty for A on B
counterparty := connection.NewCounterparty(
testClientIDB, testConnectionIDA, suite.chainB.App.IBCKeeper.ConnectionKeeper.GetCommitmentPrefix(),
)

var (
consensusHeight int64 = 0
proofHeight int64 = 0
)
testCases := []struct {
msg string
proofInit commitmentexported.Proof
proofConsensus commitmentexported.Proof
malleate func()
expPass bool
msg string
malleate func() uint64
expPass bool
}{
{"success", ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
{"success", func() uint64 {
suite.chainB.CreateClient(suite.chainA)
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.INIT)
proofHeight = suite.chainA.Header.Height
suite.chainA.CreateClient(suite.chainB)
consensusHeight = suite.chainB.Header.Height
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.INIT)
suite.chainB.updateClient(suite.chainA)
suite.chainA.updateClient(suite.chainB)
suite.chainB.updateClient(suite.chainA)
suite.chainA.updateClient(suite.chainB)
return uint64(suite.chainB.Header.Height - 1)
}, true},
{"consensus height > latest height", ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
consensusHeight = 100
{"consensus height > latest height", func() uint64 {
return 0
}, false},
{"self consensus state not found", func() uint64 {
//suite.ctx = suite.ctx.WithBlockHeight(100)
return 100
}, false},
// {"self consensus state not found", ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
// consensusHeight = 100
// suite.ctx = suite.ctx.WithBlockHeight(100)
// }, false},
{"connection state verification invalid", ibctypes.InvalidProof{}, ibctypes.ValidProof{}, func() {
{"connection state verification invalid", func() uint64 {
suite.chainB.CreateClient(suite.chainA)
suite.chainA.CreateClient(suite.chainB)
consensusHeight = suite.chainB.Header.Height
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.UNINITIALIZED)
suite.chainB.updateClient(suite.chainA)
return 0
}, false},
{"consensus state verification invalid", ibctypes.ValidProof{}, ibctypes.InvalidProof{}, func() {
{"consensus state verification invalid", func() uint64 {
suite.chainB.CreateClient(suite.chainA)
suite.chainA.CreateClient(suite.chainB)
consensusHeight = suite.chainB.Header.Height
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.INIT)
suite.chainB.updateClient(suite.chainA)
suite.chainA.updateClient(suite.chainB)
return uint64(suite.chainB.Header.Height)
}, false},
{"invalid previous connection", ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
{"invalid previous connection", func() uint64 {
suite.chainB.CreateClient(suite.chainA)
suite.chainA.CreateClient(suite.chainB)
consensusHeight = suite.chainB.Header.Height
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.UNINITIALIZED)
suite.chainB.updateClient(suite.chainA)
suite.chainA.updateClient(suite.chainB)
return 0
}, false},
{"couldn't add connection to client", ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
{"couldn't add connection to client", func() uint64 {
suite.chainB.CreateClient(suite.chainA)
consensusHeight = suite.chainB.Header.Height
suite.chainA.createConnection(testConnectionIDB, testConnectionIDA, testClientIDB, testClientIDA, exported.UNINITIALIZED)
suite.chainB.updateClient(suite.chainA)
suite.chainA.updateClient(suite.chainB)
return 0
}, false},
}

Expand All @@ -115,24 +110,24 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
suite.SetupTest() // reset

tc.malleate()
consensusHeight := tc.malleate()

// connectionKey := ibctypes.KeyConnection(testConnectionIDA)
// proofInit, proofHeight := suite.queryProof(connectionKey)
connectionKey := ibctypes.KeyConnection(testConnectionIDA)
proofInit, proofHeight := queryProof(suite.chainA, connectionKey)

// consensusKey := ibctypes.KeyConsensusState(testClientIDA, uint64(proofHeight))
// proofConsensus, consensusHeight := suite.queryProof(consensusKey)
consensusKey := ibctypes.KeyConsensusState(testClientIDB, consensusHeight)
proofConsensus, _ := queryProof(suite.chainA, consensusKey)

err := suite.chainB.App.IBCKeeper.ConnectionKeeper.ConnOpenTry(
suite.chainB.GetContext(), testConnectionIDB, counterparty, testClientIDA,
connection.GetCompatibleVersions(), tc.proofInit, tc.proofConsensus,
uint64(proofHeight), uint64(consensusHeight),
connection.GetCompatibleVersions(), proofInit, proofConsensus,
uint64(proofHeight+1), consensusHeight,
cwgoes marked this conversation as resolved.
Show resolved Hide resolved
)

if tc.expPass {
suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg)
suite.Require().NoError(err, "valid test case %d failed with consensus height %d and proof height %d: %s", i, consensusHeight, proofHeight, tc.msg)
} else {
suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg)
suite.Require().Error(err, "invalid test case %d passed with consensus height %d and proof height %d: %s", i, consensusHeight, proofHeight, tc.msg)
}
})
}
Expand All @@ -143,66 +138,60 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
func (suite *KeeperTestSuite) TestConnOpenAck() {
version := connection.GetCompatibleVersions()[0]

var (
consensusHeight int64 = 0
proofHeight int64 = 0
)

testCases := []struct {
msg string
version string
proofTry commitmentexported.Proof
proofConsensus commitmentexported.Proof
malleate func()
expPass bool
msg string
version string
malleate func() uint64
expPass bool
}{
{"success", version, ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
{"success", version, func() uint64 {
suite.chainA.CreateClient(suite.chainB)
suite.chainB.CreateClient(suite.chainA)
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.TRYOPEN)
consensusHeight = suite.chainB.Header.Height
proofHeight = suite.chainA.Header.Height
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.INIT)
suite.chainB.updateClient(suite.chainA)
suite.chainA.updateClient(suite.chainB)
return uint64(suite.chainB.Header.Height)
}, true},
{"consensus height > latest height", version, ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
consensusHeight = 100
{"consensus height > latest height", version, func() uint64 {
return 10
}, false},
{"connection not found", version, ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
consensusHeight = suite.chainB.Header.Height
{"connection not found", version, func() uint64 {
return 2
}, false},
{"connection state is not INIT", version, ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
{"connection state is not INIT", version, func() uint64 {
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, exported.UNINITIALIZED)
suite.chainB.updateClient(suite.chainA)
return uint64(suite.chainB.Header.Height)
}, false},
{"incompatible IBC versions", "2.0", ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
{"incompatible IBC versions", "2.0", func() uint64 {
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, exported.INIT)
suite.chainB.updateClient(suite.chainA)
return uint64(suite.chainB.Header.Height)
}, false},
{"self consensus state not found", version, ibctypes.ValidProof{}, ibctypes.ValidProof{}, func() {
{"self consensus state not found", version, func() uint64 {
suite.chainB.CreateClient(suite.chainA)
suite.chainA.CreateClient(suite.chainB)
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.INIT)
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.TRYOPEN)
suite.chainB.updateClient(suite.chainA)
consensusHeight = 100
return uint64(suite.chainB.Header.Height)
}, false},
{"connection state verification failed", version, ibctypes.InvalidProof{}, ibctypes.ValidProof{}, func() {
{"connection state verification failed", version, func() uint64 {
suite.chainB.CreateClient(suite.chainA)
consensusHeight = suite.chainB.Header.Height
suite.chainA.CreateClient(suite.chainB)
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.INIT)
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.UNINITIALIZED)
suite.chainB.updateClient(suite.chainA)
return uint64(suite.chainB.Header.Height)
}, false},
{"consensus state verification failed", version, ibctypes.ValidProof{}, ibctypes.InvalidProof{}, func() {
{"consensus state verification failed", version, func() uint64 {
suite.chainB.CreateClient(suite.chainA)
consensusHeight = suite.chainB.Header.Height
suite.chainA.CreateClient(suite.chainB)
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.INIT)
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.UNINITIALIZED)
suite.chainB.updateClient(suite.chainA)
return uint64(suite.chainB.Header.Height)
}, false},
}

Expand All @@ -211,60 +200,51 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
suite.SetupTest() // reset

tc.malleate()
consensusHeight := tc.malleate()

// connectionKey := ibctypes.KeyConnection(testConnectionIDB)
// proofTry, proofHeight := suite.queryProof(connectionKey)
connectionKey := ibctypes.KeyConnection(testConnectionIDB)
proofTry, proofHeight := queryProof(suite.chainB, connectionKey)

// consensusKey := ibctypes.KeyConsensusState(testClientIDB, uint64(proofHeight))
// proofConsensus, consensusHeight := suite.queryProof(consensusKey)
consensusKey := ibctypes.KeyConsensusState(testClientIDA, uint64(consensusHeight))
proofConsensus, _ := queryProof(suite.chainB, consensusKey)

err := suite.chainA.App.IBCKeeper.ConnectionKeeper.ConnOpenAck(
suite.chainA.GetContext(), testConnectionIDA, tc.version, tc.proofTry, tc.proofConsensus,
uint64(proofHeight), uint64(consensusHeight),
suite.chainA.GetContext(), testConnectionIDA, tc.version, proofTry, proofConsensus,
uint64(proofHeight+1), consensusHeight,
)

if tc.expPass {
suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg)
suite.Require().NoError(err, "valid test case %d failed with consensus height %d: %s", i, consensusHeight, tc.msg)
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
} else {
suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg)
suite.Require().Error(err, "invalid test case %d passed with consensus height %d: %s", i, consensusHeight, tc.msg)
}
})
}
}

// TestConnOpenAck - Chain B (ID #2) calls ConnOpenConfirm to confirm that
// TestConnOpenConfirm - Chain B (ID #2) calls ConnOpenConfirm to confirm that
// Chain A (ID #1) state is now OPEN.
func (suite *KeeperTestSuite) TestConnOpenConfirm() {
// consensusHeight := int64(0)
proofHeight := int64(0)

testCases := []testCase{
{"success", func() {
suite.chainB.CreateClient(suite.chainA)
suite.chainA.CreateClient(suite.chainB)
proofHeight = suite.chainB.Header.Height
// consensusHeight = suite.chainA.Header.Height
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.OPEN)
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.TRYOPEN)
suite.chainB.updateClient(suite.chainA)
}, true},
{"connection not found", func() {}, false},
{"chain B's connection state is not TRYOPEN", func() {
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.UNINITIALIZED)
}, false},
{"consensus state not found", func() {
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.TRYOPEN)
suite.chainA.createConnection(testConnectionIDB, testConnectionIDA, testClientIDB, testClientIDA, exported.OPEN)
suite.chainA.updateClient(suite.chainB)
}, false},
{"connection state verification failed", func() {
suite.chainB.CreateClient(suite.chainA)
suite.chainA.CreateClient(suite.chainB)
// consensusHeight = suite.chainA.Header.Height
proofHeight = suite.chainB.Header.Height
suite.chainB.updateClient(suite.chainA)
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDB, testClientIDA, exported.INIT)
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDA, testClientIDB, exported.TRYOPEN)
suite.chainA.createConnection(testConnectionIDA, testConnectionIDB, testClientIDA, testClientIDB, exported.INIT)
suite.chainB.createConnection(testConnectionIDB, testConnectionIDA, testClientIDB, testClientIDA, exported.TRYOPEN)
suite.chainA.updateClient(suite.chainA)
}, false},
}
Expand All @@ -276,17 +256,17 @@ func (suite *KeeperTestSuite) TestConnOpenConfirm() {

tc.malleate()

// connectionKey := ibctypes.KeyConnection(testConnectionIDB)
// proofAck, proofHeight := suite.queryProof(connectionKey)
connectionKey := ibctypes.KeyConnection(testConnectionIDA)
proofAck, proofHeight := queryProof(suite.chainA, connectionKey)

if tc.expPass {
err := suite.chainB.App.IBCKeeper.ConnectionKeeper.ConnOpenConfirm(
suite.chainB.GetContext(), testConnectionIDB, ibctypes.ValidProof{}, uint64(proofHeight),
suite.chainB.GetContext(), testConnectionIDB, proofAck, uint64(proofHeight+1),
)
suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg)
} else {
err := suite.chainB.App.IBCKeeper.ConnectionKeeper.ConnOpenConfirm(
suite.chainB.GetContext(), testConnectionIDB, ibctypes.InvalidProof{}, uint64(proofHeight),
suite.chainB.GetContext(), testConnectionIDB, proofAck, uint64(proofHeight+1),
)
suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg)
}
Expand Down
Loading