chore(fast-usdc): limited operation before connecting to noble #10641

dckc · 2024-12-07T02:50:41Z

closes: #10657
refs: #10328

Description

advancer, settler can operate without a noble account address
creatorFacet method to synchronize on fulfillment of nobleAccountV vow
restore fast-usdc a3p-integration test to working order

Security / Documentation Considerations

While the noble account vow has not fulfilled, diagnostics for advancing to non-agoric chains are likely to be obscure.

Configuration flexibility leads to possibility of configuration errors.

Scaling Considerations

n/a

Testing Considerations

existing tests, but with set -e so that when fastUsdc is not in agoricNames.instance, we actually notice.

Upgrade Considerations

nobleAccountV persists across incarnations, so we won't try to make another one on restart / upgrade.

TODO:

what to do if nobleAccountV rejects?

turadg · 2024-12-07T02:56:04Z

packages/builders/scripts/fast-usdc/init-fast-usdc.js

@@ -81,9 +84,12 @@ const configurations = {
      },
    },
    chainInfo: /** @type {Record<string, CosmosChainInfo & Passable>} */ (
-      withChainCapabilities(fetchedChainInfo)
+      withChainCapabilities({


please explain in a comment why these two customizations are necessary

good catch; turns out the noble: one is not necessary after all.

p.s. I'm also adding a comment

oh yeah... adding noble is required. to prevent that error

turadg · 2024-12-07T02:57:55Z

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

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

  const makeFeedKit = prepareTransactionFeedKit(zone, zcf);
-  assertAllDefined({ makeFeedKit, makeAdvancer, makeSettler, statusManager });
+  assertAllDefined({ makeFeedKit, makeSettler, statusManager });


this whole line can be removed. they're all made locally.

turadg · 2024-12-07T03:00:21Z

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

  });

-  const advancer = zone.makeOnce('Advancer', () =>
-    makeAdvancer({
+  const { advancer, admin: advancerAdmin } = zone.makeOnce('AdvancerKit', () =>


while separate an admin facet when advancer is never passed out of this scope? It's also exposing two advancer facets that weren't before. Please justify (perhaps in a comment) or revert

after internal discussion, I realized operating with no noble connection is change to the advancer API, so we might as well put the new method there.

0xpatrickdev

LGTM, nice solution keeping the startFn prompt.

what to do if nobleAccountV rejects?

It seems we can't do much without this. I'd suggest including a try/catch in connectToNoble and log loudly if something fails. IRL remediation seems like a contract upgrade / additional proposal if this were to happen.

packages/builders/scripts/fast-usdc/init-fast-usdc.js

0xpatrickdev · 2024-12-09T15:59:16Z

packages/fast-usdc/src/exos/advancer.js

+ * @param {ChainAddress|undefined} intermediateRecipient
+ * @returns {IBCMsgTransferOptions | undefined}
+ */
+const ibcOpts = intermediateRecipient =>


nit: consider moving this helper to utils since we also use it in settler.js. And maybe rename to ibcTransferOpts: IBCMsgTransferOptions to be more specific

For 1-2 liners like this, I tend to think DRY is more trouble than it's worth.

One thing that really bugs me about typescript is that factoring out an expression like this as a function requires all sorts of type annotations. With flow, the tool just figured it out. The only thing you have to annotate is stuff that's exported from a module.

In this case, I may just inline it. intermediateRecipient is allowed to be undefined anyway.

0xpatrickdev · 2024-12-09T16:04:52Z

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

@@ -236,6 +236,11 @@ export const startFastUSDC = async (

  produceInstance.reset();
  produceInstance.resolve(instance);
+
+  if ('uusdc' in assetInfo) {


This if block is to prevent us from calling connectToNoble in the a3p environment? If so, it might conflict with the suggestion to include chainName: 'noble' assets in assetInfo.

Maybe we should have a builderOpt instead, where the caller needs to opt-out?

good idea; I'll try that...

cloudflare-workers-and-pages · 2024-12-09T18:58:52Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`c9ba09e`
Status:	✅ Deploy successful!
Preview URL:	https://4a37413f.agoric-sdk.pages.dev
Branch Preview URL:	https://dc-start-fast-usdc.agoric-sdk.pages.dev

View logs

dckc · 2024-12-09T19:50:44Z

odd ci failure...

k 17 - upgrading › upgrade-vats › upgrade vat-board %ava-dur=84021ms
#   create initial version
#   now perform the null upgrade
dm.invoke failed, informing calling vat (Error#1)
Error#1: unregistered bundle name 'chain'
    at Object.invoke (.../swingset-vat/src/devices/vat-admin/device-vat-admin.js:291:7)

turadg · 2024-12-09T20:12:17Z

packages/builders/scripts/fast-usdc/init-fast-usdc.js

@@ -38,6 +38,7 @@ const options = {
  },
  chainInfo: { type: 'string' },
  assetInfo: { type: 'string' },
+  noNoble: { type: 'boolean', default: false },


consider making the option phrased positively. e.g. connectNoble.

I think the default could still be false. connecting to Noble has strictly more dependencies on the environment so makes sense to be opt-in

yes, it's more dependencies, but without a connection to noble, the whole notion of "fast usdc" goes away. It's supported only for a limited form of testing. If somebody forgot to set the flag, the normal condition should be that the product works.

turadg · 2024-12-09T20:13:51Z

packages/fast-usdc/src/exos/advancer.js

-                intermediateRecipient,
-              },
-            },
+            { denom: usdc.denom, value: advanceAmount.value },


I'm surprised prettier change the second argument too

it didn't. I tried a helper for the 1st arg and then took it out, and used the more concise form when I took it out. I realized the second arg didn't need the line-breaks either.

turadg · 2024-12-09T20:15:58Z

packages/fast-usdc/src/exos/settler.js

     * }} config
     */
    config => {
      log('config', config);
      return {
        ...config,
+        // make sure the state record has this property, perhaps with an undefined value


consider using "the state schema" to suggest the implications

The schema is below in stateShape, right? That's how I knew to write the code this way: it refused to store the state record without this property.

turadg · 2024-12-09T20:17:25Z

packages/fast-usdc/src/types.ts

  chainInfo: Record<string, CosmosChainInfo & Passable>;
  assetInfo: [Denom, DenomDetail & { brandKey?: string }][];
-};
+} & CopyRecord;


consider putting it at the front (like Passable & was) so it's more noticeable.

Why would we want it to be noticeable? It's only here due to limitations in tsc or the Passable type or something.

Why would we want it to be noticeable?

Because it's unexpected

turadg · 2024-12-09T20:17:54Z

packages/fast-usdc/src/utils/deploy-config.js

 /**
 * @type {Record<string, Pick<FastUSDCConfig, 'oracles' | 'feedPolicy' | 'chainInfo' | 'assetInfo' >>}
 *
 * TODO: determine OCW operator addresses
 * meanwhile, use price oracle addresses (from updatePriceFeeds.js).
 */
 export const configurations = {
+  /**
+   * NOTE: The a3p-integration runtime does _not_ include


dckc · 2024-12-09T23:36:55Z

now what?!?!?

#32 52.37 2024-12-09T23:14:06.913Z SwingSet: vat: v72: chainHub: registered connections [ 'agoric-3<->cosmoshub-4', 'agoric-3<->noble-1', 'agoric-3<->omniflixhub-1', 'agoric-3<->osmosis-1', 'agoric-3<->secret-4', 'agoric-3<->stride-1', 'agoric-3<->umee-1' ]
#32 52.37 2024-12-09T23:14:06.913Z SwingSet: vat: v72: chainHub: registering assets [ 'agoric: ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9' ]
#32 52.38 2024-12-09T23:14:06.927Z SwingSet: ls: v72: Logging sent error stack (Error#1)
#32 52.38 2024-12-09T23:14:06.928Z SwingSet: ls: v72: Error#1: must register chain noble first
#32 52.38 2024-12-09T23:14:06.928Z SwingSet: ls: v72: Error: must register chain "noble" first
#32 52.38  at apply ()
#32 52.38  at Error (/bundled-source/.../node_modules/ses/src/error/tame-error-constructor.js:60)

ci error

mergify · 2024-12-10T00:20:25Z

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

❌ integration-test-result
☑️ linear-history

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

- contract start can't wait for noble account creation; must finish in one crank for upgrade - prune allDefined(...) of locally created objects - allow undefined intermediateRecipient in advancer, settler - prune goofy advancer default caps arg - add noNoble config option - Passable obscured that FastUSDCConfig is a record - no noble chain in a3p-integration config

mergify · 2024-12-10T06:12:05Z

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #10641 has been dequeued. The pull request rule doesn't match anymore. The pull request has been synchronized by a user..

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

dckc · 2024-12-10T06:13:06Z

@Mergifyio requeue

mergify · 2024-12-10T06:13:09Z

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

dckc requested review from turadg, samsiegart and 0xpatrickdev December 7, 2024 02:50

turadg requested changes Dec 7, 2024

View reviewed changes

0xpatrickdev approved these changes Dec 9, 2024

View reviewed changes

dckc force-pushed the dc-start-fast-usdc branch from ac8cf06 to b58edbd Compare December 9, 2024 18:58

dckc force-pushed the dc-start-fast-usdc branch from b58edbd to 8056b7c Compare December 9, 2024 19:30

dckc requested a review from turadg December 9, 2024 19:30

dckc marked this pull request as ready for review December 9, 2024 19:48

dckc requested a review from a team as a code owner December 9, 2024 19:48

turadg approved these changes Dec 9, 2024

View reviewed changes

dckc force-pushed the dc-start-fast-usdc branch from 8056b7c to fac5834 Compare December 9, 2024 20:32

dckc self-assigned this Dec 9, 2024

dckc added the automerge:rebase Automatically rebase updates, then merge label Dec 9, 2024

dckc force-pushed the dc-start-fast-usdc branch from fac5834 to 8874419 Compare December 9, 2024 23:42

dckc added the force:integration Force integration tests to run on PR label Dec 10, 2024

dckc added 4 commits December 10, 2024 00:10

chore(a3p-integration): exit on error in fast-usdc a3p test

046779f

feat(builders): --noNoble option for init-fast-usdc

508a3e0

chore(a3p-integration): use --noNoble for fast-usdc

c9ba09e

dckc force-pushed the dc-start-fast-usdc branch from 8874419 to c9ba09e Compare December 10, 2024 06:11

mergify bot merged commit 340ef0d into master Dec 10, 2024
82 checks passed

mergify bot deleted the dc-start-fast-usdc branch December 10, 2024 06:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(fast-usdc): limited operation before connecting to noble #10641

chore(fast-usdc): limited operation before connecting to noble #10641

dckc commented Dec 7, 2024 •

edited

Loading

turadg Dec 7, 2024

dckc Dec 9, 2024 •

edited

Loading

dckc Dec 10, 2024

turadg Dec 7, 2024

turadg Dec 7, 2024

dckc Dec 9, 2024

0xpatrickdev left a comment

0xpatrickdev Dec 9, 2024

dckc Dec 9, 2024

0xpatrickdev Dec 9, 2024

dckc Dec 9, 2024

cloudflare-workers-and-pages bot commented Dec 9, 2024 •

edited

Loading

dckc commented Dec 9, 2024

turadg Dec 9, 2024

dckc Dec 9, 2024

turadg Dec 9, 2024

dckc Dec 9, 2024

turadg Dec 9, 2024

dckc Dec 9, 2024

turadg Dec 9, 2024

dckc Dec 9, 2024

turadg Dec 9, 2024 •

edited by dckc

Loading

turadg Dec 9, 2024

dckc commented Dec 9, 2024

mergify bot commented Dec 10, 2024

mergify bot commented Dec 10, 2024

dckc commented Dec 10, 2024

mergify bot commented Dec 10, 2024

chore(fast-usdc): limited operation before connecting to noble #10641

chore(fast-usdc): limited operation before connecting to noble #10641

Conversation

dckc commented Dec 7, 2024 • edited Loading

Description

Security / Documentation Considerations

Scaling Considerations

Testing Considerations

Upgrade Considerations

Choose a reason for hiding this comment

dckc Dec 9, 2024 • edited Loading

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

0xpatrickdev 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

cloudflare-workers-and-pages bot commented Dec 9, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

dckc commented Dec 9, 2024

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

turadg Dec 9, 2024 • edited by dckc Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc commented Dec 9, 2024

mergify bot commented Dec 10, 2024

mergify bot commented Dec 10, 2024

dckc commented Dec 10, 2024

mergify bot commented Dec 10, 2024

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

dckc commented Dec 7, 2024 •

edited

Loading

dckc Dec 9, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Dec 9, 2024 •

edited

Loading

turadg Dec 9, 2024 •

edited by dckc

Loading