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

imp: remove kv generator and construct expected channel for both single and multi hop cases #45

Open
wants to merge 4 commits into
base: polymer/multihop-main
Choose a base branch
from
Open

imp: remove kv generator and construct expected channel for both single and multi hop cases #45

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bda9276
some improvements (naming, documentation, errors)
crodriguezvega Dec 1, 2023
46b8d05
Merge branch 'polymerdao:polymer/multihop-main' into polymer/multihop…
crodriguezvega Dec 5, 2023
17196d0
imp: remove kv generator and construct expected channel for both sing…
crodriguezvega Feb 2, 2024
17638e4
remove unused function
crodriguezvega Feb 2, 2024
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
267 changes: 115 additions & 152 deletions modules/core/04-channel/keeper/handshake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,72 +152,55 @@ func (k Keeper) ChanOpenTry(
)
}

// handle multihop case
var counterpartyHops []string
if len(connectionHops) > 1 {
kvGenerator := func(mProof *types.MsgMultihopProofs, lastHopConnectionEnd *connectiontypes.ConnectionEnd) (string, []byte, error) {
// check version support
if err := checkVersion(lastHopConnectionEnd, order); err != nil {
return "", nil, err
}

// channel end storekey of the counterparty channel on the other end of the multihop channel
key := host.ChannelPath(counterparty.PortId, counterparty.ChannelId)

// connection hops
counterpartyHops, err := mProof.GetCounterpartyConnectionHops(k.cdc, &connectionEnd)
if err != nil {
return "", nil, err
}

// expectedCounterparty is the counterparty of the counterparty's channel end (i.e self)
expectedCounterparty := types.NewCounterparty(portID, "")
expectedChannel := types.NewChannel(types.INIT, order, expectedCounterparty, counterpartyHops, counterpartyVersion)

// expected value bytes
bz, err := k.cdc.Marshal(&expectedChannel)
if err != nil {
return "", nil, err
}

return key, bz, nil
var multihopProof types.MsgMultihopProofs
if err := k.cdc.Unmarshal(proofInit, &multihopProof); err != nil {
return "", nil, err
}

if err := k.connectionKeeper.VerifyMultihopMembership(
ctx, connectionEnd, proofHeight, proofInit,
connectionHops, kvGenerator); err != nil {
// get the last hop connection on the other side of the multihop channel
// the last hop connection is the connection end on the chain before the counterparty multihop chain
lastHopConnectionEnd, err := multihopProof.GetLastHopConnectionEnd(k.cdc, connectionEnd)
Copy link
Author

Choose a reason for hiding this comment

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

There is still some leakage of the multi hop abstraction at 04-channel, but I couldn't see any way around it. Open to ideas.

Copy link
Member

Choose a reason for hiding this comment

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

I took a deeper look at this today and I think I see a path forward, but it is annoying due to import cycles. I am working on a branch to merge multi-hop and single hop logic into the existing VerifyXXX() functions by changing the single hop proof into a multi-hop proof. The multi-hop proof logic then degenerates into standard single hop proof logic.

However, I need to to some moderate refactoring to get this approach to work. I'll share a draft PR so you can see the approach hopefully next week. The main changes are to:

  • move multihop message definitions under commitment types (at least not under channel/tx types)
  • remove dependency on tmclient from the multihop verification code (need to break import cycle and shouldn't really be hardcoded to tm anyway...)
  • need to add connectionHops as arg to connection keeper interface
  • add a multihop membership verification method to client state interface and implement for tendermint

Hopefully I can do this in a way where I can pick just one verification function and show how it would look. This would also remove the key generator functions as well.

if err != nil {
return "", nil, err
}

} else {
// determine counterparty hops
counterpartyHops := []string{connectionEnd.GetCounterparty().GetConnectionID()}

// check version support
if err := checkVersion(&connectionEnd, order); err != nil {
if err := checkVersion(lastHopConnectionEnd, order); err != nil {
return "", nil, err
}

// expectedCounterparty is the counterparty of the counterparty's channel end (i.e self)
expectedCounterparty := types.NewCounterparty(portID, "")
expectedChannel := types.NewChannel(
types.INIT, order, expectedCounterparty,
counterpartyHops, counterpartyVersion,
)

if err := k.connectionKeeper.VerifyChannelState(
ctx, connectionEnd, proofHeight, proofInit,
counterparty.PortId, counterparty.ChannelId, expectedChannel,
); err != nil {
counterpartyHops, err = multihopProof.GetCounterpartyConnectionHops(k.cdc, &connectionEnd)
if err != nil {
return "", nil, err
}
} else {
// check version support
if err := checkVersion(&connectionEnd, order); err != nil {
return "", nil, err
}

counterpartyHops = []string{connectionEnd.GetCounterparty().GetConnectionID()}
}

var (
capKey *capabilitytypes.Capability
err error
expectedCounterparty := types.NewCounterparty(portID, "")
expectedChannel := types.NewChannel(
types.INIT,
order,
expectedCounterparty,
counterpartyHops,
counterpartyVersion,
)

capKey, err = k.scopedKeeper.NewCapability(ctx, host.ChannelCapabilityPath(portID, channelID))
if err := k.connectionKeeper.VerifyChannelState(
ctx, connectionEnd, proofHeight, proofInit,
counterparty.PortId, counterparty.ChannelId, expectedChannel,
); err != nil {
return "", nil, err
}

capKey, err := k.scopedKeeper.NewCapability(ctx, host.ChannelCapabilityPath(portID, channelID))
if err != nil {
return "", nil, errorsmod.Wrapf(err, "could not create channel capability for port ID %s and channel ID %s", portID, channelID)
}
Expand Down Expand Up @@ -288,46 +271,39 @@ func (k Keeper) ChanOpenAck(
"connection state is not OPEN (got %s)", connectiontypes.State(connectionEnd.GetState()).String(),
)
}
// verify multihop proof
if len(channel.ConnectionHops) > 1 {

kvGenerator := func(mProof *types.MsgMultihopProofs, _ *connectiontypes.ConnectionEnd) (string, []byte, error) {
key := host.ChannelPath(channel.Counterparty.PortId, counterpartyChannelID)

counterpartyHops, err := mProof.GetCounterpartyConnectionHops(k.cdc, &connectionEnd)
if err != nil {
return "", nil, err
}
expectedCounterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.TRYOPEN, channel.Ordering, expectedCounterparty,
counterpartyHops, counterpartyVersion,
)
value, err := expectedChannel.Marshal()
if err != nil {
return "", nil, err
}
return key, value, nil
var counterpartyHops []string
if len(channel.ConnectionHops) > 1 {
var (
err error
multihopProof types.MsgMultihopProofs
)
if err = k.cdc.Unmarshal(proofTry, &multihopProof); err != nil {
return err
}

return k.connectionKeeper.VerifyMultihopMembership(
ctx, connectionEnd, proofHeight, proofTry,
channel.ConnectionHops, kvGenerator)

counterpartyHops, err = multihopProof.GetCounterpartyConnectionHops(k.cdc, &connectionEnd)
if err != nil {
return err
}
} else {
counterpartyHops := []string{connectionEnd.GetCounterparty().GetConnectionID()}
expectedCounterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.TRYOPEN, channel.Ordering, expectedCounterparty,
counterpartyHops, counterpartyVersion,
)

return k.connectionKeeper.VerifyChannelState(
ctx, connectionEnd, proofHeight, proofTry,
channel.Counterparty.PortId, counterpartyChannelID,
expectedChannel,
)
counterpartyHops = []string{connectionEnd.GetCounterparty().GetConnectionID()}
}

// counterparty of the counterparty channel end (i.e self)
expectedCounterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.TRYOPEN,
channel.Ordering,
expectedCounterparty,
counterpartyHops,
counterpartyVersion,
)

return k.connectionKeeper.VerifyChannelState(
ctx, connectionEnd, proofHeight, proofTry,
channel.Counterparty.PortId, counterpartyChannelID,
expectedChannel)
}

// WriteOpenAckChannel writes an updated channel state for the successful OpenAck handshake step.
Expand Down Expand Up @@ -394,43 +370,38 @@ func (k Keeper) ChanOpenConfirm(
)
}

// verify multihop proof or standard proof
var counterpartyHops []string
Copy link
Author

Choose a reason for hiding this comment

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

The code below setting the counterparty hops is pretty much repeated in all handlers, so we could rug it under some common function.

if len(channel.ConnectionHops) > 1 {
kvGenerator := func(mProof *types.MsgMultihopProofs, _ *connectiontypes.ConnectionEnd) (string, []byte, error) {
key := host.ChannelPath(channel.Counterparty.PortId, channel.Counterparty.ChannelId)

counterpartyHops, err := mProof.GetCounterpartyConnectionHops(k.cdc, &connectionEnd)
if err != nil {
return "", nil, err
}
counterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.OPEN, channel.Ordering, counterparty,
counterpartyHops, channel.Version,
)
value, err := expectedChannel.Marshal()
if err != nil {
return "", nil, err
}
return key, value, nil
var (
err error
multihopProof types.MsgMultihopProofs
)
if err = k.cdc.Unmarshal(proofAck, &multihopProof); err != nil {
return err
}

return k.connectionKeeper.VerifyMultihopMembership(
ctx, connectionEnd, proofHeight, proofAck,
channel.ConnectionHops, kvGenerator)

counterpartyHops, err = multihopProof.GetCounterpartyConnectionHops(k.cdc, &connectionEnd)
if err != nil {
return err
}
} else {
counterpartyHops := []string{connectionEnd.GetCounterparty().GetConnectionID()}
counterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.OPEN, channel.Ordering, counterparty,
counterpartyHops, channel.Version)

return k.connectionKeeper.VerifyChannelState(
ctx, connectionEnd, proofHeight, proofAck,
channel.Counterparty.PortId, channel.Counterparty.ChannelId,
expectedChannel)
counterpartyHops = []string{connectionEnd.GetCounterparty().GetConnectionID()}
}

// counterparty of the counterparty channel end (i.e self)
expectedCounterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.OPEN,
channel.Ordering,
expectedCounterparty,
counterpartyHops,
channel.Version,
)

return k.connectionKeeper.VerifyChannelState(
ctx, connectionEnd, proofHeight, proofAck,
channel.Counterparty.PortId, channel.Counterparty.ChannelId,
expectedChannel)
}

// WriteOpenConfirmChannel writes an updated channel state for the successful OpenConfirm handshake step.
Expand Down Expand Up @@ -548,47 +519,39 @@ func (k Keeper) ChanCloseConfirm(
)
}

// verify multihop proof
var counterpartyHops []string
if len(channel.ConnectionHops) > 1 {

kvGenerator := func(mProof *types.MsgMultihopProofs, _ *connectiontypes.ConnectionEnd) (string, []byte, error) {
key := host.ChannelPath(channel.Counterparty.PortId, channel.Counterparty.ChannelId)

counterpartyHops, err := mProof.GetCounterpartyConnectionHops(k.cdc, &connectionEnd)
if err != nil {
return "", nil, err
}
counterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.CLOSED, channel.Ordering, counterparty,
counterpartyHops, channel.Version,
)
value, err := expectedChannel.Marshal()
if err != nil {
return "", nil, err
}
return key, value, nil
}

if err := k.connectionKeeper.VerifyMultihopMembership(
ctx, connectionEnd, proofHeight, proofInit,
channel.ConnectionHops, kvGenerator); err != nil {
var (
err error
multihopProof types.MsgMultihopProofs
)
if err = k.cdc.Unmarshal(proofInit, &multihopProof); err != nil {
return err
}

} else {
counterpartyHops := []string{connectionEnd.GetCounterparty().GetConnectionID()}
counterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.CLOSED, channel.Ordering, counterparty,
counterpartyHops, channel.Version)

if err := k.connectionKeeper.VerifyChannelState(
ctx, connectionEnd, proofHeight, proofInit,
channel.Counterparty.PortId, channel.Counterparty.ChannelId,
expectedChannel); err != nil {
counterpartyHops, err = multihopProof.GetCounterpartyConnectionHops(k.cdc, &connectionEnd)
if err != nil {
return err
}
} else {
counterpartyHops = []string{connectionEnd.GetCounterparty().GetConnectionID()}
}

// counterparty of the counterparty channel end (i.e self)
expectedCounterparty := types.NewCounterparty(portID, channelID)
expectedChannel := types.NewChannel(
types.CLOSED,
channel.Ordering,
expectedCounterparty,
counterpartyHops,
channel.Version,
)

if err := k.connectionKeeper.VerifyChannelState(
ctx, connectionEnd, proofHeight, proofInit,
channel.Counterparty.PortId, channel.Counterparty.ChannelId,
expectedChannel); err != nil {
return err
}

k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", channel.State.String(), "new-state", types.CLOSED.String())
Expand Down
Loading