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

x/ibc: stateful clients #6202

Merged
merged 4 commits into from
May 12, 2020
Merged

x/ibc: stateful clients #6202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9fbf778
x/ibc: stateful clients
fedekunze May 12, 2020
2b2bf84
update localhost client
fedekunze May 12, 2020
bb9a0c5
fix tests
fedekunze May 12, 2020
c91b576
Merge branch 'master' into fedekunze/6198-remove-store
fedekunze May 12, 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
7 changes: 7 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 @@ -25,6 +25,7 @@ type ClientState interface {
// State verification functions

VerifyClientConsensusState(
store sdk.KVStore,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replaced sdk.Context with KVStore since the client won't be able to access the store thought the context unless we passed the IBC StoreKey as an argument.

This approach is more secure because we can just pass the prefix store for the specific client.

cdc *codec.Codec,
root commitmentexported.Root,
height uint64,
Expand All @@ -35,6 +36,7 @@ type ClientState interface {
consensusState ConsensusState,
) error
VerifyConnectionState(
store sdk.KVStore,
cdc codec.Marshaler,
height uint64,
prefix commitmentexported.Prefix,
Expand All @@ -44,6 +46,7 @@ type ClientState interface {
consensusState ConsensusState,
) error
VerifyChannelState(
store sdk.KVStore,
cdc codec.Marshaler,
height uint64,
prefix commitmentexported.Prefix,
Expand All @@ -54,6 +57,7 @@ type ClientState interface {
consensusState ConsensusState,
) error
VerifyPacketCommitment(
store sdk.KVStore,
height uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
Expand All @@ -64,6 +68,7 @@ type ClientState interface {
consensusState ConsensusState,
) error
VerifyPacketAcknowledgement(
store sdk.KVStore,
height uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
Expand All @@ -74,6 +79,7 @@ type ClientState interface {
consensusState ConsensusState,
) error
VerifyPacketAcknowledgementAbsence(
store sdk.KVStore,
height uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
Expand All @@ -83,6 +89,7 @@ type ClientState interface {
consensusState ConsensusState,
) error
VerifyNextSequenceRecv(
store sdk.KVStore,
height uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
Expand Down
6 changes: 1 addition & 5 deletions x/ibc/02-client/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ func InitGenesis(ctx sdk.Context, k Keeper, gs GenesisState) {
}

// client id is always "localhost"
clientState := localhosttypes.NewClientState(
k.ClientStore(ctx, exported.ClientTypeLocalHost),
ctx.ChainID(),
ctx.BlockHeight(),
)
clientState := localhosttypes.NewClientState(ctx.ChainID(), ctx.BlockHeight())

_, err := k.CreateClient(ctx, clientState, nil)
if err != nil {
Expand Down
6 changes: 1 addition & 5 deletions x/ibc/02-client/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ func HandleMsgCreateClient(ctx sdk.Context, k Keeper, msg exported.MsgCreateClie
}
case exported.Localhost:
// msg client id is always "localhost"
clientState = localhosttypes.NewClientState(
k.ClientStore(ctx, msg.GetClientID()),
ctx.ChainID(),
ctx.BlockHeight(),
)
clientState = localhosttypes.NewClientState(ctx.ChainID(), ctx.BlockHeight())
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks a lot cleaner 🙌 I think it'll be pretty straight forward to create interfaces funcs for clients to implement so we can remove custom client handling here (see #6064)

default:
return nil, sdkerrors.Wrap(ErrInvalidClientType, msg.GetClientType())
}
Expand Down
1 change: 0 additions & 1 deletion x/ibc/02-client/keeper/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
case exported.Localhost:
// override client state and update the block height
clientState = localhosttypes.NewClientState(
k.ClientStore(ctx, clientState.GetID()),
ctx.ChainID(), // use the chain ID from context since the client is from the running chain (i.e self).
ctx.BlockHeight(),
)
Expand Down
6 changes: 1 addition & 5 deletions x/ibc/02-client/keeper/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
}

func (suite *KeeperTestSuite) TestUpdateClientLocalhost() {
var localhostClient exported.ClientState = localhosttypes.NewClientState(
suite.keeper.ClientStore(suite.ctx, exported.ClientTypeLocalHost),
suite.header.ChainID,
suite.ctx.BlockHeight(),
)
var localhostClient exported.ClientState = localhosttypes.NewClientState(suite.header.ChainID, suite.ctx.BlockHeight())

suite.ctx = suite.ctx.WithBlockHeight(suite.ctx.BlockHeight() + 1)

Expand Down
13 changes: 4 additions & 9 deletions x/ibc/02-client/types/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ import (
"github.com/stretchr/testify/require"

tmtypes "github.com/tendermint/tendermint/types"
dbm "github.com/tendermint/tm-db"

"github.com/cosmos/cosmos-sdk/store/cachekv"
"github.com/cosmos/cosmos-sdk/store/dbadapter"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types"
Expand All @@ -36,8 +33,6 @@ func TestValidateGenesis(t *testing.T) {
val := tmtypes.NewValidator(pubKey, 10)
valSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{val})

mem := dbadapter.Store{DB: dbm.NewMemDB()}
store := cachekv.NewStore(mem)
header := ibctmtypes.CreateTestHeader("chainID", 10, now, valSet, []tmtypes.PrivValidator{privVal})

testCases := []struct {
Expand All @@ -55,7 +50,7 @@ func TestValidateGenesis(t *testing.T) {
genState: types.NewGenesisState(
[]exported.ClientState{
ibctmtypes.NewClientState(clientID, trustingPeriod, ubdPeriod, maxClockDrift, header),
localhosttypes.NewClientState(store, "chaindID", 10),
localhosttypes.NewClientState("chaindID", 10),
},
[]types.ClientConsensusStates{
{
Expand All @@ -76,7 +71,7 @@ func TestValidateGenesis(t *testing.T) {
genState: types.NewGenesisState(
[]exported.ClientState{
ibctmtypes.NewClientState(clientID, trustingPeriod, ubdPeriod, maxClockDrift, header),
localhosttypes.NewClientState(store, "chaindID", 0),
localhosttypes.NewClientState("chaindID", 0),
},
nil,
true,
Expand All @@ -88,7 +83,7 @@ func TestValidateGenesis(t *testing.T) {
genState: types.NewGenesisState(
[]exported.ClientState{
ibctmtypes.NewClientState(clientID, trustingPeriod, ubdPeriod, maxClockDrift, header),
localhosttypes.NewClientState(store, "chaindID", 10),
localhosttypes.NewClientState("chaindID", 10),
},
[]types.ClientConsensusStates{
{
Expand All @@ -109,7 +104,7 @@ func TestValidateGenesis(t *testing.T) {
genState: types.NewGenesisState(
[]exported.ClientState{
ibctmtypes.NewClientState(clientID, trustingPeriod, ubdPeriod, maxClockDrift, header),
localhosttypes.NewClientState(store, "chaindID", 10),
localhosttypes.NewClientState("chaindID", 10),
},
[]types.ClientConsensusStates{
types.NewClientConsensusStates(
Expand Down
21 changes: 14 additions & 7 deletions x/ibc/03-connection/keeper/verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ func (k Keeper) VerifyClientConsensusState(
}

return clientState.VerifyClientConsensusState(
k.aminoCdc, targetConsState.GetRoot(), height, connection.GetCounterparty().GetClientID(), consensusHeight, connection.GetCounterparty().GetPrefix(), proof, consensusState,
k.clientKeeper.ClientStore(ctx, clientID), k.aminoCdc, targetConsState.GetRoot(), height,
connection.GetCounterparty().GetClientID(), consensusHeight, connection.GetCounterparty().GetPrefix(), proof, consensusState,
)
}

Expand Down Expand Up @@ -63,7 +64,8 @@ func (k Keeper) VerifyConnectionState(
}

return clientState.VerifyConnectionState(
k.cdc, height, connection.GetCounterparty().GetPrefix(), proof, connectionID, connectionEnd, consensusState,
k.clientKeeper.ClientStore(ctx, connection.GetClientID()), k.cdc, height,
connection.GetCounterparty().GetPrefix(), proof, connectionID, connectionEnd, consensusState,
)
}

Expand Down Expand Up @@ -95,7 +97,8 @@ func (k Keeper) VerifyChannelState(
}

return clientState.VerifyChannelState(
k.cdc, height, connection.GetCounterparty().GetPrefix(), proof,
k.clientKeeper.ClientStore(ctx, connection.GetClientID()), k.cdc, height,
connection.GetCounterparty().GetPrefix(), proof,
portID, channelID, channel, consensusState,
)
}
Expand Down Expand Up @@ -129,7 +132,8 @@ func (k Keeper) VerifyPacketCommitment(
}

return clientState.VerifyPacketCommitment(
height, connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
k.clientKeeper.ClientStore(ctx, connection.GetClientID()), height,
connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
sequence, commitmentBytes, consensusState,
)
}
Expand Down Expand Up @@ -163,7 +167,8 @@ func (k Keeper) VerifyPacketAcknowledgement(
}

return clientState.VerifyPacketAcknowledgement(
height, connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
k.clientKeeper.ClientStore(ctx, connection.GetClientID()), height,
connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
sequence, acknowledgement, consensusState,
)
}
Expand Down Expand Up @@ -197,7 +202,8 @@ func (k Keeper) VerifyPacketAcknowledgementAbsence(
}

return clientState.VerifyPacketAcknowledgementAbsence(
height, connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
k.clientKeeper.ClientStore(ctx, connection.GetClientID()), height,
connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
sequence, consensusState,
)
}
Expand Down Expand Up @@ -230,7 +236,8 @@ func (k Keeper) VerifyNextSequenceRecv(
}

return clientState.VerifyNextSequenceRecv(
height, connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
k.clientKeeper.ClientStore(ctx, connection.GetClientID()), height,
connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
nextSequenceRecv, consensusState,
)
}
1 change: 1 addition & 0 deletions x/ibc/03-connection/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ type ClientKeeper interface {
GetClientConsensusState(ctx sdk.Context, clientID string, height uint64) (clientexported.ConsensusState, bool)
GetSelfConsensusState(ctx sdk.Context, height uint64) (clientexported.ConsensusState, bool)
IterateClients(ctx sdk.Context, cb func(clientexported.ClientState) bool)
ClientStore(ctx sdk.Context, clientID string) sdk.KVStore
}
7 changes: 7 additions & 0 deletions x/ibc/07-tendermint/types/client_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func (cs ClientState) Validate() error {
// VerifyClientConsensusState verifies a proof of the consensus state of the
// Tendermint client stored on the target machine.
func (cs ClientState) VerifyClientConsensusState(
_ sdk.KVStore,
cdc *codec.Codec,
provingRoot commitmentexported.Root,
height uint64,
Expand Down Expand Up @@ -169,6 +170,7 @@ func (cs ClientState) VerifyClientConsensusState(
// VerifyConnectionState verifies a proof of the connection state of the
// specified connection end stored on the target machine.
func (cs ClientState) VerifyConnectionState(
_ sdk.KVStore,
cdc codec.Marshaler,
height uint64,
prefix commitmentexported.Prefix,
Expand Down Expand Up @@ -206,6 +208,7 @@ func (cs ClientState) VerifyConnectionState(
// VerifyChannelState verifies a proof of the channel state of the specified
// channel end, under the specified port, stored on the target machine.
func (cs ClientState) VerifyChannelState(
_ sdk.KVStore,
cdc codec.Marshaler,
height uint64,
prefix commitmentexported.Prefix,
Expand Down Expand Up @@ -244,6 +247,7 @@ func (cs ClientState) VerifyChannelState(
// VerifyPacketCommitment verifies a proof of an outgoing packet commitment at
// the specified port, specified channel, and specified sequence.
func (cs ClientState) VerifyPacketCommitment(
_ sdk.KVStore,
height uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
Expand Down Expand Up @@ -272,6 +276,7 @@ func (cs ClientState) VerifyPacketCommitment(
// VerifyPacketAcknowledgement verifies a proof of an incoming packet
// acknowledgement at the specified port, specified channel, and specified sequence.
func (cs ClientState) VerifyPacketAcknowledgement(
_ sdk.KVStore,
height uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
Expand Down Expand Up @@ -301,6 +306,7 @@ func (cs ClientState) VerifyPacketAcknowledgement(
// incoming packet acknowledgement at the specified port, specified channel, and
// specified sequence.
func (cs ClientState) VerifyPacketAcknowledgementAbsence(
_ sdk.KVStore,
height uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
Expand Down Expand Up @@ -328,6 +334,7 @@ func (cs ClientState) VerifyPacketAcknowledgementAbsence(
// VerifyNextSequenceRecv verifies a proof of the next sequence number to be
// received of the specified channel at the specified port.
func (cs ClientState) VerifyNextSequenceRecv(
_ sdk.KVStore,
height uint64,
prefix commitmentexported.Prefix,
proof commitmentexported.Proof,
Expand Down
14 changes: 7 additions & 7 deletions x/ibc/07-tendermint/types/client_state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (suite *TendermintTestSuite) TestVerifyClientConsensusState() {
tc := tc

err := tc.clientState.VerifyClientConsensusState(
suite.aminoCdc, tc.consensusState.Root, height, "chainA", tc.consensusState.GetHeight(), tc.prefix, tc.proof, tc.consensusState,
nil, suite.aminoCdc, tc.consensusState.Root, height, "chainA", tc.consensusState.GetHeight(), tc.prefix, tc.proof, tc.consensusState,
)

if tc.expPass {
Expand Down Expand Up @@ -211,7 +211,7 @@ func (suite *TendermintTestSuite) TestVerifyConnectionState() {
tc := tc

err := tc.clientState.VerifyConnectionState(
suite.cdc, height, tc.prefix, tc.proof, testConnectionID, tc.connection, tc.consensusState,
nil, suite.cdc, height, tc.prefix, tc.proof, testConnectionID, tc.connection, tc.consensusState,
)

if tc.expPass {
Expand Down Expand Up @@ -294,7 +294,7 @@ func (suite *TendermintTestSuite) TestVerifyChannelState() {
tc := tc

err := tc.clientState.VerifyChannelState(
suite.cdc, height, tc.prefix, tc.proof, testPortID, testChannelID, &tc.channel, tc.consensusState,
nil, suite.cdc, height, tc.prefix, tc.proof, testPortID, testChannelID, &tc.channel, tc.consensusState,
)

if tc.expPass {
Expand Down Expand Up @@ -374,7 +374,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketCommitment() {
tc := tc

err := tc.clientState.VerifyPacketCommitment(
height, tc.prefix, tc.proof, testPortID, testChannelID, testSequence, tc.commitment, tc.consensusState,
nil, height, tc.prefix, tc.proof, testPortID, testChannelID, testSequence, tc.commitment, tc.consensusState,
)

if tc.expPass {
Expand Down Expand Up @@ -454,7 +454,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgement() {
tc := tc

err := tc.clientState.VerifyPacketAcknowledgement(
height, tc.prefix, tc.proof, testPortID, testChannelID, testSequence, tc.ack, tc.consensusState,
nil, height, tc.prefix, tc.proof, testPortID, testChannelID, testSequence, tc.ack, tc.consensusState,
)

if tc.expPass {
Expand Down Expand Up @@ -529,7 +529,7 @@ func (suite *TendermintTestSuite) TestVerifyPacketAcknowledgementAbsence() {
tc := tc

err := tc.clientState.VerifyPacketAcknowledgementAbsence(
height, tc.prefix, tc.proof, testPortID, testChannelID, testSequence, tc.consensusState,
nil, height, tc.prefix, tc.proof, testPortID, testChannelID, testSequence, tc.consensusState,
)

if tc.expPass {
Expand Down Expand Up @@ -604,7 +604,7 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() {
tc := tc

err := tc.clientState.VerifyNextSequenceRecv(
height, tc.prefix, tc.proof, testPortID, testChannelID, testSequence, tc.consensusState,
nil, height, tc.prefix, tc.proof, testPortID, testChannelID, testSequence, tc.consensusState,
)

if tc.expPass {
Expand Down
4 changes: 4 additions & 0 deletions x/ibc/07-tendermint/types/tendermint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (

"github.com/stretchr/testify/suite"

abci "github.com/tendermint/tendermint/abci/types"
tmtypes "github.com/tendermint/tendermint/types"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/simapp"
sdk "github.com/cosmos/cosmos-sdk/types"
ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types"
)

Expand All @@ -24,6 +26,7 @@ const (
type TendermintTestSuite struct {
suite.Suite

ctx sdk.Context
Copy link
Contributor

Choose a reason for hiding this comment

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

what was the purpose of adding this?

aminoCdc *codec.Codec
cdc codec.Marshaler
privVal tmtypes.PrivValidator
Expand All @@ -48,6 +51,7 @@ func (suite *TendermintTestSuite) SetupTest() {
val := tmtypes.NewValidator(pubKey, 10)
suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{val})
suite.header = ibctmtypes.CreateTestHeader(chainID, height, suite.now, suite.valSet, []tmtypes.PrivValidator{suite.privVal})
suite.ctx = app.BaseApp.NewContext(checkTx, abci.Header{Height: 1, Time: suite.now})
}

func TestTendermintTestSuite(t *testing.T) {
Expand Down
Loading