Skip to content

Commit

Permalink
feat(vtransfer): extract base address from parameterized address (#10250
Browse files Browse the repository at this point in the history
)

closes: #10249

## Description

This PR allows ICS-20 fungible token transfer packets to contain inbound receiver and outbound sender addresses formatted as either:
- a standard bech32 address (`"agoric1bech32addr"`), or
- a base account with optional subpaths and query parameters (`"agoric1bech32addr/opt/account?k=v&k2=v2"`)

In these cases, the "base address" is `"agoric1bech32addr"`, and tokens are transferred to that base address.  The original address can be parsed by a listening contract into "address parameters" to use at its discretion.

### Security Considerations

Should be no impact.  If a transfer listener does not explicitly look for the data in an inbound packet, it will behave as if there are no address parameters.

Careful review of the code is critical, though, since it touches the powerful `x/vtransfer` module.

### Scaling Considerations

Increases the scalability of contracts that would ordinarily have to maintain a pool of multiple Local Chain Account (LCA) addresses to have a unique ID to map to some parameters.  Now, contracts can create just one LCA and have the sender supply the data as part of a parameterized address.

The ICS-20 transfer addresses are strings limited to at most [2048 bytes, and potentially even smaller if explicitly configured](informalsystems/hermes#3765 (comment)) by a relayer operator.  Thus, the total length of the encoded base address and all the address parameters cannot exceed that address string limit, or the packet will not be forwarded.

### Documentation Considerations

Should be documented as a feature of `vtransfer`.

### Testing Considerations

Unit tests with address parameter data tests have been implemented, testing both the address parser code and the IBC end-to-end packet relay process through the `x/vtransfer` module.

### Upgrade Considerations

This is a chain-level, backwards-compatible change, so no upgrade aspects must be addressed.  However, Agoric contracts will probably need to import an URL-parsing module to parse URL escaping and query parameters syntax (since the `globalThis.URL` constructor is not currently available in the SwingSet vat environment).
  • Loading branch information
mergify[bot] authored Nov 7, 2024
2 parents 0b5aab8 + 3d44b53 commit 4b1403a
Show file tree
Hide file tree
Showing 4 changed files with 366 additions and 41 deletions.
7 changes: 4 additions & 3 deletions golang/cosmos/x/vtransfer/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,18 +331,19 @@ func (s *IntegrationTestSuite) TestTransferFromAgdToAgd() {

s.Run("TransferFromAgdToAgd", func() {
// create a transfer packet's data contents
baseReceiver := s.chainB.SenderAccounts[1].SenderAccount.GetAddress().String()
transferData := ibctransfertypes.NewFungibleTokenPacketData(
"uosmo",
"1000000",
s.chainA.SenderAccount.GetAddress().String(),
s.chainB.SenderAccounts[1].SenderAccount.GetAddress().String(),
baseReceiver+"?what=arbitrary-data&why=to-test-bridge-targets",
`"This is a JSON memo"`,
)

// Register the sender and receiver as bridge targets on their specific
// chain.
s.RegisterBridgeTarget(s.chainA, transferData.Sender)
s.RegisterBridgeTarget(s.chainB, transferData.Receiver)
s.RegisterBridgeTarget(s.chainB, baseReceiver)

s.mintToAddress(s.chainA, s.chainA.SenderAccount.GetAddress(), transferData.Denom, transferData.Amount)

Expand Down Expand Up @@ -384,7 +385,7 @@ func (s *IntegrationTestSuite) TestTransferFromAgdToAgd() {
BlockTime: writeAcknowledgementTime,
},
Event: "writeAcknowledgement",
Target: transferData.Receiver,
Target: baseReceiver,
Packet: packet,
Acknowledgement: ack.Acknowledgement(),
},
Expand Down
77 changes: 39 additions & 38 deletions golang/cosmos/x/vtransfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
"github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc"
vibctypes "github.com/Agoric/agoric-sdk/golang/cosmos/x/vibc/types"
transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
"github.com/Agoric/agoric-sdk/golang/cosmos/x/vtransfer/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"
Expand Down Expand Up @@ -106,7 +106,13 @@ func (k Keeper) GetReceiverImpl() vibctypes.ReceiverImpl {
// 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 {
ack := ibcModule.OnRecvPacket(ctx, packet, relayer)
// Pass every (stripped-receiver) inbound packet to the wrapped IBC module.
var strippedPacket channeltypes.Packet
_, err := types.ExtractBaseAddressFromPacket(k.cdc, packet, types.RoleReceiver, &strippedPacket)
if err != nil {
return channeltypes.NewErrorAcknowledgement(err)
}
ack := ibcModule.OnRecvPacket(ctx, strippedPacket, relayer)

if ack == nil {
// Already declared to be an async ack.
Expand Down Expand Up @@ -136,17 +142,21 @@ func (k Keeper) InterceptOnAcknowledgementPacket(
acknowledgement []byte,
relayer sdk.AccAddress,
) error {
// Pass every acknowledgement to the wrapped IBC module.
modErr := ibcModule.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
// Pass every (stripped-sender) acknowledgement to the wrapped IBC module.
var strippedPacket channeltypes.Packet
baseSender, err := types.ExtractBaseAddressFromPacket(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.parseTransfer(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 {
Expand All @@ -163,17 +173,21 @@ func (k Keeper) InterceptOnTimeoutPacket(
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error {
// Pass every timeout to the wrapped IBC module.
modErr := ibcModule.OnTimeoutPacket(ctx, packet, relayer)
// Pass every (stripped-sender) timeout to the wrapped IBC module.
var strippedPacket channeltypes.Packet
baseSender, err := types.ExtractBaseAddressFromPacket(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.parseTransfer(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 {
Expand All @@ -185,42 +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.parseTransfer(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.ExtractBaseAddressFromPacket(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
}

// parseTransfer checks if a packet's sender and/or receiver are targeted accounts.
func (k Keeper) parseTransfer(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
}

var sender string
var receiver string
// 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 prefixStore.Has([]byte(transferData.Sender)) {
sender = transferData.Sender
}
if prefixStore.Has([]byte(transferData.Receiver)) {
receiver = transferData.Receiver
}
return sender, receiver, nil
return prefixStore.Has([]byte(target))
}

// GetWatchedAdresses returns the watched addresses from the keeper as a slice
Expand Down
156 changes: 156 additions & 0 deletions golang/cosmos/x/vtransfer/types/baseaddr.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package types

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
}

// 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{
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 {
return "", fmt.Errorf("address must be relative path with optional query and fragment, got %s", addr)
}

baseAddr, _, _ := strings.Cut(expected.Path, "/")
if baseAddr == "" {
return "", fmt.Errorf("base address cannot be empty")
}

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
}

// ExtractBaseAddressFromPacket 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 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
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
}
Loading

0 comments on commit 4b1403a

Please sign in to comment.