From fab004b2b8e141c70e8331de0817b3c439ad48f2 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Tue, 14 Jan 2025 11:41:51 -0600 Subject: [PATCH 1/6] fix(vtransfer): some tweaks for ack handling --- golang/cosmos/types/address_hooks.go | 42 ++++ golang/cosmos/x/vtransfer/ibc_middleware.go | 6 +- golang/cosmos/x/vtransfer/keeper/keeper.go | 217 +++++++++++++++++--- 3 files changed, 237 insertions(+), 28 deletions(-) diff --git a/golang/cosmos/types/address_hooks.go b/golang/cosmos/types/address_hooks.go index 073e9d48cf4..5ef5a5302df 100644 --- a/golang/cosmos/types/address_hooks.go +++ b/golang/cosmos/types/address_hooks.go @@ -14,11 +14,15 @@ import ( ) type AddressRole string +type PacketOrigin string const ( RoleSender AddressRole = "Sender" RoleReceiver AddressRole = "Receiver" + PacketSrc PacketOrigin = "src" + PacketDst PacketOrigin = "dst" + AddressHookVersion = 0 BaseAddressLengthBytes = 2 ) @@ -211,3 +215,41 @@ func ExtractBaseAddressFromPacket(cdc codec.Codec, packet ibcexported.PacketI, r return target, nil } + +// ExtractBaseAddressFromData returns the base address from a transfer packet's data, +// either Sender (if role is RoleSender) or Receiver (if role is RoleReceiver). +// Errors in determining the base address are ignored... we then assume the base +// address is exactly the original address. +// If newDataP is not nil, it is populated with new transfer packet data whose +// corresponding Sender or Receiver is replaced with the extracted base address. +func ExtractBaseAddressFromData(cdc codec.Codec, data []byte, role AddressRole, newDataP *[]byte) (string, error) { + transferData := transfertypes.FungibleTokenPacketData{} + + if err := cdc.UnmarshalJSON(data, &transferData); err != nil { + return "", err + } + + var newTransferData *transfertypes.FungibleTokenPacketData + if newDataP != nil { + // Capture the transfer data for the new packet data. + newTransferData = &transfertypes.FungibleTokenPacketData{} + } + target, err := extractBaseTransferData(transferData, role, newTransferData) + if err != nil { + return target, err + } + + if newDataP == nil { + return target, nil + } + + // Reuse the original data if we didn't transform it. + if transferData == *newTransferData { + *newDataP = bytes.Clone(data) + } else { + // Re-serialize the packet data with the new base address. + *newDataP = newTransferData.GetBytes() + } + + return target, nil +} diff --git a/golang/cosmos/x/vtransfer/ibc_middleware.go b/golang/cosmos/x/vtransfer/ibc_middleware.go index b47c76c2a63..fc8556401c1 100644 --- a/golang/cosmos/x/vtransfer/ibc_middleware.go +++ b/golang/cosmos/x/vtransfer/ibc_middleware.go @@ -160,7 +160,11 @@ func (im IBCMiddleware) WriteAcknowledgement( packet exported.PacketI, ack exported.Acknowledgement, ) error { - return im.vtransferKeeper.InterceptWriteAcknowledgement(ctx, chanCap, packet, ack) + syncAck, origPacket := im.vtransferKeeper.InterceptWriteAcknowledgement(ctx, chanCap, packet, ack) + if syncAck != nil { + return im.vtransferKeeper.WriteAcknowledgement(ctx, chanCap, origPacket, syncAck) + } + return nil } /////////////////////////////////// diff --git a/golang/cosmos/x/vtransfer/keeper/keeper.go b/golang/cosmos/x/vtransfer/keeper/keeper.go index ed5b0f57cba..cec0d937257 100644 --- a/golang/cosmos/x/vtransfer/keeper/keeper.go +++ b/golang/cosmos/x/vtransfer/keeper/keeper.go @@ -1,8 +1,10 @@ package keeper import ( + "bytes" "context" "encoding/json" + "fmt" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" @@ -18,6 +20,7 @@ import ( "github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc" vibctypes "github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc/types" + clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v6/modules/core/24-host" @@ -25,6 +28,7 @@ import ( ) var _ porttypes.ICS4Wrapper = (*Keeper)(nil) +var _ porttypes.ICS4Wrapper = (*ics4Wrapper)(nil) var _ vibctypes.ReceiverImpl = (*Keeper)(nil) var _ vm.PortHandler = (*Keeper)(nil) @@ -33,6 +37,7 @@ var _ vm.PortHandler = (*Keeper)(nil) // the address, and its corresponding value is a non-empty but otherwise irrelevant // sentinel. const ( + packetDataStoreKeyPrefix = "originalData/" watchedAddressStoreKeyPrefix = "watchedAddress/" watchedAddressSentinel = "y" ) @@ -53,6 +58,84 @@ type Keeper struct { cdc codec.Codec vibcModule porttypes.IBCModule + + // This is a pointer so that copies of the Keeper struct share the same mutable debug options. + debug *KeeperDebugOptions +} + +type PacketDataOverrider func(ctx sdk.Context, cdc codec.Codec, data []byte) ([]byte, error) +type KeeperDebugOptions struct { + OverridePacket PacketDataOverrider + DoNotStore bool +} + +type ics4Wrapper struct { + porttypes.ICS4Wrapper + k Keeper +} + +func (i4 *ics4Wrapper) SendPacket( + ctx sdk.Context, + chanCap *capabilitytypes.Capability, + sourcePort string, + sourceChannel string, + timeoutHeight clienttypes.Height, + timeoutTimestamp uint64, + data []byte, +) (sequence uint64, err error) { + // Permute the packet data when testing. + overridePacket := i4.k.debug.OverridePacket + if overridePacket != nil { + if data, err = overridePacket(ctx, i4.k.cdc, data); err != nil { + return sequence, err + } + } + + var strippedData []byte + _, err = types.ExtractBaseAddressFromData(i4.k.cdc, data, types.RoleSender, &strippedData) + if err != nil { + return sequence, err + } + + // Send the stripped data to the next wrapper. + sequence, err = i4.ICS4Wrapper.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, strippedData) + if err != nil { + return sequence, err + } + + // Store the original data if it is hooked for later retrieval by middleware. + if !i4.k.debug.DoNotStore && !bytes.Equal(strippedData, data) { + packetStore, packetKey := i4.k.PacketStore(ctx, types.PacketSrc, sourcePort, sourceChannel, sequence) + packetStore.Set(packetKey, data) + } + + return sequence, nil +} + +func (i4 *ics4Wrapper) WriteAcknowledgement( + ctx sdk.Context, + chanCap *capabilitytypes.Capability, + packet ibcexported.PacketI, + ack ibcexported.Acknowledgement, +) error { + origPacket := channeltypes.NewPacket( + packet.GetData(), packet.GetSequence(), + packet.GetSourcePort(), packet.GetSourceChannel(), + packet.GetDestPort(), packet.GetDestChannel(), + clienttypes.MustParseHeight(packet.GetTimeoutHeight().String()), + packet.GetTimeoutTimestamp(), + ) + packetStore, packetKey := i4.k.PacketStoreFromOrigin(ctx, types.PacketDst, packet) + if packetStore.Has(packetKey) { + origPacket.Data = packetStore.Get(packetKey) + packetStore.Delete(packetKey) + } + return i4.ICS4Wrapper.WriteAcknowledgement(ctx, chanCap, origPacket, ack) +} + +// NewICS4Wrapper creates a new ICS4Wrapper instance +func NewICS4Wrapper(k Keeper, down porttypes.ICS4Wrapper) *ics4Wrapper { + return &ics4Wrapper{k: k, ICS4Wrapper: down} } // NewKeeper creates a new vtransfer Keeper instance @@ -68,15 +151,26 @@ func NewKeeper( // This vibcKeeper is used to send notifications from the vtransfer middleware // to the VM. vibcKeeper := prototypeVibcKeeper.WithScope(nil, scopedTransferKeeper, wrappedPushAction) - return Keeper{ - ICS4Wrapper: vibcKeeper, + k := Keeper{ ReceiverImpl: vibcKeeper, vibcKeeper: vibcKeeper, key: key, vibcModule: vibc.NewIBCModule(vibcKeeper), cdc: cdc, + + debug: &KeeperDebugOptions{ + OverridePacket: nil, + DoNotStore: false, + }, } + k.ICS4Wrapper = NewICS4Wrapper(k, vibcKeeper) + return k +} + +func (k Keeper) SetDebugging(doStore bool, overridePacket PacketDataOverrider) { + k.debug.DoNotStore = !doStore + k.debug.OverridePacket = overridePacket } // wrapActionPusher wraps an ActionPusher to prefix the action type with @@ -102,6 +196,42 @@ func (k Keeper) GetReceiverImpl() vibctypes.ReceiverImpl { return k } +// Replicated from the 24-host ibc-go package, since it wasn't exported from there. +func channelPath(portID, channelID string) string { + return fmt.Sprintf("%s/%s/%s/%s", host.KeyPortPrefix, portID, host.KeyChannelPrefix, channelID) +} + +// Replicated from the 24-host ibc-go package, since it wasn't exported from there. +func sequencePath(sequence uint64) string { + return fmt.Sprintf("%s/%d", host.KeySequencePrefix, sequence) +} + +// PacketStore returns a new KVStore for storing packet data, and a key for +// that store. The KVStore is divided into src or dst PacketOrigins because we +// need to record separate data for packets travelling in each direction. +func (k Keeper) PacketStore(ctx sdk.Context, ourOrigin types.PacketOrigin, ourPort string, ourChannel string, sequence uint64) (storetypes.KVStore, []byte) { + key := fmt.Sprintf("%s/%s/%s", ourOrigin, channelPath(ourPort, ourChannel), sequencePath(sequence)) + packetKey := []byte(key) + return prefix.NewStore(ctx.KVStore(k.key), []byte(packetDataStoreKeyPrefix)), packetKey +} + +func (k Keeper) PacketStoreFromOrigin(ctx sdk.Context, ourOrigin types.PacketOrigin, packet ibcexported.PacketI) (storetypes.KVStore, []byte) { + var ourPort, ourChannel string + + switch ourOrigin { + case types.PacketSrc: + ourPort = packet.GetSourcePort() + ourChannel = packet.GetSourceChannel() + case types.PacketDst: + ourPort = packet.GetDestPort() + ourChannel = packet.GetDestChannel() + default: + panic("unknown packet origin " + ourOrigin) + } + + return k.PacketStore(ctx, ourOrigin, ourPort, ourChannel, packet.GetSequence()) +} + // InterceptOnRecvPacket runs the ibcModule and eventually acknowledges a packet. // Many error acknowledgments are sent synchronously, but most cases instead return nil // to tell the IBC system that acknowledgment is async (i.e., that WriteAcknowledgement @@ -113,12 +243,7 @@ func (k Keeper) InterceptOnRecvPacket(ctx sdk.Context, ibcModule porttypes.IBCMo if err != nil { return channeltypes.NewErrorAcknowledgement(err) } - ack := ibcModule.OnRecvPacket(ctx, strippedPacket, relayer) - if ack == nil { - // Already declared to be an async ack. - return nil - } portID := packet.GetDestPort() channelID := packet.GetDestChannel() capName := host.ChannelCapabilityPath(portID, channelID) @@ -127,11 +252,21 @@ func (k Keeper) InterceptOnRecvPacket(ctx sdk.Context, ibcModule porttypes.IBCMo err := sdkerrors.Wrapf(channeltypes.ErrChannelCapabilityNotFound, "could not retrieve channel capability at: %s", capName) return channeltypes.NewErrorAcknowledgement(err) } - // Give the VM a chance to write (or override) the ack. - if err := k.InterceptWriteAcknowledgement(ctx, chanCap, packet, ack); err != nil { - return channeltypes.NewErrorAcknowledgement(err) + + if !k.debug.DoNotStore && !bytes.Equal(strippedPacket.GetData(), packet.GetData()) { + packetStore, packetKey := k.PacketStore(ctx, types.PacketDst, portID, channelID, packet.GetSequence()) + packetStore.Set(packetKey, packet.GetData()) } - return nil + + ack := ibcModule.OnRecvPacket(ctx, strippedPacket, relayer) + if ack == nil { + // Already declared to be an async ack. Will be cleaned up by ics4Wrapper.WriteAcknowledgement. + return nil + } + + // Give the VM a chance to write (or override) the ack. + syncAck, _ := k.InterceptWriteAcknowledgement(ctx, chanCap, packet, ack) + return syncAck } // InterceptOnAcknowledgementPacket checks to see if the packet sender is a @@ -143,13 +278,19 @@ func (k Keeper) InterceptOnAcknowledgementPacket( acknowledgement []byte, relayer sdk.AccAddress, ) error { - // Pass every (stripped-sender) acknowledgement to the wrapped IBC module. - var strippedPacket channeltypes.Packet - baseSender, err := types.ExtractBaseAddressFromPacket(k.cdc, packet, types.RoleSender, &strippedPacket) + baseSender, err := types.ExtractBaseAddressFromData(k.cdc, packet.GetData(), types.RoleSender, nil) if err != nil { return err } - modErr := ibcModule.OnAcknowledgementPacket(ctx, strippedPacket, acknowledgement, relayer) + + origPacket := packet + packetStore, packetKey := k.PacketStoreFromOrigin(ctx, types.PacketSrc, packet) + if packetStore.Has(packetKey) { + origPacket.Data = packetStore.Get(packetKey) + packetStore.Delete(packetKey) + } + + modErr := ibcModule.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) // If the sender is not a watched account, we're done. if !k.targetIsWatched(ctx, baseSender) { @@ -157,7 +298,7 @@ func (k Keeper) InterceptOnAcknowledgementPacket( } // Trigger VM with the original packet, regardless of errors in the ibcModule. - vmErr := k.vibcKeeper.TriggerOnAcknowledgementPacket(ctx, baseSender, packet, acknowledgement, relayer) + vmErr := k.vibcKeeper.TriggerOnAcknowledgementPacket(ctx, baseSender, origPacket, acknowledgement, relayer) // Any error from the VM is trumped by one from the wrapped IBC module. if modErr != nil { @@ -174,13 +315,20 @@ func (k Keeper) InterceptOnTimeoutPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) error { - // Pass every (stripped-sender) timeout to the wrapped IBC module. - var strippedPacket channeltypes.Packet - baseSender, err := types.ExtractBaseAddressFromPacket(k.cdc, packet, types.RoleSender, &strippedPacket) + baseSender, err := types.ExtractBaseAddressFromData(k.cdc, packet.GetData(), types.RoleSender, nil) if err != nil { return err } - modErr := ibcModule.OnTimeoutPacket(ctx, strippedPacket, relayer) + + origPacket := packet + packetStore, packetKey := k.PacketStoreFromOrigin(ctx, types.PacketSrc, packet) + if packetStore.Has(packetKey) { + origPacket.Data = packetStore.Get(packetKey) + packetStore.Delete(packetKey) + } + + // Pass every stripped-sender timeout to the wrapped IBC module. + modErr := ibcModule.OnTimeoutPacket(ctx, packet, relayer) // If the sender is not a watched account, we're done. if !k.targetIsWatched(ctx, baseSender) { @@ -188,7 +336,7 @@ func (k Keeper) InterceptOnTimeoutPacket( } // Trigger VM with the original packet, regardless of errors in the app. - vmErr := k.vibcKeeper.TriggerOnTimeoutPacket(ctx, baseSender, packet, relayer) + vmErr := k.vibcKeeper.TriggerOnTimeoutPacket(ctx, baseSender, origPacket, relayer) // Any error from the VM is trumped by one from the wrapped IBC module. if modErr != nil { @@ -199,21 +347,36 @@ func (k Keeper) InterceptOnTimeoutPacket( // InterceptWriteAcknowledgement checks to see if the packet's receiver is a // targeted account, and if so, delegates to the VM. -func (k Keeper) InterceptWriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) error { - // Get the base baseReceiver from the packet, without computing a stripped packet. +func (k Keeper) InterceptWriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, ack ibcexported.Acknowledgement) (ibcexported.Acknowledgement, ibcexported.PacketI) { + // Get the base receiver from the packet, without computing a stripped packet. baseReceiver, err := types.ExtractBaseAddressFromPacket(k.cdc, packet, types.RoleReceiver, nil) + + origPacket := channeltypes.NewPacket( + packet.GetData(), packet.GetSequence(), + packet.GetSourcePort(), packet.GetSourceChannel(), + packet.GetDestPort(), packet.GetDestChannel(), + clienttypes.MustParseHeight(packet.GetTimeoutHeight().String()), + packet.GetTimeoutTimestamp(), + ) + packetStore, packetKey := k.PacketStoreFromOrigin(ctx, types.PacketDst, packet) + if packetStore.Has(packetKey) { + origPacket.Data = packetStore.Get(packetKey) + packetStore.Delete(packetKey) + } + if err != nil || !k.targetIsWatched(ctx, baseReceiver) { // We can't parse, or not watching, but that means just to ack directly. - return k.WriteAcknowledgement(ctx, chanCap, packet, ack) + return ack, origPacket } // Trigger VM with the original packet. - if err = k.vibcKeeper.TriggerWriteAcknowledgement(ctx, baseReceiver, packet, ack); err != nil { + if err = k.vibcKeeper.TriggerWriteAcknowledgement(ctx, baseReceiver, origPacket, ack); err != nil { errAck := channeltypes.NewErrorAcknowledgement(err) - return k.WriteAcknowledgement(ctx, chanCap, packet, errAck) + return errAck, origPacket } - return nil + // The VM has taken over the ack, so we return nil to indicate that the ack is async. + return nil, origPacket } // targetIsWatched checks if a target address has been watched by the VM. From 8d59aa635313a9bb58608258f8274bd8c7b9b20f Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Wed, 22 Jan 2025 10:25:29 -0600 Subject: [PATCH 2/6] chore(vibc): make `RawAcknowledgement` public --- golang/cosmos/x/vibc/types/receiver.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/golang/cosmos/x/vibc/types/receiver.go b/golang/cosmos/x/vibc/types/receiver.go index b9b658c895d..55136c31c55 100644 --- a/golang/cosmos/x/vibc/types/receiver.go +++ b/golang/cosmos/x/vibc/types/receiver.go @@ -15,7 +15,7 @@ import ( var ( _ vm.PortHandler = (*Receiver)(nil) - _ exported.Acknowledgement = (*rawAcknowledgement)(nil) + _ exported.Acknowledgement = (*RawAcknowledgement)(nil) ) type ReceiverImpl interface { @@ -71,15 +71,21 @@ func orderToString(order channeltypes.Order) string { } } -type rawAcknowledgement struct { +type RawAcknowledgement struct { data []byte } -func (r rawAcknowledgement) Acknowledgement() []byte { +func NewRawAcknowledgement(data []byte) RawAcknowledgement { + return RawAcknowledgement{ + data: data, + } +} + +func (r RawAcknowledgement) Acknowledgement() []byte { return r.data } -func (r rawAcknowledgement) Success() bool { +func (r RawAcknowledgement) Success() bool { return true } @@ -137,7 +143,7 @@ func (ir Receiver) Receive(cctx context.Context, jsonRequest string) (jsonReply ) case "receiveExecuted": - ack := rawAcknowledgement{ + ack := RawAcknowledgement{ data: msg.Ack, } err = impl.ReceiveWriteAcknowledgement(ctx, msg.Packet, ack) From 9b7cd3a1f16eae1700970892c79c0c4a86884abd Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 13 Jan 2025 19:00:45 -0600 Subject: [PATCH 3/6] test(vtransfer): more comprehensive multiapp tests --- golang/cosmos/types/address_hooks_test.go | 8 + golang/cosmos/x/swingset/testing/queue.go | 8 + golang/cosmos/x/vstorage/testing/queue.go | 6 + .../cosmos/x/vtransfer/ibc_middleware_test.go | 658 ++++++++++++++---- golang/cosmos/x/vtransfer/utils_test.go | 106 +++ 5 files changed, 638 insertions(+), 148 deletions(-) create mode 100644 golang/cosmos/x/vtransfer/utils_test.go diff --git a/golang/cosmos/types/address_hooks_test.go b/golang/cosmos/types/address_hooks_test.go index 8dd7ed6c5b7..89943f9745b 100644 --- a/golang/cosmos/types/address_hooks_test.go +++ b/golang/cosmos/types/address_hooks_test.go @@ -211,6 +211,14 @@ func TestExtractBaseAddressFromPacket(t *testing.T) { } require.Equal(t, ftPacketData, packetData) + + roundTripBz, err := cdc.MarshalJSON(&packetData) + require.NoError(t, err) + require.Equal(t, packet.GetData(), roundTripBz) + + roundTripPacket := packet + roundTripPacket.Data = roundTripBz + require.Equal(t, packet, roundTripPacket) }) } }) diff --git a/golang/cosmos/x/swingset/testing/queue.go b/golang/cosmos/x/swingset/testing/queue.go index 86d9dd6399d..000e8a2ae04 100644 --- a/golang/cosmos/x/swingset/testing/queue.go +++ b/golang/cosmos/x/swingset/testing/queue.go @@ -15,3 +15,11 @@ func GetActionQueueRecords(t *testing.T, ctx sdk.Context, swingsetKeeper keeper. actionQueueName := keeper.StoragePathActionQueue return vstoragetesting.GetQueueItems(ctx, vstorageKeeper, actionQueueName) } + +// ResetActionQueue resets the action queue. +// This is a testing utility function. +func ResetActionQueue(t *testing.T, ctx sdk.Context, swingsetKeeper keeper.Keeper) error { + vstorageKeeper := keeper.GetVstorageKeeper(t, swingsetKeeper) + actionQueueName := keeper.StoragePathActionQueue + return vstoragetesting.ResetQueue(ctx, vstorageKeeper, actionQueueName) +} diff --git a/golang/cosmos/x/vstorage/testing/queue.go b/golang/cosmos/x/vstorage/testing/queue.go index c0cd8350ae8..1b23a3444dd 100644 --- a/golang/cosmos/x/vstorage/testing/queue.go +++ b/golang/cosmos/x/vstorage/testing/queue.go @@ -26,3 +26,9 @@ func GetQueueItems(ctx sdk.Context, vstorageKeeper keeper.Keeper, queuePath stri } return values, nil } + +func ResetQueue(ctx sdk.Context, vstorageKeeper keeper.Keeper, queuePath string) error { + unlimitedCtx := ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + vstorageKeeper.RemoveEntriesWithPrefix(unlimitedCtx, queuePath) + return nil +} diff --git a/golang/cosmos/x/vtransfer/ibc_middleware_test.go b/golang/cosmos/x/vtransfer/ibc_middleware_test.go index ee9c80ebb56..9f80714f346 100644 --- a/golang/cosmos/x/vtransfer/ibc_middleware_test.go +++ b/golang/cosmos/x/vtransfer/ibc_middleware_test.go @@ -1,6 +1,7 @@ package vtransfer_test import ( + "bytes" "context" "encoding/json" "fmt" @@ -11,7 +12,10 @@ import ( app "github.com/Agoric/agoric-sdk/golang/cosmos/app" "github.com/Agoric/agoric-sdk/golang/cosmos/vm" + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + "github.com/iancoleman/orderedmap" "github.com/stretchr/testify/suite" "github.com/tendermint/tendermint/libs/log" dbm "github.com/tendermint/tm-db" @@ -20,18 +24,25 @@ import ( swingsettesting "github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/testing" swingsettypes "github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/types" vibckeeper "github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc/keeper" + vibctypes "github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc/types" "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + packetforwardtypes "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v6/packetforward/types" ibctransfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" + ibcexported "github.com/cosmos/ibc-go/v6/modules/core/exported" ibctesting "github.com/cosmos/ibc-go/v6/testing" "github.com/cosmos/ibc-go/v6/testing/simapp" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) +const ( + StorePacketData = true +) + type IntegrationTestSuite struct { suite.Suite @@ -40,17 +51,42 @@ type IntegrationTestSuite struct { // testing chains used for convenience and readability chainA *ibctesting.TestChain chainB *ibctesting.TestChain + chainC *ibctesting.TestChain + + lastChannelOffset map[int]int + endpoints map[int]map[int]*ibctesting.Endpoint queryClient ibctransfertypes.QueryClient } +type TestingAppMaker func() (ibctesting.TestingApp, map[string]json.RawMessage) + +func TestTransferTestSuite(t *testing.T) { + s := new(IntegrationTestSuite) + suite.Run(t, s) +} + // interBlockCacheOpt returns a BaseApp option function that sets the persistent // inter-block write-through cache. func interBlockCacheOpt() func(*baseapp.BaseApp) { return baseapp.SetInterBlockCache(store.NewCommitKVStoreCacheManager()) } -type TestingAppMaker func() (ibctesting.TestingApp, map[string]json.RawMessage) +func (s *IntegrationTestSuite) getEndpoint(a, b int) *ibctesting.Endpoint { + amap := s.endpoints[a] + if amap == nil { + return nil + } + return amap[b] +} + +func (s *IntegrationTestSuite) cacheEndpoint(a, b int, endpoint *ibctesting.Endpoint) { + amap := s.endpoints[a] + if amap == nil { + amap = make(map[int]*ibctesting.Endpoint) + } + amap[b] = endpoint +} // Each instance has unique IBC genesis state with deterministic // client/connection/channel initial sequence numbers @@ -62,6 +98,15 @@ func computeSequences(instance int) (clientSeq, connectionSeq, channelSeq int) { return baseSequence, baseSequence + 10, baseSequence + 50 } +func (s *IntegrationTestSuite) nextChannelOffset(instance int) int { + offset, ok := s.lastChannelOffset[instance] + if ok { + offset += 1 + } + s.lastChannelOffset[instance] = offset + return offset +} + func SetupAgoricTestingApp(instance int) TestingAppMaker { return func() (ibctesting.TestingApp, map[string]json.RawMessage) { db := dbm.NewMemDB() @@ -132,17 +177,15 @@ func SetupAgoricTestingApp(instance int) TestingAppMaker { } } -func TestKeeperTestSuite(t *testing.T) { - suite.Run(t, new(IntegrationTestSuite)) -} - -// SetupTest initializes an IntegrationTestSuite with two similar chains, a +// SetupTest initializes an IntegrationTestSuite with three similar chains, a // shared coordinator, and a query client that happens to point at chainA. func (s *IntegrationTestSuite) SetupTest() { + s.lastChannelOffset = make(map[int]int) + s.endpoints = make(map[int]map[int]*ibctesting.Endpoint) s.coordinator = ibctesting.NewCoordinator(s.T(), 0) chains := make(map[string]*ibctesting.TestChain) - for i := 0; i < 2; i++ { + for i := 0; i < 3; i++ { ibctesting.DefaultTestingAppInit = SetupAgoricTestingApp(i) chainID := ibctesting.GetChainID(i) @@ -183,6 +226,7 @@ func (s *IntegrationTestSuite) SetupTest() { s.coordinator.Chains = chains s.chainA = s.coordinator.GetChain(ibctesting.GetChainID(0)) s.chainB = s.coordinator.GetChain(ibctesting.GetChainID(1)) + s.chainC = s.coordinator.GetChain(ibctesting.GetChainID(2)) agoricApp := s.GetApp(s.chainA) @@ -191,6 +235,10 @@ func (s *IntegrationTestSuite) SetupTest() { s.queryClient = ibctransfertypes.NewQueryClient(queryHelper) } +func (s *IntegrationTestSuite) GetChainByIndex(index int) *ibctesting.TestChain { + return s.coordinator.GetChain(ibctesting.GetChainID(index)) +} + func (s *IntegrationTestSuite) GetApp(chain *ibctesting.TestChain) *app.GaiaApp { app, ok := chain.App.(*app.GaiaApp) if !ok { @@ -200,30 +248,53 @@ func (s *IntegrationTestSuite) GetApp(chain *ibctesting.TestChain) *app.GaiaApp return app } -func (s *IntegrationTestSuite) NewTransferPath() *ibctesting.Path { - path := ibctesting.NewPath(s.chainA, s.chainB) - _, _, channelASeq := computeSequences(0) - _, _, channelBSeq := computeSequences(1) - path.EndpointA.ChannelID = fmt.Sprintf("channel-%d", channelASeq) - path.EndpointB.ChannelID = fmt.Sprintf("channel-%d", channelBSeq) +func (s *IntegrationTestSuite) NewTransferPath(endpointAChainIdx, endpointBChainIdx int) *ibctesting.Path { + endpointAChain := s.coordinator.GetChain(ibctesting.GetChainID(endpointAChainIdx)) + endpointBChain := s.coordinator.GetChain(ibctesting.GetChainID(endpointBChainIdx)) + + chAOffset := s.nextChannelOffset(endpointAChainIdx) + chBOffset := s.nextChannelOffset(endpointBChainIdx) + path := ibctesting.NewPath(endpointAChain, endpointBChain) + _, _, channelASeq := computeSequences(endpointAChainIdx) + _, _, channelBSeq := computeSequences(endpointBChainIdx) + path.EndpointA.ChannelID = fmt.Sprintf("channel-%d", channelASeq+chAOffset) + path.EndpointB.ChannelID = fmt.Sprintf("channel-%d", channelBSeq+chBOffset) path.EndpointA.ChannelConfig.PortID = ibctesting.TransferPort path.EndpointB.ChannelConfig.PortID = ibctesting.TransferPort path.EndpointA.ChannelConfig.Version = "ics20-1" path.EndpointB.ChannelConfig.Version = "ics20-1" - s.coordinator.Setup(path) + endpoint := s.getEndpoint(endpointAChainIdx, endpointBChainIdx) + if endpoint == nil { + s.coordinator.SetupConnections(path) + s.cacheEndpoint(endpointAChainIdx, endpointBChainIdx, path.EndpointA) + s.cacheEndpoint(endpointBChainIdx, endpointAChainIdx, path.EndpointB) + } else { + path.EndpointA.ClientID = endpoint.ClientID + path.EndpointA.ConnectionID = endpoint.ConnectionID + + path.EndpointB.ClientID = endpoint.Counterparty.ClientID + path.EndpointB.ConnectionID = endpoint.Counterparty.ConnectionID + } + s.coordinator.CreateChannels(path) - s.coordinator.CommitBlock(s.chainA, s.chainB) + s.coordinator.CommitBlock(endpointAChain, endpointBChain) return path } +func (s *IntegrationTestSuite) resetActionQueue(chain *ibctesting.TestChain) { + err := swingsettesting.ResetActionQueue(s.T(), chain.GetContext(), s.GetApp(chain).SwingSetKeeper) + s.Require().NoError(err) +} + func (s *IntegrationTestSuite) assertActionQueue(chain *ibctesting.TestChain, expectedRecords []swingsettypes.InboundQueueRecord) { actualRecords, err := swingsettesting.GetActionQueueRecords( s.T(), chain.GetContext(), s.GetApp(chain).SwingSetKeeper, ) + s.resetActionQueue(chain) s.Require().NoError(err) exLen := len(expectedRecords) @@ -263,44 +334,86 @@ func (s *IntegrationTestSuite) RegisterBridgeTarget(chain *ibctesting.TestChain, agdServer := s.GetApp(chain).AgdServer defer agdServer.SetControllerContext(chain.GetContext())() var reply string - err := agdServer.ReceiveMessage( + bz, err := json.Marshal(struct { + Type string + Target string + }{"BRIDGE_TARGET_REGISTER", target}) + s.Require().NoError(err) + err = agdServer.ReceiveMessage( &vm.Message{ Port: agdServer.GetPort("vtransfer"), - Data: `{"type":"BRIDGE_TARGET_REGISTER","target":"` + target + `"}`, + Data: string(bz), }, &reply, ) s.Require().NoError(err) - s.Require().Equal(reply, "true") + s.Require().Equal("true", reply) } -func (s *IntegrationTestSuite) TransferFromSourceChain( - srcChain *ibctesting.TestChain, +func (s *IntegrationTestSuite) TransferFromEndpoint( + srcContext sdk.Context, + src *ibctesting.Endpoint, data ibctransfertypes.FungibleTokenPacketData, - src, dst *ibctesting.Endpoint, -) (channeltypes.Packet, error) { +) error { tokenAmt, ok := sdk.NewIntFromString(data.Amount) s.Require().True(ok) - timeoutHeight := srcChain.GetTimeoutHeight() - packet := channeltypes.NewPacket(data.GetBytes(), 0, src.ChannelConfig.PortID, src.ChannelID, dst.ChannelConfig.PortID, dst.ChannelID, timeoutHeight, 0) + timeoutHeight := src.Counterparty.Chain.GetTimeoutHeight() // send a transfer packet from src - imt := ibctransfertypes.MsgTransfer{ - SourcePort: packet.SourcePort, - SourceChannel: packet.SourceChannel, - Memo: data.Memo, - Token: sdk.NewCoin(data.Denom, tokenAmt), - Sender: data.Sender, - Receiver: data.Receiver, - TimeoutHeight: packet.TimeoutHeight, - TimeoutTimestamp: packet.TimeoutTimestamp, + imt := ibctransfertypes.NewMsgTransfer( + src.ChannelConfig.PortID, + src.ChannelID, + sdk.NewCoin(data.Denom, tokenAmt), + data.Sender, + data.Receiver, + timeoutHeight, + 0, + data.Memo, + ) + + tk := s.GetApp(src.Chain).TransferKeeper + _, err := tk.Transfer(srcContext, imt) + return err +} + +func (s *IntegrationTestSuite) prependDenomTrace(sender *ibctesting.Endpoint, trace string) string { + return fmt.Sprintf("%s/%s/%s", sender.ChannelConfig.PortID, sender.ChannelID, trace) +} + +func (s *IntegrationTestSuite) overrideSendPacketData(cdc codec.Codec, data []byte, hookedSender string) ([]byte, error) { + var ftpd ibctransfertypes.FungibleTokenPacketData + err := json.Unmarshal(data, &ftpd) + if err != nil { + return nil, err + } + + // XXX: This is a hack to get around the fact that `TransferKeeper.Transfer` + // doesn't understand hooked senders. We need to put the hooked sender back + // in so that the vtransfer keeper can strip it out as if it had been there + // all along. + newFtpd := ftpd + newFtpd.Sender = hookedSender + + // Permute the encoded data to ensure that it is different that what the TransferKeeper.Transfer specified. + if bz := ftpd.GetBytes(); !bytes.Equal(data, bz) { + return newFtpd.GetBytes(), nil + } + + bz, err := cdc.MarshalJSON(&ftpd) + if err != nil { + return nil, err + } + + if !bytes.Equal(data, bz) { + newBz, err := cdc.MarshalJSON(&newFtpd) + if err != nil { + return nil, err + } + return newBz, nil } - imr, err := s.GetApp(srcChain).TransferKeeper.Transfer(srcChain.GetContext(), &imt) - s.Require().NoError(err) - packet.Sequence = imr.Sequence - return packet, nil + return nil, fmt.Errorf("failed to find a way to permute packet data: %s", string(data)) } func (s *IntegrationTestSuite) mintToAddress(chain *ibctesting.TestChain, addr sdk.AccAddress, denom, amount string) { @@ -314,139 +427,388 @@ func (s *IntegrationTestSuite) mintToAddress(chain *ibctesting.TestChain, addr s s.Require().NoError(err) err = app.BankKeeper.SendCoinsFromModuleToAccount(chain.GetContext(), ibctransfertypes.ModuleName, addr, coins) s.Require().NoError(err) - - // Verify success. - balances := app.BankKeeper.GetAllBalances(chain.GetContext(), addr) - s.Require().Equal(coins[0], balances[1]) } -// TestTransferFromAgdToAgd relays an IBC transfer initiated from a chain A to a -// chain B, and relays the chain B's resulting acknowledgement in return. It -// verifies that the source and destination accounts' bridge targets are called -// by inspecting their resulting actionQueue records. By committing blocks -// between actions, the test verifies that the VM results are permitted to be -// async across blocks. -func (s *IntegrationTestSuite) TestTransferFromAgdToAgd() { - path := s.NewTransferPath() - s.Require().Equal(path.EndpointA.ChannelID, "channel-1050") - - s.Run("TransferFromAgdToAgd", func() { - // create a transfer packet's data contents - baseReceiver := s.chainB.SenderAccounts[1].SenderAccount.GetAddress().String() - receiverHook, err := types.JoinHookedAddress(baseReceiver, []byte("?what=arbitrary-data&why=to-test-bridge-targets")) - s.Require().NoError(err) - transferData := ibctransfertypes.NewFungibleTokenPacketData( - "uosmo", - "1000000", - s.chainA.SenderAccount.GetAddress().String(), - receiverHook, - `"This is a JSON memo"`, - ) +// TestHops relays an IBC transfer initiated from a chain A to a chain B, via 0 +// or more intermediate chains' PacketForwardMiddleware, and relays the chain +// B's resulting acknowledgement back through the intermediate chains to chain A +// in return. It verifies that the source and destination accounts' bridge +// targets are called by inspecting their resulting actionQueue records. By +// committing blocks between actions, the test verifies that the VM results are +// permitted to be async across blocks. +func (s *IntegrationTestSuite) TestHops() { + testCases := []struct { + name string + senderIsTarget bool + receiverIsTarget bool + senderHookData []byte + receiverHookData []byte + }{ + {"NoTargetsNoHooks", false, false, nil, nil}, + {"NoTargetsReceiverHook", false, false, nil, []byte("?what=arbitrary-data&why=to-test-bridge-targets")}, + {"NoTargetsSenderHook", false, false, []byte("?name=alice&peer=bob"), nil}, + {"NoTargetsBothHooks", false, false, []byte("?name=alice&peer=bob"), []byte("?what=arbitrary-data&why=to-test-bridge-targets")}, + {"SenderTargetNoHooks", true, false, nil, nil}, + {"SenderTargetReceiverHook", true, false, nil, []byte("?what=arbitrary-data&why=to-test-bridge-targets")}, + {"SenderTargetSenderHook", true, false, []byte("?name=alice&peer=bob"), nil}, + {"SenderTargetBothHooks", true, false, []byte("?name=alice&peer=bob"), []byte("?what=arbitrary-data&why=to-test-bridge-targets")}, + {"ReceiverTargetNoHooks", false, true, nil, nil}, + {"ReceiverTargetReceiverHook", false, true, nil, []byte("?what=arbitrary-data&why=to-test-bridge-targets")}, + {"ReceiverTargetSenderHook", false, true, []byte("?name=alice&peer=bob"), nil}, + {"ReceiverTargetBothHooks", false, true, []byte("?name=alice&peer=bob"), []byte("?what=arbitrary-data&why=to-test-bridge-targets")}, + {"BothTargetsNoHooks", true, true, nil, nil}, + {"BothTargetsReceiverHook", true, true, nil, []byte("?what=arbitrary-data&why=to-test-bridge-targets")}, + {"BothTargetsSenderHook", true, true, []byte("?name=alice&peer=bob"), nil}, + {"BothTargetsBothHooks", true, true, []byte("?name=alice&peer=bob"), []byte("?what=arbitrary-data&why=to-test-bridge-targets")}, + } - // Register the sender and receiver as bridge targets on their specific - // chain. - s.RegisterBridgeTarget(s.chainA, transferData.Sender) - s.RegisterBridgeTarget(s.chainB, baseReceiver) + for hops := 1; hops <= 2; hops += 1 { + for _, tc := range testCases { + tc := tc + name := fmt.Sprintf("%s_%dHop", tc.name, hops) + s.Run(name, func() { + _, _, baseSenderAddr := testdata.KeyTestPubAddr() + baseSender := baseSenderAddr.String() + + _, _, baseReceiverAddr := testdata.KeyTestPubAddr() + baseReceiver := baseReceiverAddr.String() + + var receiver, sender string + var err error + if tc.senderHookData != nil { + sender, err = types.JoinHookedAddress(baseSender, tc.senderHookData) + s.Require().NoError(err) + } else { + sender = baseSender + } - s.mintToAddress(s.chainA, s.chainA.SenderAccount.GetAddress(), transferData.Denom, transferData.Amount) + if tc.receiverHookData != nil { + receiver, err = types.JoinHookedAddress(baseReceiver, tc.receiverHookData) + s.Require().NoError(err) + } else { + receiver = baseReceiver + } - // Initiate the transfer - packet, err := s.TransferFromSourceChain(s.chainA, transferData, path.EndpointA, path.EndpointB) - s.Require().NoError(err) + var overriddenPacketData []byte + overrideSendPacketData := func(ctx sdk.Context, cdc codec.Codec, data []byte) ([]byte, error) { + newData, err := s.overrideSendPacketData(cdc, data, sender) + overriddenPacketData = newData + return overriddenPacketData, err + } + // Reset the chain state. + for i := 0; i <= hops; i += 1 { + chain := s.GetChainByIndex(i) + s.resetActionQueue(chain) + s.GetApp(chain).VtransferKeeper.SetDebugging(StorePacketData, overrideSendPacketData) + + // Only the first chain is the sender, so don't override any other packets. + overrideSendPacketData = nil + } - // Relay the packet - s.coordinator.CommitBlock(s.chainA) - err = path.EndpointB.UpdateClient() - s.Require().NoError(err) - s.coordinator.CommitBlock(s.chainB) + // Construct the transfer path from chainA=0, [chainC=2, [chainD=3...]], and finally chainB=1. + // This guarantees that the first and last chains are s.chainA and s.chainB. + paths := make([]*ibctesting.Path, hops) + { + endpointAChainIndex := 0 // s.chainA + endpointBChainIndex := 2 // s.chainC + for i := 0; i < hops; i += 1 { + if i == hops-1 { + // Final path's endpointB is s.chainB=1. + endpointBChainIndex = 1 + } + // Each path is an endpointA->endpointB pair. We specify them by index. + paths[i] = s.NewTransferPath(endpointAChainIndex, endpointBChainIndex) + // The next path's A is the current path's B... + endpointAChainIndex = endpointBChainIndex + // and the next path's B is the next chain in the sequence. + endpointBChainIndex += 1 + } + } - writeAcknowledgementHeight := s.chainB.CurrentHeader.Height - writeAcknowledgementTime := s.chainB.CurrentHeader.Time.Unix() + // create a transfer packet's data contents + hopReceiver := receiver + var memoBytes []byte + for pathIdx := hops - 1; pathIdx > 0; pathIdx -= 1 { + m := struct { + Forward packetforwardtypes.ForwardMetadata `json:"forward"` + }{} + if memoBytes != nil { + m.Forward.Next = packetforwardtypes.NewJSONObject(false, memoBytes, *orderedmap.New()) + } + m.Forward.Receiver = hopReceiver + + // Previous hops should not have a bech32 address in the receiver field, + // or tokens may get stuck en route rather than returned on error. + hopReceiver = "pfm" + m.Forward.Port = paths[pathIdx].EndpointA.ChannelConfig.PortID + m.Forward.Channel = paths[pathIdx].EndpointA.ChannelID + + memoBytes, err = json.Marshal(m) + s.Require().NoError(err) + } - err = path.EndpointB.RecvPacket(packet) - s.Require().NoError(err) + var memo string + if memoBytes != nil { + memo = string(memoBytes) + } else { + memo = `This is not a JSON memo` + } - // Create a success ack as defined by ICS20. - ack := channeltypes.NewResultAcknowledgement([]byte{1}) - // Create a different ack to show that a contract can change it. - contractAck := channeltypes.NewResultAcknowledgement([]byte{5}) + denomTrace := "uosmo" + transferData := ibctransfertypes.NewFungibleTokenPacketData( + denomTrace, + "1000000", + baseSender, // TODO: ideally this would just be sender, and `TransferKeeper.Transfer` would accept address hooks. + hopReceiver, + memo, + ) + + // Register the sender and receiver as bridge targets on their specific + // chain. + if tc.senderIsTarget { + s.RegisterBridgeTarget(s.chainA, baseSender) + } + if tc.receiverIsTarget { + s.RegisterBridgeTarget(s.chainB, baseReceiver) + } - s.coordinator.CommitBlock(s.chainA, s.chainB) + s.mintToAddress(s.chainA, baseSenderAddr, transferData.Denom, transferData.Amount) - { - expectedRecords := []swingsettypes.InboundQueueRecord{} - s.assertActionQueue(s.chainA, expectedRecords) - } + // Initiate the transfer + sendContext := s.chainA.GetContext() + err = s.TransferFromEndpoint(sendContext, paths[0].EndpointA, transferData) + s.Require().NoError(err) + + sendPacket, err := ParsePacketFromEvents(sendContext.EventManager().Events()) + s.Require().NoError(err) + + s.coordinator.CommitBlock(s.chainA) + + // Relay the packet through the intermediaries to the final destination. + var packetRes *sdk.Result + var writeAcknowledgementHeight, writeAcknowledgementTime int64 + for pathIdx := 0; pathIdx < hops; pathIdx += 1 { + nextPath := paths[pathIdx] + err = nextPath.EndpointB.UpdateClient() + s.Require().NoError(err) + s.coordinator.CommitBlock(nextPath.EndpointB.Chain) + + writeAcknowledgementHeight = nextPath.EndpointB.Chain.CurrentHeader.Height + writeAcknowledgementTime = nextPath.EndpointB.Chain.CurrentHeader.Time.Unix() + + packetRes, err = nextPath.EndpointB.RecvPacketWithResult(sendPacket) + s.Require().NoError(err) + + s.coordinator.CommitBlock(nextPath.EndpointA.Chain, nextPath.EndpointB.Chain) + + denomTrace = s.prependDenomTrace(nextPath.EndpointB, denomTrace) + + { + expectedRecords := []swingsettypes.InboundQueueRecord{} + s.assertActionQueue(nextPath.EndpointA.Chain, expectedRecords) + } + + if pathIdx >= hops-1 { + break + } + + // The PFM should have received the packet and advertised a send toward the last path. + sendPacket, err = ParsePacketFromEvents(packetRes.GetEvents()) + s.Require().NoError(err) + } + + var ack ibcexported.Acknowledgement + var ackedPacket channeltypes.Packet + + expectedAck := channeltypes.NewResultAcknowledgement([]byte{1}) - { - expectedRecords := []swingsettypes.InboundQueueRecord{ { - Action: &vibckeeper.WriteAcknowledgementEvent{ - ActionHeader: &vm.ActionHeader{ - Type: "VTRANSFER_IBC_EVENT", - BlockHeight: writeAcknowledgementHeight, - BlockTime: writeAcknowledgementTime, - }, - Event: "writeAcknowledgement", - Target: baseReceiver, - Packet: packet, - Acknowledgement: ack.Acknowledgement(), - }, - Context: swingsettypes.ActionContext{ - BlockHeight: writeAcknowledgementHeight, - // TxHash is filled in below - MsgIdx: 0, - }, - }, - } + var events sdk.Events + var ackData []byte + if packetRes != nil { + events = packetRes.GetEvents() + ackData, err = ParseAckFromEvents(events) + } + if tc.receiverIsTarget { + s.Require().Nil(ackData) + // The packet was not yet acknowledged, so write out an ack from the VM, one block later + s.coordinator.CommitBlock(s.chainB) + + vmAckContext := s.chainB.GetContext() + err = s.GetApp(s.chainB).VtransferKeeper.ReceiveWriteAcknowledgement(vmAckContext, sendPacket, expectedAck) + s.Require().NoError(err) + + events = vmAckContext.EventManager().Events() + ackData, err = ParseAckFromEvents(events) + } + + s.Require().NoError(err) + + ackedPacket, err = ParsePacketFromFilteredEvents(events, channeltypes.EventTypeWriteAck) + s.Require().NoError(err) + ack = vibctypes.NewRawAcknowledgement(ackData) + + s.coordinator.CommitBlock(s.chainB) + + expectedRecords := []swingsettypes.InboundQueueRecord{} + if tc.receiverIsTarget { + expectedRecords = append(expectedRecords, swingsettypes.InboundQueueRecord{ + Action: &vibckeeper.WriteAcknowledgementEvent{ + ActionHeader: &vm.ActionHeader{ + Type: "VTRANSFER_IBC_EVENT", + BlockHeight: writeAcknowledgementHeight, + BlockTime: writeAcknowledgementTime, + }, + Event: "writeAcknowledgement", + Target: baseReceiver, + Packet: sendPacket, + Acknowledgement: expectedAck.Acknowledgement(), + }, + Context: swingsettypes.ActionContext{ + BlockHeight: writeAcknowledgementHeight, + // TxHash is filled in below + MsgIdx: 0, + }, + }) + } + + s.assertActionQueue(s.chainB, expectedRecords) + } - s.assertActionQueue(s.chainB, expectedRecords) + // Send the acks back. + for pathIdx := hops - 1; pathIdx > 0; pathIdx -= 1 { + priorPath := paths[pathIdx] - // write out a different acknowledgement from the "contract", one block later. - s.coordinator.CommitBlock(s.chainB) - err = s.GetApp(s.chainB).VtransferKeeper.ReceiveWriteAcknowledgement(s.chainB.GetContext(), packet, contractAck) - s.Require().NoError(err) + // Update Client + err = priorPath.EndpointA.UpdateClient() + s.Require().NoError(err) - s.coordinator.CommitBlock(s.chainB) - } + // Prove the PFM packet's acknowledgement. + ackRes, err := acknowledgePacketWithResult(priorPath.EndpointA, ackedPacket, ack.Acknowledgement()) + s.Require().NoError(err) - // Update Client - err = path.EndpointA.UpdateClient() - s.Require().NoError(err) + ackedPacket, err = ParsePacketFromFilteredEvents(ackRes.GetEvents(), channeltypes.EventTypeWriteAck) + s.Require().NoError(err) - acknowledgementHeight := s.chainA.CurrentHeader.Height - acknowledgementTime := s.chainA.CurrentHeader.Time.Unix() + ackData, err := ParseAckFromEvents(ackRes.GetEvents()) + s.Require().NoError(err) + ack = vibctypes.NewRawAcknowledgement(ackData) - // Prove the packet's acknowledgement. - err = path.EndpointA.AcknowledgePacket(packet, contractAck.Acknowledgement()) - s.Require().NoError(err) + s.coordinator.CommitBlock(priorPath.EndpointA.Chain, priorPath.EndpointB.Chain) + } + + // Update Client + err = paths[0].EndpointA.UpdateClient() + s.Require().NoError(err) + + acknowledgementHeight := s.chainA.CurrentHeader.Height + acknowledgementTime := s.chainA.CurrentHeader.Time.Unix() + + // Prove the initial packet's acknowledgement. + ackRes, err := acknowledgePacketWithResult(paths[0].EndpointA, ackedPacket, ack.Acknowledgement()) + s.Require().NoError(err) + + // Commit the block to finalize the acknowledgement. + s.coordinator.CommitBlock(s.chainA, s.chainB) + + // Verify the resulting events. + gotEvents := 0 + expectedEvents := 2 + for _, event := range ackRes.GetEvents() { + if event.Type == ibctransfertypes.EventTypePacket { + gotEvents += 1 + if gotEvents == 2 && len(event.Attributes) == 1 { + // We get a trailing event with a single "success" attribute. + s.Require().Equal(ibctransfertypes.AttributeKeyAckSuccess, string(event.Attributes[0].Key)) + s.Require().Equal("\x01", string(event.Attributes[0].Value)) + continue + } + expectedAttrs := 6 + gotAttrs := 0 + for _, attr := range event.Attributes { + switch string(attr.Key) { + case "module": + s.Require().Equal(ibctransfertypes.ModuleName, string(attr.Value)) + gotAttrs += 1 + case ibctransfertypes.AttributeKeyAckSuccess: + s.Require().Equal("\x01", string(attr.Value)) + gotAttrs += 1 + case ibctransfertypes.AttributeKeyMemo: + s.Require().Equal(transferData.Memo, string(attr.Value)) + gotAttrs += 1 + case ibctransfertypes.AttributeKeyReceiver: + s.Require().Equal(transferData.Receiver, string(attr.Value)) + gotAttrs += 1 + case "sender": // ibctransfertypes.AttributeKeySender: + s.Require().Equal(transferData.Sender, string(attr.Value)) + gotAttrs += 1 + case ibctransfertypes.AttributeKeyDenom: + s.Require().Equal(transferData.Denom, string(attr.Value)) + gotAttrs += 1 + case ibctransfertypes.AttributeKeyAmount: + s.Require().Equal(transferData.Amount, string(attr.Value)) + gotAttrs += 1 + } + } + s.Require().Equal(expectedAttrs, gotAttrs, `expected %d %s type attributes, got %d`, expectedAttrs, ibctransfertypes.EventTypePacket, gotAttrs) + } + } - s.coordinator.CommitBlock(s.chainA, s.chainB) + // The resulting IBC packet event should be what we expected. + s.Require().Equal(expectedEvents, gotEvents, `expected %d %s type events, got %d`, expectedEvents, ibctransfertypes.EventTypePacket, gotEvents) - { - expectedRecords := []swingsettypes.InboundQueueRecord{ { - Action: &vibckeeper.WriteAcknowledgementEvent{ - ActionHeader: &vm.ActionHeader{ - Type: "VTRANSFER_IBC_EVENT", - BlockHeight: acknowledgementHeight, - BlockTime: acknowledgementTime, - }, - Event: "acknowledgementPacket", - Target: transferData.Sender, - Packet: packet, - Acknowledgement: contractAck.Acknowledgement(), - Relayer: s.chainA.SenderAccount.GetAddress(), - }, - Context: swingsettypes.ActionContext{ - BlockHeight: acknowledgementHeight, - // TxHash is filled in below - MsgIdx: 0, - }, - }, - } + // Undo the sender data override. + expectedPacket := ackedPacket + expectedPacket.Data = overriddenPacketData + + expectedRecords := []swingsettypes.InboundQueueRecord{} + if tc.senderIsTarget { + expectedRecords = append(expectedRecords, swingsettypes.InboundQueueRecord{ + Action: &vibckeeper.WriteAcknowledgementEvent{ + ActionHeader: &vm.ActionHeader{ + Type: "VTRANSFER_IBC_EVENT", + BlockHeight: acknowledgementHeight, + BlockTime: acknowledgementTime, + }, + Event: "acknowledgementPacket", + Target: baseSender, + Packet: expectedPacket, + Acknowledgement: ack.Acknowledgement(), + Relayer: s.chainA.SenderAccount.GetAddress(), + }, + Context: swingsettypes.ActionContext{ + BlockHeight: acknowledgementHeight, + // TxHash is filled in below + MsgIdx: 0, + }, + }) + } + + s.assertActionQueue(s.chainA, expectedRecords) + } - s.assertActionQueue(s.chainA, expectedRecords) + // Verify the resulting received coin balance. + req := &banktypes.QueryAllBalancesRequest{ + Address: baseReceiver, + } + res, err := s.GetApp(s.chainB).BankKeeper.AllBalances(s.chainB.GetContext(), req) + s.Require().NoError(err) + + amt, ok := sdk.NewIntFromString(transferData.Amount) + s.Require().True(ok) + + // Decode the denom trace to get the denom hash. + hashReq := &ibctransfertypes.QueryDenomHashRequest{ + Trace: denomTrace, + } + hashRes, err := s.GetApp(s.chainB).TransferKeeper.DenomHash(s.chainB.GetContext(), hashReq) + s.Require().NoError(err) + receivedDenom := `ibc/` + hashRes.Hash + + coins := sdk.NewCoins(sdk.NewCoin(receivedDenom, amt)) + s.Require().True(coins.IsEqual(res.Balances)) + }) } - }) + } } diff --git a/golang/cosmos/x/vtransfer/utils_test.go b/golang/cosmos/x/vtransfer/utils_test.go new file mode 100644 index 00000000000..6ff48694b67 --- /dev/null +++ b/golang/cosmos/x/vtransfer/utils_test.go @@ -0,0 +1,106 @@ +package vtransfer_test + +import ( + "fmt" + "strconv" + + sdk "github.com/cosmos/cosmos-sdk/types" + clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" + channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" + host "github.com/cosmos/ibc-go/v6/modules/core/24-host" + ibctesting "github.com/cosmos/ibc-go/v6/testing" +) + +// acknowledgePacketWithResult sends a MsgAcknowledgement to the channel associated with the endpoint. +func acknowledgePacketWithResult(endpoint *ibctesting.Endpoint, packet channeltypes.Packet, ack []byte) (*sdk.Result, error) { + // get proof of acknowledgement on counterparty + packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + proof, proofHeight := endpoint.Counterparty.QueryProof(packetKey) + + ackMsg := channeltypes.NewMsgAcknowledgement(packet, ack, proof, proofHeight, endpoint.Chain.SenderAccount.GetAddress().String()) + + return endpoint.Chain.SendMsgs(ackMsg) +} + +// ParseAckFromEvents parses events emitted from a MsgRecvPacket and returns the +// acknowledgement. +func ParseAckFromEvents(events sdk.Events) ([]byte, error) { + return ParseAckFromFilteredEvents(events, channeltypes.EventTypeWriteAck) +} + +// ParseAckFromFilteredEvents parses events emitted matching filteredType and returns the acknowledgement. +func ParseAckFromFilteredEvents(events sdk.Events, filteredType string) ([]byte, error) { + for _, ev := range events { + if ev.Type == filteredType { + for _, attr := range ev.Attributes { + if string(attr.Key) == channeltypes.AttributeKeyAck { //nolint:staticcheck // DEPRECATED + return attr.Value, nil + } + } + } + } + return nil, fmt.Errorf("acknowledgement event attribute not found") +} + +// ParsePacketFromEvents parses the send_packet type events emitted by the IBC +// module and returns the packet. +func ParsePacketFromEvents(events sdk.Events) (channeltypes.Packet, error) { + return ParsePacketFromFilteredEvents(events, channeltypes.EventTypeSendPacket) +} + +// ParsePacketFromFilteredEvents parses events emitted matching filteredType and returns the packet. +func ParsePacketFromFilteredEvents(events sdk.Events, filteredType string) (channeltypes.Packet, error) { + for _, ev := range events { + if ev.Type == filteredType { + packet := channeltypes.Packet{} + for _, attr := range ev.Attributes { + switch string(attr.Key) { + case channeltypes.AttributeKeyData: //nolint:staticcheck // DEPRECATED + packet.Data = attr.Value + + case channeltypes.AttributeKeySequence: + seq, err := strconv.ParseUint(string(attr.Value), 10, 64) + if err != nil { + return channeltypes.Packet{}, err + } + + packet.Sequence = seq + + case channeltypes.AttributeKeySrcPort: + packet.SourcePort = string(attr.Value) + + case channeltypes.AttributeKeySrcChannel: + packet.SourceChannel = string(attr.Value) + + case channeltypes.AttributeKeyDstPort: + packet.DestinationPort = string(attr.Value) + + case channeltypes.AttributeKeyDstChannel: + packet.DestinationChannel = string(attr.Value) + + case channeltypes.AttributeKeyTimeoutHeight: + height, err := clienttypes.ParseHeight(string(attr.Value)) + if err != nil { + return channeltypes.Packet{}, err + } + + packet.TimeoutHeight = height + + case channeltypes.AttributeKeyTimeoutTimestamp: + timestamp, err := strconv.ParseUint(string(attr.Value), 10, 64) + if err != nil { + return channeltypes.Packet{}, err + } + + packet.TimeoutTimestamp = timestamp + + default: + continue + } + } + + return packet, nil + } + } + return channeltypes.Packet{}, fmt.Errorf("filtered event type %s not found", filteredType) +} From a0f085e9162e4c67aae2f747bc25da91157dfbbe Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Sat, 1 Feb 2025 19:59:45 -0600 Subject: [PATCH 4/6] refactor(vibc): use `MustParseHeight(GetTimeoutHeight)` idiom --- golang/cosmos/x/vibc/keeper/keeper.go | 7 ++----- golang/cosmos/x/vibc/keeper/triggers.go | 9 +++------ 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/golang/cosmos/x/vibc/keeper/keeper.go b/golang/cosmos/x/vibc/keeper/keeper.go index 4d5587012ef..2388483123e 100644 --- a/golang/cosmos/x/vibc/keeper/keeper.go +++ b/golang/cosmos/x/vibc/keeper/keeper.go @@ -113,10 +113,7 @@ func (k Keeper) ReceiveChanOpenInit(ctx sdk.Context, order channeltypes.Order, c func (k Keeper) ReceiveSendPacket(ctx sdk.Context, packet ibcexported.PacketI) (uint64, error) { sourcePort := packet.GetSourcePort() sourceChannel := packet.GetSourceChannel() - timeoutHeight := packet.GetTimeoutHeight() - timeoutRevisionNumber := timeoutHeight.GetRevisionNumber() - timeoutRevisionHeight := timeoutHeight.GetRevisionHeight() - clientTimeoutHeight := clienttypes.NewHeight(timeoutRevisionNumber, timeoutRevisionHeight) + timeoutHeight := clienttypes.MustParseHeight(packet.GetTimeoutHeight().String()) timeoutTimestamp := packet.GetTimeoutTimestamp() data := packet.GetData() @@ -125,7 +122,7 @@ func (k Keeper) ReceiveSendPacket(ctx sdk.Context, packet ibcexported.PacketI) ( if !ok { return 0, sdkioerrors.Wrapf(channeltypes.ErrChannelCapabilityNotFound, "could not retrieve channel capability at: %s", capName) } - return k.SendPacket(ctx, chanCap, sourcePort, sourceChannel, clientTimeoutHeight, timeoutTimestamp, data) + return k.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data) } // SendPacket defines a wrapper function for the channel Keeper's function diff --git a/golang/cosmos/x/vibc/keeper/triggers.go b/golang/cosmos/x/vibc/keeper/triggers.go index 10416d0cade..ac6d265bf6d 100644 --- a/golang/cosmos/x/vibc/keeper/triggers.go +++ b/golang/cosmos/x/vibc/keeper/triggers.go @@ -12,11 +12,8 @@ import ( ) func reifyPacket(packet ibcexported.PacketI) channeltypes.Packet { - height := packet.GetTimeoutHeight() - ctHeight := clienttypes.Height{ - RevisionHeight: height.GetRevisionHeight(), - RevisionNumber: height.GetRevisionNumber(), - } + + timeoutHeight := clienttypes.MustParseHeight(packet.GetTimeoutHeight().String()) return channeltypes.Packet{ Sequence: packet.GetSequence(), SourcePort: packet.GetSourcePort(), @@ -24,7 +21,7 @@ func reifyPacket(packet ibcexported.PacketI) channeltypes.Packet { DestinationPort: packet.GetDestPort(), DestinationChannel: packet.GetDestChannel(), Data: packet.GetData(), - TimeoutHeight: ctHeight, + TimeoutHeight: timeoutHeight, TimeoutTimestamp: packet.GetTimeoutTimestamp(), } } From 6be6dd7d835b5561398a2c5fab7b511831676afe Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Sat, 1 Feb 2025 20:02:11 -0600 Subject: [PATCH 5/6] refactor(address_hooks): expose `ExtractBaseAddressFromData` to avoid packeting --- golang/cosmos/types/address_hooks.go | 25 ++++++----------------- golang/cosmos/types/address_hooks_test.go | 19 +++++++---------- 2 files changed, 13 insertions(+), 31 deletions(-) diff --git a/golang/cosmos/types/address_hooks.go b/golang/cosmos/types/address_hooks.go index 5ef5a5302df..36248d5d502 100644 --- a/golang/cosmos/types/address_hooks.go +++ b/golang/cosmos/types/address_hooks.go @@ -177,17 +177,12 @@ func extractBaseTransferData(transferData transfertypes.FungibleTokenPacketData, // If newPacket is not nil, it is populated with a new transfer packet whose // corresponding Sender or Receiver is replaced with the extracted base address. func ExtractBaseAddressFromPacket(cdc codec.Codec, packet ibcexported.PacketI, role AddressRole, newPacket *channeltypes.Packet) (string, error) { - transferData := transfertypes.FungibleTokenPacketData{} - if err := cdc.UnmarshalJSON(packet.GetData(), &transferData); err != nil { - return "", err - } - - var newTransferData *transfertypes.FungibleTokenPacketData + var newDataP *[]byte if newPacket != nil { - // Capture the transfer data for the new packet. - newTransferData = &transfertypes.FungibleTokenPacketData{} + // Capture the data for the new packet. + newDataP = new([]byte) } - target, err := extractBaseTransferData(transferData, role, newTransferData) + target, err := ExtractBaseAddressFromData(cdc, packet.GetData(), role, newDataP) if err != nil { return target, err } @@ -197,19 +192,11 @@ func ExtractBaseAddressFromPacket(cdc codec.Codec, packet ibcexported.PacketI, r } // Create a new packet with the new transfer packet data. - // Re-serialize the packet data with the base addresses. - newData, err := cdc.MarshalJSON(newTransferData) - if err != nil { - return target, err - } - - // Create the new packet. - th := packet.GetTimeoutHeight() *newPacket = channeltypes.NewPacket( - newData, packet.GetSequence(), + *newDataP, packet.GetSequence(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetDestPort(), packet.GetDestChannel(), - clienttypes.NewHeight(th.GetRevisionNumber(), th.GetRevisionHeight()), + clienttypes.MustParseHeight(packet.GetTimeoutHeight().String()), packet.GetTimeoutTimestamp(), ) diff --git a/golang/cosmos/types/address_hooks_test.go b/golang/cosmos/types/address_hooks_test.go index 89943f9745b..8945fd752dc 100644 --- a/golang/cosmos/types/address_hooks_test.go +++ b/golang/cosmos/types/address_hooks_test.go @@ -171,8 +171,7 @@ func TestExtractBaseAddressFromPacket(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { ftPacketData := transfertypes.NewFungibleTokenPacketData("denom", "100", tc.addrs[types.RoleSender].addr, tc.addrs[types.RoleReceiver].addr, "my-favourite-memo") - packetBz, err := cdc.MarshalJSON(&ftPacketData) - require.NoError(t, err) + packetBz := ftPacketData.GetBytes() packet := channeltypes.NewPacket(packetBz, 1234, "my-port", "my-channel", "their-port", "their-channel", clienttypes.NewHeight(133, 445), 10999) for role, addrs := range tc.addrs { @@ -184,9 +183,13 @@ func TestExtractBaseAddressFromPacket(t *testing.T) { require.NoError(t, err) require.Equal(t, addrs.baseAddr, baseAddr) - packetBaseAddr, err := types.ExtractBaseAddressFromPacket(cdc, packet, role, nil) + packetBaseAddr0, err := types.ExtractBaseAddressFromData(cdc, packet.GetData(), role, nil) require.NoError(t, err) - require.Equal(t, addrs.baseAddr, packetBaseAddr) + require.Equal(t, addrs.baseAddr, packetBaseAddr0) + + packetBaseAddr1, err := types.ExtractBaseAddressFromPacket(cdc, packet, role, nil) + require.NoError(t, err) + require.Equal(t, addrs.baseAddr, packetBaseAddr1) var newPacket channeltypes.Packet packetBaseAddr2, err := types.ExtractBaseAddressFromPacket(cdc, packet, role, &newPacket) @@ -211,14 +214,6 @@ func TestExtractBaseAddressFromPacket(t *testing.T) { } require.Equal(t, ftPacketData, packetData) - - roundTripBz, err := cdc.MarshalJSON(&packetData) - require.NoError(t, err) - require.Equal(t, packet.GetData(), roundTripBz) - - roundTripPacket := packet - roundTripPacket.Data = roundTripBz - require.Equal(t, packet, roundTripPacket) }) } }) From 3e74f5179047cd0ea1fed85e7333e5140e01c992 Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Mon, 3 Feb 2025 20:08:54 -0600 Subject: [PATCH 6/6] docs(vtransfer): add notes to funcs inspired by `ibctesting` --- golang/cosmos/x/vtransfer/utils_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/golang/cosmos/x/vtransfer/utils_test.go b/golang/cosmos/x/vtransfer/utils_test.go index 6ff48694b67..a29dc379048 100644 --- a/golang/cosmos/x/vtransfer/utils_test.go +++ b/golang/cosmos/x/vtransfer/utils_test.go @@ -12,6 +12,7 @@ import ( ) // acknowledgePacketWithResult sends a MsgAcknowledgement to the channel associated with the endpoint. +// [AGORIC] Would be nice to create a new ibctesting.AcknowledgePacketWithResult func acknowledgePacketWithResult(endpoint *ibctesting.Endpoint, packet channeltypes.Packet, ack []byte) (*sdk.Result, error) { // get proof of acknowledgement on counterparty packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) @@ -24,11 +25,13 @@ func acknowledgePacketWithResult(endpoint *ibctesting.Endpoint, packet channelty // ParseAckFromEvents parses events emitted from a MsgRecvPacket and returns the // acknowledgement. +// [AGORIC] Signature taken from ibctesting.ParseAckFromEvents func ParseAckFromEvents(events sdk.Events) ([]byte, error) { return ParseAckFromFilteredEvents(events, channeltypes.EventTypeWriteAck) } // ParseAckFromFilteredEvents parses events emitted matching filteredType and returns the acknowledgement. +// [AGORIC] Would be nice to improve the implementation and upstream it func ParseAckFromFilteredEvents(events sdk.Events, filteredType string) ([]byte, error) { for _, ev := range events { if ev.Type == filteredType { @@ -44,11 +47,13 @@ func ParseAckFromFilteredEvents(events sdk.Events, filteredType string) ([]byte, // ParsePacketFromEvents parses the send_packet type events emitted by the IBC // module and returns the packet. +// [AGORIC] Signature taken from ibctesting.ParsePacketFromEvents func ParsePacketFromEvents(events sdk.Events) (channeltypes.Packet, error) { return ParsePacketFromFilteredEvents(events, channeltypes.EventTypeSendPacket) } // ParsePacketFromFilteredEvents parses events emitted matching filteredType and returns the packet. +// [AGORIC] Would be nice to improve the implementation and upstream it func ParsePacketFromFilteredEvents(events sdk.Events, filteredType string) (channeltypes.Packet, error) { for _, ev := range events { if ev.Type == filteredType {