fixup! feat(vtransfer): extract base address from parameterized address

Author: michaelfig

Diff for: golang/cosmos/x/vtransfer/keeper/keeper.go

@@ -17,7 +17,6 @@ import (
 	"github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc"
 	vibctypes "github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc/types"
 	"github.com/Agoric/agoric-sdk/golang/cosmos/x/vtransfer/types"
-	transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/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"
@@ -102,41 +101,17 @@ func (k Keeper) GetReceiverImpl() vibctypes.ReceiverImpl {
 	return k
 }
 
-// Extract the base address from the packet sender (if senderIsLocal) or
-// receiver (if !senderIsLocal), since the local ibcModule doesn't understand
-// address parameters.
-func (k Keeper) packetWithOnlyBaseAddresses(packet channeltypes.Packet, senderIsLocal bool) channeltypes.Packet {
-	transferData := transfertypes.FungibleTokenPacketData{}
-	if err := k.cdc.UnmarshalJSON(packet.GetData(), &transferData); err != nil {
-		return packet
-	}
-	if senderIsLocal {
-		baseSender, err := types.ExtractBaseAddress(transferData.Sender)
-		if err == nil {
-			transferData.Sender = baseSender
-		}
-	} else {
-		baseReceiver, err := types.ExtractBaseAddress(transferData.Receiver)
-		if err == nil {
-			transferData.Receiver = baseReceiver
-		}
-	}
-
-	// Re-serialize the packet data with the base addresses.
-	data, _ := k.cdc.MarshalJSON(&transferData)
-
-	// Replace the packet's data with the new data.
-	packet.Data = data
-	return packet
-}
-
 // 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
 // will be called later, after the VM has dealt with the packet).
 func (k Keeper) InterceptOnRecvPacket(ctx sdk.Context, ibcModule porttypes.IBCModule, packet channeltypes.Packet, relayer sdk.AccAddress) ibcexported.Acknowledgement {
-	// Pass every (stripped-receiver) inbound to the wrapped IBC module.
-	strippedPacket := k.packetWithOnlyBaseAddresses(packet, false)
+	// Pass every (stripped-receiver) inbound packet to the wrapped IBC module.
+	var strippedPacket channeltypes.Packet
+	_, err := types.ExtractBaseTransferPacket(k.cdc, packet, types.RoleReceiver, &strippedPacket)
+	if err != nil {
+		return channeltypes.NewErrorAcknowledgement(err)
+	}
 	ack := ibcModule.OnRecvPacket(ctx, strippedPacket, relayer)
 
 	if ack == nil {
@@ -168,17 +143,20 @@ func (k Keeper) InterceptOnAcknowledgementPacket(
 	relayer sdk.AccAddress,
 ) error {
 	// Pass every (stripped-sender) acknowledgement to the wrapped IBC module.
-	strippedPacket := k.packetWithOnlyBaseAddresses(packet, true)
+	var strippedPacket channeltypes.Packet
+	baseSender, err := types.ExtractBaseTransferPacket(k.cdc, packet, types.RoleSender, &strippedPacket)
+	if err != nil {
+		return err
+	}
 	modErr := ibcModule.OnAcknowledgementPacket(ctx, strippedPacket, acknowledgement, relayer)
 
-	// If the sender is not a targeted account, we're done.
-	sender, _, err := k.findTransferTargets(ctx, packet)
-	if err != nil || sender == "" {
+	// If the sender is not a watched account, we're done.
+	if !k.targetIsWatched(ctx, baseSender) {
 		return modErr
 	}
 
-	// Trigger VM, regardless of errors in the ibcModule.
-	vmErr := k.vibcKeeper.TriggerOnAcknowledgementPacket(ctx, sender, packet, acknowledgement, relayer)
+	// Trigger VM with the original packet, regardless of errors in the ibcModule.
+	vmErr := k.vibcKeeper.TriggerOnAcknowledgementPacket(ctx, baseSender, packet, acknowledgement, relayer)
 
 	// Any error from the VM is trumped by one from the wrapped IBC module.
 	if modErr != nil {
@@ -196,17 +174,20 @@ func (k Keeper) InterceptOnTimeoutPacket(
 	relayer sdk.AccAddress,
 ) error {
 	// Pass every (stripped-sender) timeout to the wrapped IBC module.
-	strippedPacket := k.packetWithOnlyBaseAddresses(packet, true)
+	var strippedPacket channeltypes.Packet
+	baseSender, err := types.ExtractBaseTransferPacket(k.cdc, packet, types.RoleSender, &strippedPacket)
+	if err != nil {
+		return err
+	}
 	modErr := ibcModule.OnTimeoutPacket(ctx, strippedPacket, relayer)
 
-	// If the sender is not a targeted account, we're done.
-	sender, _, err := k.findTransferTargets(ctx, packet)
-	if err != nil || sender == "" {
+	// If the sender is not a watched account, we're done.
+	if !k.targetIsWatched(ctx, baseSender) {
 		return modErr
 	}
 
-	// Trigger VM, regardless of errors in the app.
-	vmErr := k.vibcKeeper.TriggerOnTimeoutPacket(ctx, sender, packet, relayer)
+	// Trigger VM with the original packet, regardless of errors in the app.
+	vmErr := k.vibcKeeper.TriggerOnTimeoutPacket(ctx, baseSender, packet, relayer)
 
 	// Any error from the VM is trumped by one from the wrapped IBC module.
 	if modErr != nil {
@@ -218,51 +199,29 @@ 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 {
-	_, receiver, err := k.findTransferTargets(ctx, packet)
-	if err != nil || receiver == "" {
-		// We can't parse, but that means just to ack directly.
+	// Get the base baseReceiver from the packet, without computing a stripped packet.
+	baseReceiver, err := types.ExtractBaseTransferPacket(k.cdc, packet, types.RoleReceiver, nil)
+	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)
 	}
 
-	// Trigger VM
-	if err = k.vibcKeeper.TriggerWriteAcknowledgement(ctx, receiver, packet, ack); err != nil {
+	// Trigger VM with the original packet.
+	if err = k.vibcKeeper.TriggerWriteAcknowledgement(ctx, baseReceiver, packet, ack); err != nil {
 		errAck := channeltypes.NewErrorAcknowledgement(err)
 		return k.WriteAcknowledgement(ctx, chanCap, packet, errAck)
 	}
 
 	return nil
 }
 
-// findTransferTargets checks if a packet's sender and/or receiver correspond to targeted accounts.
-func (k Keeper) findTransferTargets(ctx sdk.Context, packet ibcexported.PacketI) (string, string, error) {
-	var transferData transfertypes.FungibleTokenPacketData
-	err := k.cdc.UnmarshalJSON(packet.GetData(), &transferData)
-	if err != nil {
-		return "", "", err
-	}
-
-	// Extract the base addresses from the transferData.
-	senderTarget, err := types.ExtractBaseAddress(transferData.Sender)
-	if err != nil {
-		senderTarget = transferData.Sender
-	}
-	receiverTarget, err := types.ExtractBaseAddress(transferData.Receiver)
-	if err != nil {
-		receiverTarget = transferData.Receiver
-	}
+// targetIsWatched checks if a target address has been watched by the VM.
+func (k Keeper) targetIsWatched(ctx sdk.Context, target string) bool {
 	prefixStore := prefix.NewStore(
 		ctx.KVStore(k.key),
 		[]byte(watchedAddressStoreKeyPrefix),
 	)
-	if senderTarget != "" && !prefixStore.Has([]byte(senderTarget)) {
-		// Not a targeted sender.
-		senderTarget = ""
-	}
-	if receiverTarget != "" && !prefixStore.Has([]byte(receiverTarget)) {
-		// Not a targeted receiver.
-		receiverTarget = ""
-	}
-	return senderTarget, receiverTarget, nil
+	return prefixStore.Has([]byte(target))
 }
 
 // GetWatchedAdresses returns the watched addresses from the keeper as a slice

Diff for: golang/cosmos/x/vtransfer/types/baseaddr.go

@@ -4,28 +4,50 @@ import (
 	"fmt"
 	"net/url"
 	"strings"
+
+	"github.com/cosmos/cosmos-sdk/codec"
+
+	transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/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"
+	ibcexported "github.com/cosmos/ibc-go/v6/modules/core/exported"
+)
+
+type AddressRole string
+
+const (
+	RoleSender   AddressRole = "Sender"
+	RoleReceiver AddressRole = "Receiver"
 )
 
 func trimSlashPrefix(s string) string {
 	return strings.TrimPrefix(s, "/")
 }
 
 // ExtractBaseAddress extracts the base address from a parameterized address.
+// It removes all subpath and query components from addr.
 func ExtractBaseAddress(addr string) (string, error) {
 	parsed, err := url.Parse(addr)
 	if err != nil {
 		return "", err
 	}
 
-	// Remove leading slashes from the path and opaque fields so that only
-	// parsed relative paths match the expected paths.
+	// Specify the fields and values we expect.  Unspecified fields will only
+	// match if they are zero values in order to be robust against extensions to
+	// the url.URL struct.
+	//
+	// Remove leading slashes from the path fields so that only parsed relative
+	// paths match the expected test.
 	expected := url.URL{
-		Opaque:      trimSlashPrefix(parsed.Opaque),
 		Path:        trimSlashPrefix(parsed.Path),
 		RawPath:     trimSlashPrefix(parsed.RawPath),
 		RawQuery:    parsed.RawQuery,
 		Fragment:    parsed.Fragment,
 		RawFragment: parsed.RawFragment,
+
+		// Skip over parsing control flags.
+		ForceQuery: parsed.ForceQuery,
+		OmitHost:   parsed.OmitHost,
 	}
 
 	if *parsed != expected {
@@ -39,3 +61,93 @@ func ExtractBaseAddress(addr string) (string, error) {
 
 	return baseAddr, nil
 }
+
+// ExtractBaseTransferData returns the base address from the transferData.Sender
+// (if RoleSender) or transferData.Receiver (if RoleReceiver). Errors in
+// determining the base address are ignored... we then assume the base address
+// is exactly the original address.  If newTransferData is not nil, it will be
+// populated with a new FungibleTokenPacketData consisting of the role replaced
+// with its base address.
+func ExtractBaseTransferData(transferData transfertypes.FungibleTokenPacketData, role AddressRole, newTransferData *transfertypes.FungibleTokenPacketData) (string, error) {
+	var target string
+	sender := transferData.Sender
+	receiver := transferData.Receiver
+
+	switch role {
+	case RoleSender:
+		baseSender, err := ExtractBaseAddress(sender)
+		if err == nil {
+			sender = baseSender
+		}
+		target = sender
+
+	case RoleReceiver:
+		baseReceiver, err := ExtractBaseAddress(receiver)
+		if err == nil {
+			receiver = baseReceiver
+		}
+		target = receiver
+
+	default:
+		err := fmt.Errorf("invalid address role: %s", role)
+		return target, err
+	}
+
+	if newTransferData == nil {
+		return target, nil
+	}
+
+	// Create the new transfer data.
+	*newTransferData = transfertypes.NewFungibleTokenPacketData(
+		transferData.Denom,
+		transferData.Amount,
+		sender, receiver,
+		transferData.Memo,
+	)
+
+	return target, nil
+}
+
+// ExtractBaseTransferPacket returns the base address from the transfer packet's
+// transferData.Sender (if RoleSender) or transferData.Receiver (if
+// RoleReceiver). If newPacket is not nil, it is populated with a new transfer
+// packet whose corresponding Sender/Receiver is replaced with its base address.
+func ExtractBaseTransferPacket(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
+	if newPacket != nil {
+		// Capture the transfer data for the new packet.
+		newTransferData = &transfertypes.FungibleTokenPacketData{}
+	}
+	target, err := ExtractBaseTransferData(transferData, role, newTransferData)
+	if err != nil {
+		return target, err
+	}
+
+	if newPacket == nil {
+		return target, nil
+	}
+
+	// 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(),
+		packet.GetSourcePort(), packet.GetSourceChannel(),
+		packet.GetDestPort(), packet.GetDestChannel(),
+		clienttypes.NewHeight(th.GetRevisionNumber(), th.GetRevisionHeight()),
+		packet.GetTimeoutTimestamp(),
+	)
+
+	return target, nil
+}