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

local-orchestration-account: transfer supports multi-hop pfm routes #10445

Closed
Tracked by #10006
0xpatrickdev opened this issue Nov 11, 2024 · 0 comments · Fixed by #10591
Closed
Tracked by #10006

local-orchestration-account: transfer supports multi-hop pfm routes #10445

0xpatrickdev opened this issue Nov 11, 2024 · 0 comments · Fixed by #10591
Assignees
Labels
enhancement New feature or request

Comments

@0xpatrickdev
Copy link
Member

What is the Problem Being Solved?

As a FastUSDC developer, I expect LocalOrchestration.transfer(destination: ChainAddress: amount: AmountArg) to support multi-hop PFM routes.

Example: transfer 1 uusdc from agoric to osmosis routed through noble.

See #10006 for the parent ticket.

Description of the Design

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations

@0xpatrickdev 0xpatrickdev added the enhancement New feature or request label Nov 11, 2024
@LuqiPan LuqiPan assigned 0xpatrickdev and unassigned iomekam Nov 15, 2024
@LuqiPan LuqiPan added this to the FU1: package integration milestone Nov 18, 2024
mergify bot added a commit that referenced this issue Nov 26, 2024
refs: #10006
refs: #10445 

## Description
- adds `pfmEnabled: bool` to `CosmosChainInfo` to support multi-hop forwarding logic
- adds and exports `withChainCapabilities` helper that mixes in `PfmEnabled` and `IcqEnabled` constants to `ChainInfo`
- adds and exports `registerChainsAndAssets` helper that registers info in a `chainHub` in a contract `startFn`
- implements `chainHub` initialization in `fast-usdc` and `send-anywhere` contracts

### Security Considerations
- `chain-capabilities.js` is authoritative, but consumers have the ability to provide their own data. It's not published to vstorage and will be mixed in to local ChainHub's in example contracts that rely on it.

### Scaling Considerations
- Authors must maintain `chain-capabilities.js` over time

### Documentation Considerations
- documented via typedoc

### Testing Considerations
- updates snapshot tests

### Upgrade Considerations
Library code, part of an NPM Orch release
mergify bot added a commit that referenced this issue Nov 28, 2024
refs: #10445
refs: #10006

## Description
Adds `.makeTransferRoute(destination: ChainAddress, amount: DenomAmount, holdingChainName: string)`  to `ChainHub` to facilitate building IBC `MsgTransfer` parameters for single-hop and multi-hop (pfm) routes.

Returns synchronously using local `chainHub` data to facilitate ease of use at call sites and support future plans to make every call synchronous. It achieves this by interfacing  with the `chainInfos` and `connInfos` map stores directly instead of the public interface methods that currently fall back to remote calls to `agoricNames`.

Assumes the longest route will only be 1 intermediary hop (through the issuing chain). Does not support unwinding nested denoms (e.g. noble uusdc  sent directly from osmosis to agoric `agoric(osmosis(noble(uusdc)))`).

### Security Considerations
An incorrect implementation could result in loss of funds. #9324 remains open to determine sensible defaults for timeout parameters.

### Scaling Considerations
Nothing new, but each contract's `chainHub` will accumulate quite a bit of data .

### Documentation Considerations
JSdoc and tests

### Testing Considerations
Includes unit tests covering all codepaths and known scenarios.

### Upgrade Considerations
N/A, library code. This feature is needed for FUSDC.
mergify bot added a commit that referenced this issue Dec 3, 2024
refs: #10445 

## Description
IBC Denoms are unique to a chain but not all not all chains. e.g., if `channel-0` points to `osmosis` for two chains, the `uosmo` denom  will be `ibc/ED07A3391A112B175915CD8FAF43A2DA8E4790EDE12566649D0C2F97716B8518` for both.

This change requires callers to specify the `holdingChainName` for the denom they wish to retrieve information about.

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
Updates existing tests. Motivated from work in #10571 which surfaced this issue.

### Upgrade Considerations
Breaking change, but for library code that will be part on NPM or FUSDC release.
mergify bot added a commit that referenced this issue Dec 4, 2024
refs: #10445 

## Description

- leverages `ChainHub.makeTransferRoute` to support PFM routing in `LocalOrchAccount.transfer()`
- updates test harness to supply chain and asset info for `chainHub` in exo and contract tests
- refactors send-anywhere `contract-upgrade.test.ts` to no longer rely on a buggy `agoricNames`
- updates several orchestration examples contracts to seed their own `chainHub` to facilitate boot and multichain testing

### Security Considerations
No new considerations from these changes.

### Scaling Considerations
No new considerations from these changes.

### Documentation Considerations
None

### Testing Considerations
Includes unit tests. See #10584 for more robust testing of the pfm route logic.  Existing `multichain-testing` will cover potential regressions (single-hop) transfers. Multi-hop tests are forthcoming.

### Upgrade Considerations
N/A, library code. Part of NPM Orch or FUSDC release.
@mergify mergify bot closed this as completed in #10591 Dec 4, 2024
@mergify mergify bot closed this as completed in cf1d435 Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants