Use Tendermint lite client verification #5666

AdityaSripal · 2020-02-18T15:17:49Z

Closes: #XXX

Description

For contributor use:

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

For admin use:

Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
Reviewers assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

…-sdk into aditya/lite-verify

x/ibc/07-tendermint/types/client_state.go

x/ibc/07-tendermint/misbehaviour.go

jackzampolin

Like moving over to just headers. Simplifies things greatly and will reduce confusion ++

x/ibc/07-tendermint/types/header.go

cwgoes

Do we check the consensus state timestamp when checking misbehaviour?
Is the chain ID field duplicated by the chain ID in the stored header?

Otherwise LGTM.

x/ibc/07-tendermint/misbehaviour.go

golangcibot · 2020-02-19T20:43:35Z

x/ibc/04-channel/keeper/keeper_test.go

 }

 func (suite *KeeperTestSuite) SetupTest() {
 	isCheckTx := false
 	app := simapp.Setup(isCheckTx)
-
+	suite.now = time.Date(2020, 1, 2, 0, 0, 0, 0, time.UTC)
+	now2 := suite.now.Add(time.Duration(time.Hour * 1))


unnecessary conversion (from unconvert)

golangcibot · 2020-02-20T09:49:15Z

x/ibc/ante/ante_test.go

 	testConnection = "testconnection"

 	testChannelVersion = "1.0"

+	height = 10


const height is unused (from unused)

golangcibot · 2020-02-20T09:49:16Z

x/ibc/02-client/keeper/client_test.go

 				}

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

-				consensusState, found := suite.keeper.GetClientConsensusState(suite.ctx, testClientID, uint64(suite.header.GetHeight()))
+				consensusState, found := suite.keeper.GetClientConsensusState(suite.ctx, testClientID, uint64(updateHeader.GetHeight()))


unnecessary conversion (from unconvert)

golangcibot · 2020-02-20T09:49:16Z

x/ibc/20-transfer/keeper/keeper_test.go

@@ -24,6 +25,7 @@ const (
 	testChainID    = "test-chain-id"
 	testClient     = "test-client"
 	testClientType = clientexported.Tendermint
+	height         = 10


height is unused (from deadcode)

golangcibot · 2020-02-20T09:49:17Z

x/ibc/20-transfer/handler_test.go

@@ -37,6 +37,9 @@ const (
 	testChannel1   = "firstchannel"
 	testChannel2   = "secondchannel"

+	height  = 10


height is unused (from deadcode)

golangcibot · 2020-02-20T18:51:06Z

x/ibc/03-connection/keeper/keeper_test.go

 	return connection
 }

-func (suite *KeeperTestSuite) createChannel(
+func (chain *TestChain) createChannel(


func (*TestChain).createChannel is unused (from unused)

golangcibot · 2020-02-20T18:51:06Z

x/ibc/03-connection/keeper/keeper_test.go

-	_, err = suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, clientState, consensusState)
-	suite.Require().NoError(err)
+// createClient will create a client for clientChain on targetChain
+func (target *TestChain) CreateClient(client *TestChain) error {


receiver name target should be consistent with previous receiver name chain for TestChain (from golint)

golangcibot · 2020-02-20T18:51:06Z

x/ibc/03-connection/keeper/keeper_test.go

@@ -140,28 +185,32 @@ func (suite *KeeperTestSuite) createClient(clientID string) {
 	// )
 }

-func (suite *KeeperTestSuite) updateClient(clientID string) {
+func (target *TestChain) updateClient(client *TestChain) {


receiver name target should be consistent with previous receiver name chain for TestChain (from golint)

…-sdk into aditya/lite-verify

x/ibc/07-tendermint/types/header.go

cwgoes · 2020-02-21T10:27:51Z

x/ibc/03-connection/keeper/verify.go

@@ -19,9 +19,10 @@ func (k Keeper) VerifyClientConsensusState(
 	proof commitment.ProofI,
 	consensusState clientexported.ConsensusState,
 ) error {
-	clientState, found := k.clientKeeper.GetClientState(ctx, connection.GetClientID())
+	clientID := connection.GetCounterparty().GetClientID()


This line seems wrong to me - we should never be fetching the local client state by looking up the counterparty's client identifier. The only thing we need the counterparty's client identifier for is to check whether the correct client state is stored under it (for this chain's consensus algorithm).

…rect fixes

golangcibot · 2020-02-21T15:31:15Z

x/ibc/20-transfer/handler_test.go

@@ -37,6 +40,9 @@ const (
 	testChannel1   = "firstchannel"
 	testChannel2   = "secondchannel"

+	height  = 10
+	chainID = "gaia"


chainID is unused (from varcheck)

golangcibot · 2020-02-21T15:31:15Z

x/ibc/20-transfer/keeper/keeper_test.go

@@ -39,6 +43,8 @@ const (
 	testChannelOrder   = channelexported.UNORDERED
 	testChannelVersion = "1.0"

+	chainID = "gaia"


chainID is unused (from varcheck)

golangcibot · 2020-02-21T15:31:16Z

x/ibc/04-channel/keeper/keeper_test.go

@@ -45,6 +46,7 @@ const (
 	testChannelOrder   = exported.ORDERED
 	testChannelVersion = "1.0"

+	chainID    = "gaia"


chainID is unused (from varcheck)

golangcibot · 2020-02-21T15:31:16Z

x/ibc/20-transfer/handler_test.go

+}
+
+// createClient will create a client for clientChain on targetChain
+func (target *TestChain) CreateClient(client *TestChain) error {


receiver name target should be consistent with previous receiver name chain for TestChain (from golint)

golangcibot · 2020-02-21T15:31:16Z

x/ibc/20-transfer/handler_test.go

+	// )
+}
+
+func (target *TestChain) updateClient(client *TestChain) {


receiver name target should be consistent with previous receiver name chain for TestChain (from golint)

golangcibot · 2020-02-21T15:31:16Z

x/ibc/20-transfer/keeper/keeper_test.go

-			ConnectionID: testConnection,
-			Prefix:       suite.app.IBCKeeper.ConnectionKeeper.GetCommitmentPrefix(),
+// createClient will create a client for clientChain on targetChain
+func (target *TestChain) CreateClient(client *TestChain) error {


receiver name target should be consistent with previous receiver name chain for TestChain (from golint)

golangcibot · 2020-02-21T17:09:39Z

x/ibc/20-transfer/keeper/keeper_test.go

-		Data:  key,
-		Prove: true,
-	})
+func (target *TestChain) updateClient(client *TestChain) {


func (*TestChain).updateClient is unused (from unused)

golangcibot · 2020-02-21T17:09:39Z

x/ibc/ante/ante_test.go

 	testClientType = clientexported.Tendermint

+	chainID        = "gaia"


chainID is unused (from deadcode)

golangcibot · 2020-02-21T17:09:39Z

x/ibc/ante/ante_test.go

+}
+
+// createClient will create a client for clientChain on targetChain
+func (target *TestChain) CreateClient(client *TestChain) error {


receiver name target should be consistent with previous receiver name chain for TestChain (from golint)

golangcibot · 2020-02-21T17:09:39Z

x/ibc/ante/ante_test.go

+	// )
+}
+
+func (target *TestChain) updateClient(client *TestChain) {


receiver name target should be consistent with previous receiver name chain for TestChain (from golint)

cwgoes

utACK modulo @fedekunze comment

cwgoes · 2020-02-21T17:19:43Z

Also tested locally 👍 .

golangcibot · 2020-02-21T17:28:02Z

x/ibc/20-transfer/handler_test.go

+	// )
+}
+
+func (chain *TestChain) updateClient(client *TestChain) {


func (*TestChain).updateClient is unused (from unused)

AdityaSripal added 2 commits February 18, 2020 07:15

start ics07 integration with lite.Verify

7b8afe1

Merge branch 'aditya/lite-verify' of https://github.com/cosmos/cosmos…

0732280

…-sdk into aditya/lite-verify

AdityaSripal requested review from alessio, alexanderbez, fedekunze and jackzampolin as code owners February 18, 2020 15:17

AdityaSripal commented Feb 18, 2020

View reviewed changes

x/ibc/07-tendermint/types/client_state.go Show resolved Hide resolved

AdityaSripal commented Feb 18, 2020

View reviewed changes

x/ibc/07-tendermint/types/client_state.go Show resolved Hide resolved

x/ibc/07-tendermint/types/client_state.go Show resolved Hide resolved

jackzampolin reviewed Feb 18, 2020

View reviewed changes

x/ibc/07-tendermint/misbehaviour.go Outdated Show resolved Hide resolved

jackzampolin reviewed Feb 18, 2020

View reviewed changes

jackzampolin approved these changes Feb 18, 2020

View reviewed changes

fedekunze reviewed Feb 18, 2020

View reviewed changes

x/ibc/07-tendermint/types/header.go Show resolved Hide resolved

jackzampolin and others added 5 commits February 18, 2020 16:03

make build passes, there are still a number of tests to update

8f9f329

WIP test refactor

77c14a4

start fixing tendermint tests

f9e0f30

add test cases to improve coverage

4ac36fa

fix merge conflicts

32ac85a

cwgoes suggested changes Feb 19, 2020

View reviewed changes

x/ibc/07-tendermint/misbehaviour.go Outdated Show resolved Hide resolved

Tests compiling

8c5d6ab

golangcibot reviewed Feb 19, 2020

View reviewed changes

cwgoes mentioned this pull request Feb 19, 2020

The one true master IBC implementation tracking issue cosmos/gaia#201

Closed

34 tasks

AdityaSripal added 3 commits February 20, 2020 00:57

fix tests

b504fed

fix merge

d99fb3c

fix missed merge conflicts and fix ics2 tests

07aae46

golangcibot reviewed Feb 20, 2020

View reviewed changes

AdityaSripal added 2 commits February 20, 2020 03:48

add comments for connection handshake code

6a17783

start refactor of test code

bc2b74d

golangcibot reviewed Feb 20, 2020

View reviewed changes

AdityaSripal and others added 2 commits February 20, 2020 12:13

complete verify_test

73a209f

Merge branch 'ibc-alpha' into aditya/lite-verify

433d9a3

cwgoes and others added 4 commits February 20, 2020 22:05

Fix spacing from merge

195afbc

Note blocking ICS issue

e5dd0ff

fix 03-connection tests

d95840e

Merge branch 'aditya/lite-verify' of https://github.com/cosmos/cosmos…

b03efa8

…-sdk into aditya/lite-verify

fedekunze reviewed Feb 21, 2020

View reviewed changes

x/ibc/07-tendermint/types/header.go Show resolved Hide resolved

readd slashing defensive check

99116c5

cwgoes reviewed Feb 21, 2020

View reviewed changes

AdityaSripal added 4 commits February 21, 2020 03:51

fix connection tests to have right connection structure; revert incor…

e32aa1c

…rect fixes

fix keeper and handshake tests

5e2cfaf

fix 04-channel tests

baffeda

fix 20-transfer tests

1e035b0

golangcibot reviewed Feb 21, 2020

View reviewed changes

AdityaSripal added 2 commits February 21, 2020 07:50

get ante test to build

faed84d

fixed ante tests

b78c92e

AdityaSripal added x/ibc labels Feb 21, 2020

golangcibot reviewed Feb 21, 2020

View reviewed changes

cwgoes approved these changes Feb 21, 2020

View reviewed changes

address fede review

0a3b207

linting

d41d155

cwgoes merged commit c3bb696 into ibc-alpha Feb 21, 2020

cwgoes deleted the aditya/lite-verify branch February 21, 2020 17:25

golangcibot reviewed Feb 21, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Tendermint lite client verification #5666

Use Tendermint lite client verification #5666

AdityaSripal commented Feb 18, 2020

jackzampolin left a comment

cwgoes left a comment

golangcibot Feb 19, 2020

golangcibot Feb 20, 2020

golangcibot Feb 20, 2020

golangcibot Feb 20, 2020

golangcibot Feb 20, 2020

golangcibot Feb 20, 2020

golangcibot Feb 20, 2020

golangcibot Feb 20, 2020

cwgoes Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

golangcibot Feb 21, 2020

cwgoes left a comment

cwgoes commented Feb 21, 2020

golangcibot Feb 21, 2020

Use Tendermint lite client verification #5666

Use Tendermint lite client verification #5666

Conversation

AdityaSripal commented Feb 18, 2020

Description

jackzampolin left a comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

cwgoes commented Feb 21, 2020

Choose a reason for hiding this comment