Internal JSDoc for fastUSDC and state transition diagram #11061

Chris-Hibbert · 2025-02-27T01:28:12Z

Description

Add some prose description about how Advancer and Settler work together, and improve the state transition diagram.

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

Internal documentation only. No implications for user docs.

Testing Considerations

None

Upgrade Considerations

None

cloudflare-workers-and-pages · 2025-02-27T01:28:41Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`287ea01`
Status:	✅ Deploy successful!
Preview URL:	https://c8b50fb2.agoric-sdk.pages.dev
Branch Preview URL:	https://cth-fastusdc-doc.agoric-sdk.pages.dev

View logs

Chris-Hibbert · 2025-02-27T01:42:07Z

@0xpatrickdev, would you mind giving me an initial review? Once you've made a pass over it, I'll ask the rest of the team to also look.

dckc · 2025-02-27T06:39:36Z

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

@@ -17,6 +17,12 @@ import {
 } from '../type-guards.js';
 import { makeFeeTools } from '../utils/fees.js';

+/**
+ * @file Advancer subscribes (handleTransactionEvent) to events published by the


@file docs don't tend to show up when navigating code in an IDE, generating docs, etc. How about putting this on prepareAdvancer?

a 1-line /** @file main export: @see {prepareAdvancer} */ at the top could help when looking at the file itself.

I think this @file comment is good but file-level comments should be at the top of the file

dckc · 2025-02-27T06:42:32Z

packages/fast-usdc/src/exos/status-manager.js

-       * Add a new transaction with OBSERVED status
+       * Add a new transaction with OBSERVED status.
+       *
+       * This message isn't currently being sent.


really? it's not?

Please convince me I'm wrong. I couldn't find any instances of it.

I only see it in tests.

I wonder if this is left-over from a design change.

Changed in,

Use skipAdvance rather than observe when precondition checks fail #10994

Let's remove the function since it's not used.

dckc

looks useful

turadg · 2025-02-27T21:21:56Z

packages/fast-usdc/README.md

-  Advancing --> AdvanceFailed
-  Forwarding --> ForwardFailed
-```
+  [*] --> AdvanceSkipped : Risks identified


Please retain the Observed state as it is part of the sequence written to vstorage:

agoric-sdk/packages/fast-usdc/src/exos/status-manager.js

Lines 145 to 151 in 90db359

const publishEvidence = (hash, evidence) => {

// Don't await, just writing to vstorage.

void publishTxnRecord(

hash,

harden({ evidence, status: TxStatus.Observed }),

);

};

turadg · 2025-02-27T21:26:38Z

packages/fast-usdc/README.md

+  [*] --> Advancing : No risks, can advance
+  [*] --> Forwarding* : No risks, Mint deposited before advance
+
+  state Advancer  {


this works visually but if someone knows this syntax, composite states, they might thing that these are composite states.

It's all states of the Transaction and not a composite. The boxes indicate what exo can set transition to that state.

I'm not sure that's useful but if you want to keep it please clarify in a comment on this sytnax.

I thought it was clarifying to separate advancer actions from settler actions, but I can see how calling them composites would be confusing.

I also tried to find a way to use different colors for actions in Advancer and Settler, but I could only see how to color states, and that didn't carry the right meaning.

turadg · 2025-02-27T21:29:06Z

packages/fast-usdc/README.md

-```
+  [*] --> AdvanceSkipped : Risks identified
+  [*] --> Advancing : No risks, can advance
+  [*] --> Forwarding* : No risks, Mint deposited before advance


I don't think this is accurate. If there is no observation there is no transaction to have a state

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

Lines 241 to 245 in 90db359

log('⚠️ tap: minted before observed', nfa, amount);

// XXX consider capturing in vstorage

// we would need a new key, as this does not have a txHash

this.state.mintedEarly.add(makeMintedEarlyKey(nfa, amount));

}

turadg · 2025-02-27T21:30:44Z

packages/fast-usdc/README.md

+  }
+
+  Forwarding* --> Forwarded : settler.forward() succeeds
+  Advancing --> Advanced : advancer's transferHandler detects success


while you're bringing in more sophisticated diagram features, consider using choice for success conditions

thanks. that's hepful

turadg · 2025-02-27T21:31:23Z

packages/fast-usdc/README.md

+  Forwarding* --> ForwardFailed : settler.forward() fails
+ ```
+
+* There is no actual state for **Forwarding**. It is used here to represent the


maybe better expressed within the diagram as a note

I turned Forwarding into a choice rather than a state, so I don't need the note. What do you think of this version?

It also let me get rid of the transition on "Mint deposited before Advance".

turadg · 2025-02-27T21:32:44Z

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

@@ -17,6 +17,12 @@ import {
 } from '../type-guards.js';
 import { makeFeeTools } from '../utils/fees.js';

+/**
+ * @file Advancer subscribes (handleTransactionEvent) to events published by the


I think this @file comment is good but file-level comments should be at the top of the file

turadg · 2025-02-27T21:33:45Z

packages/fast-usdc/src/exos/liquidity-pool.js

-        /** @param {ZCFSeat} lp */
-        async handle(lp) {
+        /** @param {ZCFSeat} lpSeat */
+        async handle(lpSeat) {


I lightly oppose having the type in the name. I think the existing names were clear

mint was very confusing to me. "mint" is a kind in ERTP (the shareMint is used nearby), a verb, a generic noun, and several other things. I don't know how it distinguishes the zcfSeat that hold lpShares that will be transferred to the liquidity provider.

mintedShares might be okay, but it's a seat holding shares, not the shares themselves, and it's certainly not a "mint".

Good reasons against mint but not to retain Seat. The union of our points yields: lp and sharePayout.

turadg · 2025-02-27T21:34:14Z

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

@@ -14,6 +14,13 @@ import {
  makeNatAmountShape,
 } from '../type-guards.js';

+/**
+ * @file Settler is responsible for monitoring (receiveUpcall) deposits to the


ditto about file at top

turadg

Significant improvement! The choice fields really clarify things. I never tried them before.

Approving contingent on fixing AcvancingChoice and the Shares suffix

turadg · 2025-02-28T00:43:06Z

packages/fast-usdc/src/exos/liquidity-pool.js

-        /** @param {ZCFSeat} lp */
-        async handle(lp) {
+        /** @param {ZCFSeat} lpSeat */
+        async handle(lpSeat) {


Good reasons against mint but not to retain Seat. The union of our points yields: lp and sharePayout.

turadg · 2025-02-28T00:44:16Z

packages/fast-usdc/README.md

-  Advancing --> AdvanceFailed
-  Forwarding --> ForwardFailed
-```
+  AcvancingChoice --> AdvanceFailed : advancer's transferHandler detects failure


typo, breaking the image.

but it's not really a choice. Please use AdvancingSuccess or something that describes the range of values.

I’m also not crazy about AdvancingChoice - it seems like a TxStatus.

We are waiting for a cross-chain call to settle here - specifically Success Ack, Error Ack, Timeout from the IBC protocol. Maybe:

Advancing --> Advanced : IBC Success Ack Advancing --> AdvanceFailed : IBC Error Ack or Timeout

I didn't understand @turadg's suggestion, so this iteration follows @0xpatrickdev's version.

it's not really a choice.

Please say more. AIUI, choice describes a situation where the next state depends on some external input, which I think is what the IBC provides.

Please use AdvancingSuccess or something that describes the range of values.

Do you mean to add a state called AdvancingSuccess? There's no such state recorded. Is patrick's pair of transitions clear enough about the possibilities?

0xpatrickdev · 2025-02-28T16:09:00Z

packages/fast-usdc/README.md

-  Advancing --> AdvanceFailed
-  Forwarding --> ForwardFailed
-```
+  AcvancingChoice --> AdvanceFailed : advancer's transferHandler detects failure


I’m also not crazy about AdvancingChoice - it seems like a TxStatus.

We are waiting for a cross-chain call to settle here - specifically Success Ack, Error Ack, Timeout from the IBC protocol. Maybe:

Advancing --> Advanced : IBC Success Ack Advancing --> AdvanceFailed : IBC Error Ack or Timeout

0xpatrickdev · 2025-02-28T16:11:48Z

packages/fast-usdc/README.md

-  Forwarding --> ForwardFailed
-```
+  AcvancingChoice --> AdvanceFailed : advancer's transferHandler detects failure
+  Forwarding --> ForwardFailed : settler.forward() fails


Should we see Forwarding? Not seeing it in the rendered diagram for some reason.

[possible duplicate, I thought I wrote this already.]

The old Forwarding node didn't represent a recorded state, so I was looking for a way to hide it. it's now hidden behind an unlabelled choice node. The code mostly doesn't check the prior state before deciding on a new state, so three states can lead to these transitions.

choice nodes render just as the diamond, not with their label

0xpatrickdev · 2025-02-28T16:19:38Z

packages/fast-usdc/README.md


 ```mermaid
 stateDiagram-v2
+  state Forwarding <<choice>>
+  state AdvancingChoice <<choice>>
+
  Observed --> AdvanceSkipped : Risks identified
  Observed --> Advancing : No risks, can advance
  Observed --> Forwarding : No risks, Mint deposited before advance


Observed --> Forwarding : No risks, Mint deposited before advance

We should expand this more because it's no longer accurate. An invariant is that the contract never Forwards without seeing evidence. When Settler.receiveUpcall fires and there are no matches in StatusManager's pendingTxs, or the found match is Advancing, it goes into the mintedEarly store. Each time Advancer.handleTransactionEvent is invoked, it will call Settler.notifier.checkMintedEarly() and early return if true. checkMintedEarly will clear the item from mintedEarly and initiate a .forward(). Settler.notifier.notifyAdvancingResult, which is called at the end of the Advancing state transition, also checks mintedEarly.

I agree that all that happens, and it complicates our process, but if it complicates the state transitions, it's not obvious to me how it should be represented. AFAICT, the state transitions are unaffected by all of this.

Should we represent those possibilities more explicitly in the states?

Perhaps I could add a note on the choice node listing the conditions that the code pays attention to to decide whether and when to attempt to forward.

0xpatrickdev · 2025-02-28T16:23:32Z

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

@@ -103,6 +105,10 @@ export const stateShape = harden({
 });

 /**
+ * Advancer subscribes (using handleTransactionEvent) to events published by the
+ * transaction feed. When notified of an appropriate opportunity, it is


nb: this could be helpful for navigation:

Suggested change

* transaction feed. When notified of an appropriate opportunity, it is

* {@link TransactionFeedKit}. When notified of an appropriate opportunity, it is

With this up top:

* @import {TransactionFeedKit} from './transaction-feed.js';

0xpatrickdev · 2025-02-28T16:28:55Z

packages/fast-usdc/README.md


-### Complete state diagram (starting from Transaction Feed into Advancer)
+The Settler is responsible for monitoring (via `receiveUpcall`) deposits to the
+settlementAccount. It either `disburse`s funds to the Pool (if funds were


The Settler is responsible for monitoring (via receiveUpcall) deposits to the
settlementAccount.

Is it worth mentioning how receiveUpcall works? The Settler also needs to register a handler for the settlementAccount: OrchestrationAccount - that's what sets up the events:

E(settlementAccount).monitorTransfers(settler.tap); // "tap" is any durable exo with a `receiveUpcall` handler

I don't think that goes in the state diagram, but I can put it in the class comment.

0xpatrickdev · 2025-02-28T16:31:08Z

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

+ * Settler is responsible for monitoring (using receiveUpcall) deposits to the
+ * settlementAccount. It either "disburses" funds to the Pool (if funds were
+ * "advance"d to the payee), or "forwards" funds to the payee (if pool funds
+ * were not advanced).


I think it's worth mentioning:

it needs to register it's tap facet (receiveUpcall) via the settlementAccount so it gets events

something about the mintedEarly flow - we don't .forward() until notifier.checkMinted is called by the Advancer.

I mentioned the registration.

In Settler, self.forward() is called in

receiveUpcall in Observed, AdvanceSkipped, and AdvanceFailed

notifyAdvancingResult() if mintedEarly and no success, or

checkMintedEarly() if mintedEarly has the tx.

The first of these doesn't seem to match what the second bullet says. Would you rephrase?

0xpatrickdev · 2025-02-28T16:32:14Z

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

@@ -300,6 +307,9 @@ export const prepareSettler = (
      },
      self: {
        /**
+         * The intended payee received an advance from the pool. When the funds
+         * are minted disburse them to the pool.


Suggested change

* are minted disburse them to the pool.

* are minted disburse them to the pool and fee seats.

that's right. Thanks.

0xpatrickdev · 2025-02-28T16:35:09Z

packages/fast-usdc/src/exos/status-manager.js

-      observe(evidence) {
-        initPendingTx(evidence, PendingTxStatus.Observed);
-      },
-


Thanks. I think we also need some cleanup in status-manager.test.ts. Some tests can be removed, and others should be updated to use .advanceSkipped.

0xpatrickdev · 2025-02-28T16:37:21Z

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

@@ -103,6 +105,10 @@ export const stateShape = harden({
 });

 /**
+ * Advancer subscribes (using handleTransactionEvent) to events published by the
+ * transaction feed. When notified of an appropriate opportunity, it is
+ * responsible for advancing funds to fastUSDC payees.


fastUSDC payees

Might be worth working in EUD.

0xpatrickdev · 2025-02-28T18:24:27Z

@Chris-Hibbert P.S. i worked on these at one point, but they were never completed / checked in. It includes:

Notify Advancing Result Settlement Flow which demonstrates what happens after an advance succeeds in the Settler.
Pending Settlement State Diagram which might be helpful to incorporate or take a peek at. But it also lacks a representation of mintedEarly in between NoStatus and Forwarding.

And this was an attempt to get everything into one diagram, but it didn't end being very readable. It was created with LLM-assistance and might contain mistakes, but i think the bulk of it is accurate:
https://gist.github.com/0xpatrickdev/d343549f12f7754e25f3f13795025583#file-diagram-md

dckc · 2025-03-01T03:35:20Z

packages/fast-usdc/test/exos/settler.test.ts

-    { evidence: cctpTxEvidence, status: 'OBSERVED' },
+    { evidence: cctpTxEvidence, status: undefined },


I'm surprised to see OBSERVED replaced by undefined.

It makes sense to see the .observed() method go away, but not the state.

Chris-Hibbert added documentation Improvements or additions to documentation code-style defensive correctness patterns; readability thru consistency labels Feb 27, 2025

Chris-Hibbert self-assigned this Feb 27, 2025

Chris-Hibbert requested a review from a team as a code owner February 27, 2025 01:28

Chris-Hibbert requested a review from AgoricTriage February 27, 2025 01:28

Chris-Hibbert force-pushed the cth-fastUSDC-doc branch from 82c6605 to 1265dea Compare February 27, 2025 01:36

Chris-Hibbert added 2 commits February 26, 2025 17:39

docs: document fastUSDC's processing plan and state transitions

6602a58

refactor: rename seats in liquidityPool

90db359

Chris-Hibbert force-pushed the cth-fastUSDC-doc branch from 1265dea to 90db359 Compare February 27, 2025 01:39

Chris-Hibbert requested a review from 0xpatrickdev February 27, 2025 01:41

dckc reviewed Feb 27, 2025

View reviewed changes

Chris-Hibbert requested a review from turadg February 27, 2025 18:53

turadg requested changes Feb 27, 2025

View reviewed changes

docs: diagram improvements and moved file comments to definitions

3a43f53

Chris-Hibbert requested a review from turadg February 28, 2025 00:37

turadg approved these changes Feb 28, 2025

View reviewed changes

0xpatrickdev reviewed Feb 28, 2025

View reviewed changes

Chris-Hibbert added 3 commits February 28, 2025 17:35

docs: improvements to state transition and responsibility write-ups

60ae2d6

test: update tests for loss of 'Observed' state, etc.

8e9b99a

refactor: improvements to names of variables holding seats

287ea01

dckc reviewed Mar 1, 2025

View reviewed changes

	const publishEvidence = (hash, evidence) => {
	// Don't await, just writing to vstorage.
	void publishTxnRecord(
	hash,
	harden({ evidence, status: TxStatus.Observed }),
	);
	};

	log('⚠️ tap: minted before observed', nfa, amount);
	// XXX consider capturing in vstorage
	// we would need a new key, as this does not have a txHash
	this.state.mintedEarly.add(makeMintedEarlyKey(nfa, amount));
	}

	* transaction feed. When notified of an appropriate opportunity, it is
	* {@link TransactionFeedKit}. When notified of an appropriate opportunity, it is

	* are minted disburse them to the pool.
	* are minted disburse them to the pool and fee seats.

		{ evidence: cctpTxEvidence, status: 'OBSERVED' },
		{ evidence: cctpTxEvidence, status: undefined },

Internal JSDoc for fastUSDC and state transition diagram #11061

Are you sure you want to change the base?

Internal JSDoc for fastUSDC and state transition diagram #11061

Conversation

Chris-Hibbert commented Feb 27, 2025

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Feb 27, 2025 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

Chris-Hibbert commented Feb 27, 2025

dckc Feb 27, 2025 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc 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

turadg left a comment • 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

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 Feb 28, 2025 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xpatrickdev Feb 28, 2025 • 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

Choose a reason for hiding this comment

0xpatrickdev commented Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Feb 27, 2025 •

edited

Loading

dckc Feb 27, 2025 •

edited

Loading

turadg Feb 27, 2025 •

edited

Loading

turadg left a comment •

edited

Loading

0xpatrickdev Feb 28, 2025 •

edited

Loading

0xpatrickdev Feb 28, 2025 •

edited

Loading

0xpatrickdev commented Feb 28, 2025 •

edited

Loading