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.

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

ibc: async acknowledgements #7361

Merged
merged 24 commits into from Oct 1, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions proto/ibc/lightclients/solomachine/v1/solomachine.proto
Expand Up @@ -27,7 +27,7 @@ message ClientState {
message ConsensusState {
option (gogoproto.goproto_getters) = false;
// public key of the solo machine
google.protobuf.Any public_key = 1 [(gogoproto.moretags) = "yaml:\"public_key\""];
google.protobuf.Any public_key = 1 [(gogoproto.moretags) = "yaml:\"public_key\""];
// diversifier allows the same public key to be re-used across different solo machine clients
// (potentially on different chains) without being considered misbehaviour.
string diversifier = 2;
Expand All @@ -38,11 +38,11 @@ message ConsensusState {
message Header {
option (gogoproto.goproto_getters) = false;
// sequence to update solo machine public key at
uint64 sequence = 1;
uint64 timestamp = 2;
bytes signature = 3;
uint64 sequence = 1;
uint64 timestamp = 2;
bytes signature = 3;
google.protobuf.Any new_public_key = 4 [(gogoproto.moretags) = "yaml:\"new_public_key\""];
string new_diversifier = 5 [(gogoproto.moretags) = "yaml:\"new_diversifier\""];
string new_diversifier = 5 [(gogoproto.moretags) = "yaml:\"new_diversifier\""];
}

// Misbehaviour defines misbehaviour for a solo machine which consists
Expand Down Expand Up @@ -142,9 +142,9 @@ message PacketAcknowledgementData {
bytes acknowledgement = 2;
}

// PacketAcknowledgementAbsenceSignBytes returns the SignBytes data for
// acknowledgement absence verification.
message PacketAcknowledgementAbsenseData {
// PacketReceiptAbsenceSignBytes returns the SignBytes data for
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
// packet receipt absence verification.
message PacketReceiptAbsenseData {
bytes path = 1;
}

Expand Down
14 changes: 1 addition & 13 deletions x/ibc-transfer/keeper/keeper.go
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/cosmos/cosmos-sdk/x/ibc-transfer/types"
channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types"
host "github.com/cosmos/cosmos-sdk/x/ibc/24-host"
ibcexported "github.com/cosmos/cosmos-sdk/x/ibc/exported"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
)

Expand Down Expand Up @@ -72,19 +71,8 @@ func (k Keeper) GetTransferAccount(ctx sdk.Context) authtypes.ModuleAccountI {
return k.authKeeper.GetModuleAccount(ctx, types.ModuleName)
}

// ReceiveExecuted defines a wrapper function for the channel Keeper's function
// in order to expose it to the ICS20 transfer handler.
// Keeper retrieves channel capability and passes it into channel keeper for authentication
func (k Keeper) ReceiveExecuted(ctx sdk.Context, packet ibcexported.PacketI, acknowledgement []byte) error {
chanCap, ok := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(packet.GetDestPort(), packet.GetDestChannel()))
if !ok {
return sdkerrors.Wrap(channeltypes.ErrChannelCapabilityNotFound, "channel capability could not be retrieved for packet")
}
return k.channelKeeper.ReceiveExecuted(ctx, chanCap, packet, acknowledgement)
}

// ChanCloseInit defines a wrapper function for the channel Keeper's function
// in order to expose it to the ICS20 trasfer handler.
// in order to expose it to the ICS20 transfer handler.
func (k Keeper) ChanCloseInit(ctx sdk.Context, portID, channelID string) error {
capName := host.ChannelCapabilityPath(portID, channelID)
chanCap, ok := k.scopedKeeper.GetCapability(ctx, capName)
Expand Down
1 change: 1 addition & 0 deletions x/ibc-transfer/module.go
Expand Up @@ -323,6 +323,7 @@ func (am AppModule) OnRecvPacket(
),
)

// NOTE: acknowledgement will be written synchronously during IBC handler execution.
return &sdk.Result{
Events: ctx.EventManager().Events().ToABCIEvents(),
}, acknowledgement.GetBytes(), nil
Expand Down
1 change: 0 additions & 1 deletion x/ibc-transfer/types/expected_keepers.go
Expand Up @@ -29,7 +29,6 @@ type ChannelKeeper interface {
GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channeltypes.Channel, found bool)
GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool)
SendPacket(ctx sdk.Context, channelCap *capabilitytypes.Capability, packet ibcexported.PacketI) error
ReceiveExecuted(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement []byte) error
ChanCloseInit(ctx sdk.Context, portID, channelID string, chanCap *capabilitytypes.Capability) error
}

Expand Down
2 changes: 1 addition & 1 deletion x/ibc/02-client/types/errors.go
Expand Up @@ -23,7 +23,7 @@ var (
ErrFailedChannelStateVerification = sdkerrors.Register(SubModuleName, 16, "channel state verification failed")
ErrFailedPacketCommitmentVerification = sdkerrors.Register(SubModuleName, 17, "packet commitment verification failed")
ErrFailedPacketAckVerification = sdkerrors.Register(SubModuleName, 18, "packet acknowledgement verification failed")
ErrFailedPacketAckAbsenceVerification = sdkerrors.Register(SubModuleName, 19, "packet acknowledgement absence verification failed")
ErrFailedPacketReceiptVerification = sdkerrors.Register(SubModuleName, 19, "packet receipt verification failed")
ErrFailedNextSeqRecvVerification = sdkerrors.Register(SubModuleName, 20, "next sequence receive verification failed")
ErrSelfConsensusStateNotFound = sdkerrors.Register(SubModuleName, 21, "self consensus state not found")
ErrUpdateClientFailed = sdkerrors.Register(SubModuleName, 22, "unable to update light client")
Expand Down
8 changes: 4 additions & 4 deletions x/ibc/03-connection/keeper/verify.go
Expand Up @@ -175,10 +175,10 @@ func (k Keeper) VerifyPacketAcknowledgement(
return nil
}

// VerifyPacketAcknowledgementAbsence verifies a proof of the absence of an
// incoming packet acknowledgement at the specified port, specified channel, and
// VerifyPacketReceiptAbsence verifies a proof of the absence of an
// incoming packet receipt at the specified port, specified channel, and
// specified sequence.
func (k Keeper) VerifyPacketAcknowledgementAbsence(
func (k Keeper) VerifyPacketReceiptAbsence(
ctx sdk.Context,
connection exported.ConnectionI,
height exported.Height,
Expand All @@ -192,7 +192,7 @@ func (k Keeper) VerifyPacketAcknowledgementAbsence(
return sdkerrors.Wrap(clienttypes.ErrClientNotFound, connection.GetClientID())
}

if err := clientState.VerifyPacketAcknowledgementAbsence(
if err := clientState.VerifyPacketReceiptAbsence(
k.clientKeeper.ClientStore(ctx, connection.GetClientID()), k.cdc, height,
connection.GetCounterparty().GetPrefix(), proof, portID, channelID,
sequence,
Expand Down
15 changes: 9 additions & 6 deletions x/ibc/03-connection/keeper/verify_test.go
Expand Up @@ -336,7 +336,10 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgement() {
err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB)
suite.Require().NoError(err)

err = suite.coordinator.ReceiveExecuted(suite.chainB, suite.chainA, packet, clientA)
err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA)
suite.Require().NoError(err)

err = suite.coordinator.WriteAcknowledgement(suite.chainB, suite.chainA, packet, clientA)
suite.Require().NoError(err)

packetAckKey := host.KeyPacketAcknowledgement(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
Expand All @@ -361,10 +364,10 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgement() {
}
}

// TestVerifyPacketAcknowledgementAbsence has chainA verify the acknowledgement
// TestVerifyPacketReceiptAbsence has chainA verify the receipt
// absence on channelB. The channels on chainA and chainB are fully opened and
// a packet is sent from chainA to chainB and not received.
func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgementAbsence() {
func (suite *KeeperTestSuite) TestVerifyPacketReceiptAbsence() {
cases := []struct {
msg string
changeClientID bool
Expand Down Expand Up @@ -396,7 +399,7 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgementAbsence() {
suite.Require().NoError(err)

if tc.recvAck {
err = suite.coordinator.ReceiveExecuted(suite.chainB, suite.chainA, packet, clientA)
err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA)
suite.Require().NoError(err)
} else {
// need to update height to prove absence
Expand All @@ -407,7 +410,7 @@ func (suite *KeeperTestSuite) TestVerifyPacketAcknowledgementAbsence() {
packetAckKey := host.KeyPacketAcknowledgement(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
proof, proofHeight := suite.chainB.QueryProof(packetAckKey)

err = suite.chainA.App.IBCKeeper.ConnectionKeeper.VerifyPacketAcknowledgementAbsence(
err = suite.chainA.App.IBCKeeper.ConnectionKeeper.VerifyPacketReceiptAbsence(
suite.chainA.GetContext(), connection, malleateHeight(proofHeight, tc.heightDiff), proof,
packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(),
)
Expand Down Expand Up @@ -455,7 +458,7 @@ func (suite *KeeperTestSuite) TestVerifyNextSequenceRecv() {
err := suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB)
suite.Require().NoError(err)

err = suite.coordinator.ReceiveExecuted(suite.chainB, suite.chainA, packet, clientA)
err = suite.coordinator.WriteReceipt(suite.chainB, suite.chainA, packet, clientA)
suite.Require().NoError(err)

nextSeqRecvKey := host.KeyNextSequenceRecv(packet.GetDestPort(), packet.GetDestChannel())
Expand Down
23 changes: 23 additions & 0 deletions x/ibc/04-channel/keeper/keeper.go
Expand Up @@ -125,6 +125,23 @@ func (k Keeper) SetNextSequenceAck(ctx sdk.Context, portID, channelID string, se
store.Set(host.KeyNextSequenceAck(portID, channelID), bz)
}

// GetPacketReceipt gets a packet receipt from the store
func (k Keeper) GetPacketReceipt(ctx sdk.Context, portID, channelID string, sequence uint64) (string, bool) {
store := ctx.KVStore(k.storeKey)
bz := store.Get(host.KeyPacketReceipt(portID, channelID, sequence))
if bz == nil {
return "", false
}

return string(bz), true
}

// SetPacketReceipt sets an empty packet receipt to the store
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
func (k Keeper) SetPacketReceipt(ctx sdk.Context, portID, channelID string, sequence uint64) {
store := ctx.KVStore(k.storeKey)
store.Set(host.KeyPacketReceipt(portID, channelID, sequence), []byte(""))
}

// GetPacketCommitment gets the packet commitment hash from the store
func (k Keeper) GetPacketCommitment(ctx sdk.Context, portID, channelID string, sequence uint64) []byte {
store := ctx.KVStore(k.storeKey)
Expand Down Expand Up @@ -165,6 +182,12 @@ func (k Keeper) GetPacketAcknowledgement(ctx sdk.Context, portID, channelID stri
return bz, true
}

// HasPacketAcknowledgement check if the packet ack hash is already on the store
func (k Keeper) HasPacketAcknowledgement(ctx sdk.Context, portID, channelID string, sequence uint64) bool {
store := ctx.KVStore(k.storeKey)
return store.Has(host.KeyPacketAcknowledgement(portID, channelID, sequence))
}

// IteratePacketSequence provides an iterator over all send, receive or ack sequences.
// For each sequence, cb will be called. If the cb returns true, the iterator
// will close and stop.
Expand Down
9 changes: 5 additions & 4 deletions x/ibc/04-channel/keeper/keeper_test.go
Expand Up @@ -291,13 +291,14 @@ func (suite *KeeperTestSuite) TestSetPacketAcknowledgement() {
seq := uint64(10)

storedAckHash, found := suite.chainA.App.IBCKeeper.ChannelKeeper.GetPacketAcknowledgement(ctxA, channelA.PortID, channelA.ID, seq)
suite.False(found)
suite.Nil(storedAckHash)
suite.Require().False(found)
suite.Require().Nil(storedAckHash)

ackHash := []byte("ackhash")
suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketAcknowledgement(ctxA, channelA.PortID, channelA.ID, seq, ackHash)

storedAckHash, found = suite.chainA.App.IBCKeeper.ChannelKeeper.GetPacketAcknowledgement(ctxA, channelA.PortID, channelA.ID, seq)
suite.True(found)
suite.Equal(ackHash, storedAckHash)
suite.Require().True(found)
suite.Require().Equal(ackHash, storedAckHash)
suite.Require().True(suite.chainA.App.IBCKeeper.ChannelKeeper.HasPacketAcknowledgement(ctxA, channelA.PortID, channelA.ID, seq))
}
95 changes: 71 additions & 24 deletions x/ibc/04-channel/keeper/packet.go
Expand Up @@ -201,19 +201,19 @@ func (k Keeper) RecvPacket(
)
}

// check if the packet acknowledgement has been received already for unordered channels
if channel.Ordering == types.UNORDERED {
switch channel.Ordering {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
case types.UNORDERED:
// check if the packet acknowledgement has been received already for unordered channels
_, found := k.GetPacketAcknowledgement(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we be checking for packet receipts now instead of packet acknowledgements?

Copy link
Contributor

Choose a reason for hiding this comment

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

if found {
return sdkerrors.Wrapf(
types.ErrInvalidPacket,
"packet sequence (%d) already has been received", packet.GetSequence(),
)
}
}

// check if the packet is being received in order
if channel.Ordering == types.ORDERED {
case types.ORDERED:
// check if the packet is being received in order
nextSequenceRecv, found := k.GetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel())
if !found {
return sdkerrors.Wrapf(
Expand All @@ -238,19 +238,18 @@ func (k Keeper) RecvPacket(
return sdkerrors.Wrap(err, "couldn't verify counterparty packet commitment")
}

// NOTE: the remaining code is located in the ReceiveExecuted function
// NOTE: the remaining code is located in the WriteReceipt function
return nil
}

// ReceiveExecuted writes the packet execution acknowledgement to the state,
// which will be verified by the counterparty chain using AcknowledgePacket.
// WriteReceipt updates the receive sequence in the case of an ordered channel or sets an empty receipt
// if the channel is unordered.
//
// CONTRACT: this function must be called in the IBC handler
func (k Keeper) ReceiveExecuted(
func (k Keeper) WriteReceipt(
ctx sdk.Context,
chanCap *capabilitytypes.Capability,
packet exported.PacketI,
acknowledgement []byte,
) error {
channel, found := k.GetChannel(ctx, packet.GetDestPort(), packet.GetDestChannel())
if !found {
Expand All @@ -273,16 +272,6 @@ func (k Keeper) ReceiveExecuted(
)
}

if len(acknowledgement) == 0 {
return sdkerrors.Wrap(types.ErrInvalidAcknowledgement, "acknowledgement cannot be empty")
}

// always set the acknowledgement so that it can be verified on the other side
k.SetPacketAcknowledgement(
ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(),
types.CommitAcknowledgement(acknowledgement),
)

if channel.Ordering == types.ORDERED {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
nextSequenceRecv, found := k.GetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel())
if !found {
Expand All @@ -294,20 +283,32 @@ func (k Keeper) ReceiveExecuted(

nextSequenceRecv++

AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
// incrementng nextSequenceRecv and storing under this chain's channelEnd identifiers
// incrementing nextSequenceRecv and storing under this chain's channelEnd identifiers
// Since this is the receiving chain, our channelEnd is packet's destination port and channel
k.SetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel(), nextSequenceRecv)
} else {
// For unordered channels we must set the receipt so it can be verified on the other side.
// This receipt does not contain any data, since the packet has not yet been processed,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
// it's just a single store key set to an empty string to indicate that the packet has been received
_, found := k.GetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
if found {
return sdkerrors.Wrapf(
types.ErrPacketReceived,
"destination port: %s, destination channel: %s, sequence: %d", packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(),
)
}

k.SetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
}

// log that a packet has been received & executed
k.Logger(ctx).Info(fmt.Sprintf("packet received & executed: %v", packet))
k.Logger(ctx).Info("packet received", "packet", fmt.Sprintf("%v", packet))

// emit an event that the relayer can query for
ctx.EventManager().EmitEvents(sdk.Events{
sdk.NewEvent(
types.EventTypeRecvPacket,
sdk.NewAttribute(types.AttributeKeyData, string(packet.GetData())),
sdk.NewAttribute(types.AttributeKeyAck, string(acknowledgement)),
sdk.NewAttribute(types.AttributeKeyTimeoutHeight, packet.GetTimeoutHeight().String()),
sdk.NewAttribute(types.AttributeKeyTimeoutTimestamp, fmt.Sprintf("%d", packet.GetTimeoutTimestamp())),
sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
Expand All @@ -326,6 +327,52 @@ func (k Keeper) ReceiveExecuted(
return nil
}

// WriteAcknowledgement writes the packet execution acknowledgement to the state,
// which will be verified by the counterparty chain using AcknowledgePacket.
//
// CONTRACT: for synchronous execution, this function is be called in the IBC handler .
// For async handling, it needs to be called directly by the module which originally
// processed the packet.
func (k Keeper) WriteAcknowledgement(
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
ctx sdk.Context,
packet exported.PacketI,
acknowledgement []byte,
) error {
// NOTE: IBC app modules might have written the acknowledgement synchronously on
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
// the OnRecvPacket callback so we need to check if the acknowledgement is already
// set on the store and perform a no-op if so.
if k.HasPacketAcknowledgement(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) {
return nil
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
}

if len(acknowledgement) == 0 {
return sdkerrors.Wrap(types.ErrInvalidAcknowledgement, "acknowledgement cannot be empty")
}

// always set the acknowledgement so that it can be verified on the other side
k.SetPacketAcknowledgement(
ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(),
types.CommitAcknowledgement(acknowledgement),
)

// log that a packet has been acknowledged
k.Logger(ctx).Info("packet acknowledged", "packet", fmt.Sprintf("%v", packet))

// emit an event that the relayer can query for
ctx.EventManager().EmitEvents(sdk.Events{
sdk.NewEvent(
types.EventTypeRecvPacket,
sdk.NewAttribute(types.AttributeKeyAck, string(acknowledgement)),
),
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
),
})

return nil
}

// AcknowledgePacket is called by a module to process the acknowledgement of a
// packet previously sent by the calling module on a channel to a counterparty
// module on the counterparty chain. Its intended usage is within the ante
Expand Down Expand Up @@ -459,7 +506,7 @@ func (k Keeper) AcknowledgementExecuted(

nextSequenceAck++

// incrementng NextSequenceAck and storing under this chain's channelEnd identifiers
// incrementing NextSequenceAck and storing under this chain's channelEnd identifiers
// Since this is the original sending chain, our channelEnd is packet's source port and channel
k.SetNextSequenceAck(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), nextSequenceAck)
}
Expand Down