fix(fast-usdc): update noble connection info, make new ICA #11025

dckc · 2025-02-20T17:16:11Z

Description

feat: methods to update chainHub
- feat: expose chainHub on creatorFacet
fix: write off original noble ICA vow in baggage
core eval to update noble connection info, make new ICA
- builder for core eval with CLI args to handle starship connection info

TODO:

in fast-usdc.contract.js: address upgrade TODOs
consider combining this core-eval with fee updates and such

Security Considerations

creatorFacet.getChainHub() increases the surface area of the contract.
Since the creator provided chainInfo and assetInfo in privateArgs to start with, it's a small increase.

Scaling Considerations

none, I think.

Testing Considerations

unit tests for chainHub methods
bootstrap test for upgrade
- downloads materials from fast-usdc-beta-1 release to start the contract in pre-upgrade state

TODO:

finish multichain-testing (I switched to working in boot/. maybe this works now?)

Upgrade / Documentation Considerations

OCW operators have to accept their invitations before the upgrade, since their invitations are attached to ephemeral offer handlers.

TODO:

consider making the handlers durable and issuing new invitations

- adds `updateChain`, `updateConnection`, and `updateAsset` interfaces to `ChainHub`

... used in mainnet proposal 87 - skip start-fast-usdc in favor of prop 87 - never ack initial noble ICA creation

- snapshot fastUsdc.nobleICA

Their invitations are made with non-durable offer handlers.

cloudflare-workers-and-pages · 2025-02-20T17:16:36Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`d613d1a`
Status:	✅ Deploy successful!
Preview URL:	https://e3f8675f.agoric-sdk.pages.dev
Branch Preview URL:	https://dc-noble-conn.agoric-sdk.pages.dev

View logs

dckc · 2025-02-20T17:17:52Z

multichain-testing/test/support.ts

-    );
-    if (contractName in instances) {
-      return t.log('Contract found. Skipping installation...');
+    if (contractBuilder.endsWith('/start-fast-usdc.core.js')) {


this is a bit of a KLUDGE. Once I can tell that it's working, I expect to refactor it.

see XXX note below

Suggestion: add a 4th opts bag argument to startContract, { skipInstanceCheck = false }, to make this declarative.

dckc · 2025-02-20T17:30:04Z

packages/fast-usdc/src/update-noble-ica.core.js

+  trace('noble ICA', nobleICAaddr);
+
+  // publish ICA addr at published.fastUsdc.nobleICA
+  // TODO: update vstorage snapshot tests


this is done.

Suggested change

// TODO: update vstorage snapshot tests

dckc · 2025-02-20T17:30:19Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -279,6 +285,8 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
  const [_agoric, _noble, agToNoble] = await vowTools.when(
    chainHub.getChainsAndConnection('agoric', 'noble'),
  );
+  // 1) should we have used zone.makeOnce here?


yes, to preserve precious state (mintedEarly). TODO.

let's discuss synchronously.

dckc · 2025-02-20T17:30:44Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -279,6 +285,8 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
  const [_agoric, _noble, agToNoble] = await vowTools.when(
    chainHub.getChainsAndConnection('agoric', 'noble'),
  );
+  // 1) should we have used zone.makeOnce here?
+  // 2) will we get an error since we're trying to register a tap on the same LCA?


no, I gather. NEEDSTEST?

likewise, no re monitorMintingDeposits below.

turadg

We're going to need an a3p-integration proposal test eventually (since this proposal has to go into agoric-3-proposals) but using bootstrap suffices for merging to master.

turadg · 2025-02-20T17:45:37Z

packages/boot/test/fast-usdc/snapshots/fast-usdc.test.ts.md

+
+    [
+      [
+        'published.fastUsdc.nobleICA',


why create this node?

Aside from being generally useful for grabbing the address for use in explorers and such, it's a handy way to tell whether this core eval succeeded.

Perhaps we should publish just the .value of the address?

why not put it on published.fastUsdc with the other addresses? (e.g. poolAddress)

sure; that will work.

turadg · 2025-02-20T17:47:58Z

packages/boot/tools/supports.ts

@@ -294,6 +294,8 @@ export const AckBehavior = {
  Queued: 'QUEUED',
  /** inbound messages are delivered immediately */
  Immediate: 'IMMEDIATE',
+  /** inbound responses never arrive (to simulate mis-configured connections etc.) */
+  Never: 'NEVER',


nice. do we need this implemented in all cases?

there are other cases? help?

just looking at that switch, you handled case 'startChannelOpenInit' but no change to case 'sendPacket'

turadg · 2025-02-20T17:49:33Z

packages/builders/scripts/fast-usdc/fast-usdc-reconfigure.build.js

+  const { writeCoreEval } = await makeHelpers(homeP, endowments);
+
+  /** @returns {{ agoricToNoble?: IBCConnectionInfo } & Passable} */
+  const parseConnection = () => {


usually these builder functions are almost declarative.

consider moving this up to module scope,

export const parseConnection = (args) => {

that would also allow unit testing

turadg · 2025-02-20T17:52:57Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -197,6 +197,9 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
    deleteCompletedTxs() {
      return statusManager.deleteCompletedTxs();
    },
+    getChainHub() {


food for thought: withOrchestration could automatically provide such methods, wrapping the contract return value. E.g. instead of creatorFacet it could return orchestrationFacet or something.

probably too much magic as yet

turadg · 2025-02-20T17:54:12Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -258,7 +261,10 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
    );
  }

-  const nobleAccountV = zone.makeOnce('NobleAccount', () => makeNobleAccount());
+  // v1 has `NobleAccount` which we don't expect to ever settle. how can we delete it?
+  const nobleAccountV = zone.makeOnce('NobleAccount2', () =>


maybe we can give this a name that doesn't reek of "oops"

perhaps with the channel id?

Suggested change

const nobleAccountV = zone.makeOnce('NobleAccount2', () =>

const nobleAccountV = zone.makeOnce('NobleAccount-62', () =>

or, NobleICA

turadg · 2025-02-20T17:54:23Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -279,6 +285,8 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
  const [_agoric, _noble, agToNoble] = await vowTools.when(
    chainHub.getChainsAndConnection('agoric', 'noble'),
  );
+  // 1) should we have used zone.makeOnce here?


let's discuss synchronously.

turadg · 2025-02-20T17:55:14Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -305,6 +313,7 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
    },
  });

+  // TODO do we need to wrap this in `if (!baggage.has(firstIncarnationKey))` ?


depends on whether settlerKit persists. it was designed at first to be per incarnation

I think with mintedEarly: MapStore in its state warrants making a singleton

turadg · 2025-02-20T17:56:58Z

packages/orchestration/src/exos/chain-hub.js

+        bech32PrefixToChainName.delete(oldInfo.bech32Prefix);
+      }
+      chainInfos.set(chainName, chainInfo);
+      if (chainInfo.bech32Prefix) {


why would new info and old info have different presence of bech32prefix?

i'm not sure why someone would do this, but i think it's a foreseeable state this code needs to handle

turadg · 2025-02-20T17:57:36Z

packages/orchestration/src/exos/chain-hub.js

+     * @throws {Error} If asset not registered or referenced chains not
+     *   registered
+     */
+    updateAsset(denom, detail) {


since it's replacing the entry replaceAsset ?

if we want replace*, we should make the same change on the *connection, *chain methods

0xpatrickdev · 2025-02-20T19:06:09Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -258,7 +261,10 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
    );
  }

-  const nobleAccountV = zone.makeOnce('NobleAccount', () => makeNobleAccount());
+  // v1 has `NobleAccount` which we don't expect to ever settle. how can we delete it?
+  const nobleAccountV = zone.makeOnce('NobleAccount2', () =>


or, NobleICA

0xpatrickdev · 2025-02-20T19:06:23Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -279,6 +285,8 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
  const [_agoric, _noble, agToNoble] = await vowTools.when(
    chainHub.getChainsAndConnection('agoric', 'noble'),
  );
+  // 1) should we have used zone.makeOnce here?
+  // 2) will we get an error since we're trying to register a tap on the same LCA?


0xpatrickdev · 2025-02-20T19:07:27Z

packages/fast-usdc/src/fast-usdc.contract.js

@@ -305,6 +313,7 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
    },
  });

+  // TODO do we need to wrap this in `if (!baggage.has(firstIncarnationKey))` ?


I think with mintedEarly: MapStore in its state warrants making a singleton

0xpatrickdev · 2025-02-20T19:08:26Z

packages/orchestration/src/exos/chain-hub.js

+      if (!chainInfos.has(chainName)) {
+        throw makeError(`Chain ${q(chainName)} not registered`);
+      }
+      if (!chainInfos.has(baseName)) {
+        throw makeError(`Chain ${q(baseName)} not registered`);
+      }


This code was written in haste - i think this block should be above the one where we brandDenoms.delete

0xpatrickdev · 2025-02-20T19:09:31Z

packages/orchestration/src/exos/chain-hub.js

+     * @throws {Error} If asset not registered or referenced chains not
+     *   registered
+     */
+    updateAsset(denom, detail) {


if we want replace*, we should make the same change on the *connection, *chain methods

0xpatrickdev · 2025-02-20T19:10:57Z

packages/orchestration/src/exos/chain-hub.js

+        bech32PrefixToChainName.delete(oldInfo.bech32Prefix);
+      }
+      chainInfos.set(chainName, chainInfo);
+      if (chainInfo.bech32Prefix) {


i'm not sure why someone would do this, but i think it's a foreseeable state this code needs to handle

0xpatrickdev · 2025-02-20T19:13:43Z

multichain-testing/test/support.ts

-    );
-    if (contractName in instances) {
-      return t.log('Contract found. Skipping installation...');
+    if (contractBuilder.endsWith('/start-fast-usdc.core.js')) {


Suggestion: add a 4th opts bag argument to startContract, { skipInstanceCheck = false }, to make this declarative.

0xpatrickdev · 2025-02-20T19:16:35Z

multichain-testing/test/fast-usdc/fast-usdc.test.ts

+test.serial('reconfigure: fix noble ICA', async t => {
+  const { startContract, commonBuilderOpts } = t.context;
+  const builder =
+    '../packages/builders/scripts/fast-usdc/fast-usdc-reconfigure.build.js';


Nice. Thinking out loud...

Is it critical we have a reference to the original bundleId / build ? i.e. something without the new creator facet methods?

Probably not for now, the faulty chainInfo seems sufficient. Likely just something to think about for the future

Here's hoping we get a3p + multichain better connected:

multichain-e2e: resume a3p chain instead of gentx #10079

0xpatrickdev and others added 12 commits February 20, 2025 10:33

feat(fast-usdc): ChainHub update interfaces

406375a

- adds `updateChain`, `updateConnection`, and `updateAsset` interfaces to `ChainHub`

feat(fast-usdc): creatorFacet.getChainHub

f4b5791

WIP(fast-usdc): nobleAccountV upgrade notes

14017a9

feat(fast-usdc): core eval to update chainInfo, noble ICA

047d65a

feat(builders): to reconfigure Fast USDC

9ae76bc

test(boot): fix 'oracles provision...' to do all, not just 1

853c49c

test(boot): start fastUsdc using fast-usdc-beta-1 release

8e356da

... used in mainnet proposal 87 - skip start-fast-usdc in favor of prop 87 - never ack initial noble ICA creation

test(boot): upgrade Fast USDC; update noble ICA

f27ddd4

- snapshot fastUsdc.nobleICA

test(boot): oracles accept before upgrade

b29192a

Their invitations are made with non-durable offer handlers.

WIP(multichain): don't skip contract install when redoing ICA (WIP)

a1b9df3

test(multichain): misconfigure noble->agoric conn like mainnet (WIP)

42c8898

test(multichain): reconfigure: fix noble ICA (WIP)

d613d1a

dckc requested review from turadg and 0xpatrickdev February 20, 2025 17:16

dckc commented Feb 20, 2025

View reviewed changes

turadg reviewed Feb 20, 2025

View reviewed changes

0xpatrickdev reviewed Feb 20, 2025

View reviewed changes

	const nobleAccountV = zone.makeOnce('NobleAccount2', () =>
	const nobleAccountV = zone.makeOnce('NobleAccount-62', () =>

fix(fast-usdc): update noble connection info, make new ICA #11025

Are you sure you want to change the base?

fix(fast-usdc): update noble connection info, make new ICA #11025

Conversation

dckc commented Feb 20, 2025

Description

Security Considerations

Scaling Considerations

Testing Considerations

Upgrade / Documentation Considerations

cloudflare-workers-and-pages bot commented Feb 20, 2025

Deploying agoric-sdk with Cloudflare Pages

dckc Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc Feb 20, 2025 •

edited

Loading

dckc Feb 20, 2025 •

edited

Loading

dckc Feb 20, 2025 •

edited

Loading