Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 206 forks source link

Orchestration - Create Generic Chain (Chain Object for non well known chain) #9187

Closed rowgraus closed 4 months ago

rowgraus commented 7 months ago

What is the Problem Being Solved?

9063 covers creating a chain object for a well known chain. We separately need to be able to create an object for a generic chain that isn't well known to the API

Story

A contract, SendBLD, lets you send BLD to arbitrary chains.

It is deployed with powers from vat-network and vat-orchestration.

Developer uses the adminFacet to call a method with chainConfig data that creates a Chain object. The contract code gets the chain object for interacting with that chain.

The contract code cannot distinguish between this chain object and chain objects returned from getChain(chainName).

That chain is later registered as a known chain. Nothing breaks.

Description of the Design

The chain object lives in the contract vat.

Powers

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations

0xpatrickdev commented 6 months ago

A different approach that might achieve our intended goal - a way for developers to interface with chains unknown to the KnownChains or the Chain NameHub - is to expose some interfaces currently present in OrchestrationService. These were created in the course of #9195 and #9198, and would probably have been moved to an internal facet in the course of #9063, but maybe they can be added to the public facet.

The only thing this approach might leave short is the getBrandInfo and asAmount helpers.

git diff --staged packages/orchestration/src/types.d.ts
diff --git a/packages/orchestration/src/types.d.ts b/packages/orchestration/src/types.d.ts
index e41541f39..4393ebc36 100644
--- a/packages/orchestration/src/types.d.ts
+++ b/packages/orchestration/src/types.d.ts
@@ -12,6 +12,8 @@ import type {
   Redelegation,
   UnbondingDelegation,
 } from '@agoric/cosmic-proto/cosmos/staking/v1beta1/staking.js';
+import type { IBCConnectionID } from '@agoric/vats';

 export type AttenuatedNetwork = Pick<RouterProtocol, 'bindPort'>;

@@ -109,7 +111,26 @@ export interface Orchestrator {
   getChain: <C extends keyof KnownChains>(chainName: C) => Promise<Chain<C>>;

   makeLocalAccount: () => Promise<LocalChainAccount>;
-
+  /**
+   * Low level primitive for creating an ICA Account. Used internally by the `Chain` object,
+   * but can be used to connect to chains not present in `KnownChains`.
+   *
+   * @internal
+   * Binds an icacontroller-* port and connects to a remote chain using a version specfied
+   * by the ICS-27 protocol
+   */
+  // TODO - is `createInterchainAccount` or `createICA` a better name?
+  createAccount: (
+    hostConnectionId: IBCConnectionID,
+    controllerConnectionId: IBCConnectionID,
+    // TODO considering exposing opts bag arg, like `makeICAConnectionAddress`, which includes version
+  ) => Promise<ChainAccount>;
+
+
+  // TODO - make this `provideQueryConnection`, unless there is reason to
+  // allow consumers to request their own port + QueryConnection obj
+  createQueryConnection: (
+    // is `provideInterchainQueryConnection` / `provideICQConn` a better name?
+    controllerConnectionId: IBCConnectionID,
+  ) => Promise<QueryConnection>;
   /**
    * For a denom, return information about a denom including the equivalent
    * local Brand, the Chain on which the denom is held, and the Chain that

I left comments for things i think we should adjust, namely -

  1. add optional opts arg, for things like version: ics27-1, icq-1
  2. should we call the createAccount method createInterchainAccount or createICA instead? Tangential, should the return type be InterchainAccount, or InterchainV1Account, a type of ChainAccount like LocalChainAccount?
  3. likewise, should we use a more specific name for createQueryConnection? Right now it just captures async-icq, and there multiple application protocols for querying.
  4. create a task to make createQueryConnection use a provide pattern (task, instead of adjusting scope for #9072 as it's already in review)

    Would be great to get feedback and input 🙂

turadg commented 5 months ago

This requirement is driven by supporting Calypso's use cases. @schnetzlerjoe explains they'll need something like E(calypso).openAccount(chainConfig); where chainConfig is like https://chains.cosmos.directory/agoric

That value comes from an end user of the dapp, so it can only be trusted in the context of that user's interactions. (It may be faulty so should not affect others.)

Therefore the Chain object would not be a canonical one. I think @0xpatrickdev 's design above makes sense: there is no "Chain" object abstracting the chainConfig. Instead there are lower level operations for opening an account with a certain set of parameters.

I propose it be outside the Orchestrator interface because that abstraction is able to coerce a Denom to an ERTP Amount, but doing so requires a stable mapping from the Denom string and that won't be available with user-defined chain configs.