feat: fusdc advancer #10420

0xpatrickdev · 2024-11-07T15:22:49Z

refs: #10390

Description

advancer performs balance check
advancer requests payment from LiquidityPool
advancer deposits payment in PoolAccount
advancer submits IBC transfer

Security Considerations

Deals with live payments and ensures they do not get lost during failure paths

Scaling Considerations

No new ones introduced

Documentation Considerations

Testing Considerations

Includes tests

Upgrade Considerations

NA, unreleased

cloudflare-workers-and-pages · 2024-11-07T15:26:22Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`99707ef`
Status:	✅ Deploy successful!
Preview URL:	https://e35dd1d4.agoric-sdk.pages.dev
Branch Preview URL:	https://pc-fusdc-advancer.agoric-sdk.pages.dev

View logs

turadg · 2024-11-14T18:00:07Z

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

 import { assertAllDefined } from '@agoric/internal';
 import { ChainAddressShape } from '@agoric/orchestration';
+import { pickFacet } from '@agoric/vat-data';


out of scope, but this helper should move eventually. It's about Exos, not vat data.

We could move it down to @agoric/internal but that is meant to not be consumed externally and this is valuable to other contract authors.

I wonder if it belongs in Endo.

turadg · 2024-11-14T18:05:00Z

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

+ */
+
+/** type guards internal to the AdvancerKit */
+const WatcherHandlersShape = {


Shape is for objects that get passed around. This is an interface guard. It's a kit interface guard but you could still call it AdvancerI.

Why is this defined here and then composed with advancer in the Exo interface param?

I was getting cute and trying to obscure the Vow handlers from the actual external interface. Changed to AdvancerI

turadg · 2024-11-14T18:14:50Z

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

- * @param {LogFn} caps.log
- * @param {StatusManager} caps.statusManager
- * @param {VowTools} caps.vowTools
+ * @param {AdvancerKitCaps} caps


this name makes sense. Elsewhere we've called it io. I don't think the style guide takes a position

powers is used similarly. Sometimes tools, though those are usually more closely related.

caps is not a dictionary word (or rather: it is, but it means "things you put on your head"). For very local usage, that doesn't matter much, but AdvancerKitCaps looks like something that might have a wider scope. Maybe AdvancerKitPowers instead?

oh yeah, Powers. I think powers is strictly better than caps

turadg · 2024-11-14T18:29:13Z

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

-        }
+      advancer: {
+        /**
+         * Returns a Promise for a Vow in the happy path, or undefined


please put this in @returns

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

turadg · 2024-11-14T18:41:23Z

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

+            try {
+              // should LiquidityPool return a vow here?
+              const { USDC: advancePmtP } =
+                await assetManagerFacet.borrowUnderlying(requestedAmount);


what's underlying? did you mean underwriting?

Underlying assets of the pool, not to be confused with shares, the other asset it owns. We can probably simplify to borrow() and ~~return()~~repay()

turadg · 2024-11-14T18:50:05Z

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

-          destination,
-          amount: requestedAmount.value,
-        });
+              // do we actually need to await here?


deposit takes a payment so yes, unless borrowUnderlying returns a payment instead of a paymentP

turadg · 2024-11-14T18:53:37Z

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

+              const depositV = E(poolAccount).deposit(advancePmt);
+              return watch(depositV, this.facets.depositHandler, destination);
+            } catch (e) {
+              // TODO how should we think about failure here?


the deposit is unlikely to fail but the borrow could if the balance changes since it was looked up. Even though we're using a ZCFSeat we should design for if this were an ICA.

Ok I've put a TODO for borrow failing. I think we can flesh that out more when we integrate the with the LiquidityPool.

For .deposit() failing, we can catch this separately in onRejected() and attempt to return the payment. The LP might need an interface for this, or return() will need to be trained to handle this scenario (right now, it's expected fees + principal).

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

packages/orchestration/src/cosmos-api.ts

turadg

Some small requests. I wouldn't object to them coming in a later PR.

I didn't carefully review error handling because we have that scheduled and it's probably best to tackle that holistically.

turadg · 2024-11-15T16:55:19Z

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

 */

+/**
+ * Expected interface from LiquidityPool


I'll assume this @dckc agrees

turadg · 2024-11-15T16:56:20Z

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

+ *  chainHub: ChainHub;
+ *  log: LogFn;
+ *  statusManager: StatusManager;
+ *  USDC: { brand: Brand<'nat'>; denom: Denom; };


non-blocking: I think we should refer to this as usdc in code. Treat "USDC" as a word and capitalize accordingly. Fully capitalized initialisms are to convey that each letter represents another word (United States Dollar Coin) but that's not relevant to the contract.

turadg · 2024-11-15T16:58:08Z

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

-     *   localDenom: Denom;
-     *   poolAccount: HostInterface<OrchestrationAccount<{ chainId: 'agoric' }>>;
+     *   assetManagerFacet: AssetManagerFacet;
+     *   poolAccount: ERef<HostInterface<OrchestrationAccount<{chainId: 'agoric';}>>>


nit: ; unnecessary and a little distracting

turadg · 2024-11-15T16:59:37Z

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

-        localDenom: M.string(),
-        poolAccount: M.remotable(),
+        assetManagerFacet: M.remotable(),
+        poolAccount: M.or(VowShape, M.remotable()),


TODO pick one

This seems like it'll depend on:

how we create the LCA (I believe zone.makeOnce() will return a Vow)

what incarnation we're in (first time it's a Vow, subsequently, a Remotable)

turadg · 2024-11-15T17:01:24Z

packages/fast-usdc/src/typeGuards.js

@@ -37,3 +37,8 @@ export const PendingTxShape = {
  status: M.or(...Object.values(PendingTxStatus)),
 };
 harden(PendingTxShape);
+
+export const QueryParamsShape = {


this is too generic and is easily confused with QueryParamsShape in address.js

consider EudParamShape

turadg · 2024-11-15T17:02:09Z

packages/fast-usdc/src/utils/address.js

+
+/**
+ * Default pattern matcher for `getQueryParams`.
+ * Does not assert keys exist, but ensures existing keys are strings.


I think this is already guaranteed by your parser, but it doesn't hurt

Agree for the getQueryParams fn, but {} seems to satisfy M.splitRecord({}, {}, M.recordOf(M.string(), M.string()))

- refs: #10390

0xpatrickdev changed the base branch from master to pc/fusdc-status-manager November 7, 2024 15:23

0xpatrickdev force-pushed the pc/fusdc-advancer branch from 751d762 to ffe20a3 Compare November 7, 2024 16:02

0xpatrickdev force-pushed the pc/fusdc-status-manager branch 2 times, most recently from 444aedd to 88f51f3 Compare November 8, 2024 15:14

0xpatrickdev force-pushed the pc/fusdc-advancer branch 2 times, most recently from 725edd7 to 3483477 Compare November 8, 2024 20:47

0xpatrickdev force-pushed the pc/fusdc-status-manager branch from 88f51f3 to c1b9538 Compare November 8, 2024 23:55

0xpatrickdev force-pushed the pc/fusdc-advancer branch from 3483477 to 9c4e291 Compare November 9, 2024 00:02

0xpatrickdev force-pushed the pc/fusdc-status-manager branch 4 times, most recently from 8acd20c to a92b0ec Compare November 11, 2024 19:54

0xpatrickdev force-pushed the pc/fusdc-advancer branch 3 times, most recently from 229333d to 6f0bd04 Compare November 11, 2024 20:18

0xpatrickdev force-pushed the pc/fusdc-status-manager branch 2 times, most recently from 9eac623 to f3d1e36 Compare November 12, 2024 19:27

Base automatically changed from pc/fusdc-status-manager to master November 12, 2024 20:25

0xpatrickdev force-pushed the pc/fusdc-advancer branch 2 times, most recently from 37d455f to 1673c45 Compare November 13, 2024 21:31

turadg reviewed Nov 14, 2024

View reviewed changes

0xpatrickdev force-pushed the pc/fusdc-advancer branch from 1673c45 to 9de3393 Compare November 15, 2024 16:29

types: .deposit returns Amount<'nat'>

d1d7829

0xpatrickdev force-pushed the pc/fusdc-advancer branch from 9de3393 to 165b3d7 Compare November 15, 2024 16:34

0xpatrickdev marked this pull request as ready for review November 15, 2024 16:34

0xpatrickdev requested a review from a team as a code owner November 15, 2024 16:34

0xpatrickdev requested review from AgoricTriage, dckc and turadg November 15, 2024 16:34

turadg approved these changes Nov 15, 2024

View reviewed changes

0xpatrickdev added 2 commits November 15, 2024 12:18

feat: Advancer exo behaviors

4cd2f3f

- refs: #10390

feat: getQueryParams takes shape parameter

99707ef

0xpatrickdev force-pushed the pc/fusdc-advancer branch from 165b3d7 to 99707ef Compare November 15, 2024 17:20

0xpatrickdev added the automerge:rebase Automatically rebase updates, then merge label Nov 15, 2024

mergify bot merged commit 40ad937 into master Nov 15, 2024
91 checks passed

mergify bot deleted the pc/fusdc-advancer branch November 15, 2024 18:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: fusdc advancer #10420

feat: fusdc advancer #10420

0xpatrickdev commented Nov 7, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Nov 7, 2024 •

edited

Loading

turadg Nov 14, 2024

turadg Nov 14, 2024

0xpatrickdev Nov 15, 2024

turadg Nov 14, 2024

dckc Nov 14, 2024

turadg Nov 14, 2024

turadg Nov 14, 2024

turadg Nov 14, 2024

0xpatrickdev Nov 14, 2024 •

edited

Loading

turadg Nov 14, 2024

turadg Nov 14, 2024

0xpatrickdev Nov 15, 2024

turadg left a comment

turadg Nov 15, 2024

turadg Nov 15, 2024

turadg Nov 15, 2024

turadg Nov 15, 2024

0xpatrickdev Nov 15, 2024 •

edited

Loading

turadg Nov 15, 2024

turadg Nov 15, 2024

0xpatrickdev Nov 15, 2024

feat: fusdc advancer #10420

feat: fusdc advancer #10420

Conversation

0xpatrickdev commented Nov 7, 2024 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Nov 7, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

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

0xpatrickdev Nov 14, 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

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

0xpatrickdev Nov 15, 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

0xpatrickdev commented Nov 7, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Nov 7, 2024 •

edited

Loading

0xpatrickdev Nov 14, 2024 •

edited

Loading

0xpatrickdev Nov 15, 2024 •

edited

Loading