From 65695726decd478cc166589d0e5cf61dce992f16 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Wed, 20 Nov 2024 17:34:18 -0500 Subject: [PATCH 1/6] feat(status-manager): simplify `seenTx` key - no need to support legacy ethereum txs that do not include chain id in the tx evelope - aligns implementation with vstorage plans --- packages/fast-usdc/src/exos/status-manager.js | 11 +++++------ packages/fast-usdc/test/exos/advancer.test.ts | 2 +- packages/fast-usdc/test/exos/status-manager.test.ts | 6 ++---- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/fast-usdc/src/exos/status-manager.js b/packages/fast-usdc/src/exos/status-manager.js index c77f6f0817f..c2d071ed610 100644 --- a/packages/fast-usdc/src/exos/status-manager.js +++ b/packages/fast-usdc/src/exos/status-manager.js @@ -14,8 +14,8 @@ import { PendingTxStatus } from '../constants.js'; /** * Create the key for the pendingTxs MapStore. * - * The key is a composite of `txHash` and `chainId` and not meant to be - * parsable. + * The key is a composite of Noble address `addr` and transaction `amount` and + * not meant to be parsable. * * @param {NobleAddress} addr * @param {bigint} amount @@ -38,15 +38,14 @@ const pendingTxKeyOf = evidence => { /** * Get the key for the seenTxs SetStore. * - * The key is a composite of `NobleAddress` and transaction `amount` and not - * meant to be parsable. + * The key is based on `txHash` and not meant to be parsable. * * @param {CctpTxEvidence} evidence * @returns {SeenTxKey} */ const seenTxKeyOf = evidence => { - const { txHash, chainId } = evidence; - return `seenTx:${JSON.stringify([txHash, chainId])}`; + const { txHash } = evidence; + return `seenTx:${txHash}`; }; /** diff --git a/packages/fast-usdc/test/exos/advancer.test.ts b/packages/fast-usdc/test/exos/advancer.test.ts index fb21deabf86..deef0b27fc0 100644 --- a/packages/fast-usdc/test/exos/advancer.test.ts +++ b/packages/fast-usdc/test/exos/advancer.test.ts @@ -399,7 +399,7 @@ test('will not advance same txHash:chainId evidence twice', async t => { t.deepEqual(inspectLogs(1), [ 'Advancer error:', - '"[Error: Transaction already seen: \\"seenTx:[\\\\\\"0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761702\\\\\\",1]\\"]"', + '"[Error: Transaction already seen: \\"seenTx:0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761702\\"]"', ]); }); diff --git a/packages/fast-usdc/test/exos/status-manager.test.ts b/packages/fast-usdc/test/exos/status-manager.test.ts index 8b87fe23ddb..9c76e7f231c 100644 --- a/packages/fast-usdc/test/exos/status-manager.test.ts +++ b/packages/fast-usdc/test/exos/status-manager.test.ts @@ -46,18 +46,16 @@ test('cannot process same tx twice', t => { t.throws(() => statusManager.advance(evidence), { message: - 'Transaction already seen: "seenTx:[\\"0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761702\\",1]"', + 'Transaction already seen: "seenTx:0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761702"', }); t.throws(() => statusManager.observe(evidence), { message: - 'Transaction already seen: "seenTx:[\\"0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761702\\",1]"', + 'Transaction already seen: "seenTx:0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761702"', }); // new txHash should not throw t.notThrows(() => statusManager.advance({ ...evidence, txHash: '0xtest2' })); - // new chainId with existing txHash should not throw - t.notThrows(() => statusManager.advance({ ...evidence, chainId: 9999 })); }); test('settle removes entries from PendingTxs', t => { From d0db7106773ccabfa73375aceb222753b365b928 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Wed, 20 Nov 2024 17:45:43 -0500 Subject: [PATCH 2/6] fix(status-manager): `hasPendingSettlement` --- packages/fast-usdc/src/exos/status-manager.js | 1 + .../test/exos/status-manager.test.ts | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/packages/fast-usdc/src/exos/status-manager.js b/packages/fast-usdc/src/exos/status-manager.js index c2d071ed610..ff27dcdb553 100644 --- a/packages/fast-usdc/src/exos/status-manager.js +++ b/packages/fast-usdc/src/exos/status-manager.js @@ -129,6 +129,7 @@ export const prepareStatusManager = zone => { */ hasPendingSettlement(address, amount) { const key = makePendingTxKey(address, amount); + if (!pendingTxs.has(key)) return false; const pending = pendingTxs.get(key); return !!pending.length; }, diff --git a/packages/fast-usdc/test/exos/status-manager.test.ts b/packages/fast-usdc/test/exos/status-manager.test.ts index 9c76e7f231c..793b04d88d6 100644 --- a/packages/fast-usdc/test/exos/status-manager.test.ts +++ b/packages/fast-usdc/test/exos/status-manager.test.ts @@ -178,3 +178,24 @@ test('StatusManagerKey logic handles addresses with hyphens', async t => { ); t.is(remainingEntries.length, 0, 'Entry should be settled'); }); + +test('hasPendingSettlement code paths', t => { + const zone = provideDurableZone('status-test'); + const statusManager = prepareStatusManager(zone.subZone('status-manager')); + const evidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(); + const { forwardingAddress, amount } = evidence.tx; + + // Nothing in store + t.false(statusManager.hasPendingSettlement(forwardingAddress, amount)); + + // Observed in store + statusManager.observe(evidence); + t.true(statusManager.hasPendingSettlement(forwardingAddress, amount)); + statusManager.settle(forwardingAddress, amount); + + // Advanced in store + statusManager.advance({ ...evidence, txHash: '0xtest2' }); + t.true(statusManager.hasPendingSettlement(forwardingAddress, amount)); + statusManager.settle(forwardingAddress, amount); + t.false(statusManager.hasPendingSettlement(forwardingAddress, amount)); +}); From 72a3cef40b034c85da9e5661100b9eabfae60018 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Wed, 20 Nov 2024 17:48:18 -0500 Subject: [PATCH 3/6] docs: move status manager state diagrams --- packages/fast-usdc/README.md | 26 ++++++++++++++++++++++++++ packages/fast-usdc/src/exos/README.md | 26 -------------------------- 2 files changed, 26 insertions(+), 26 deletions(-) delete mode 100644 packages/fast-usdc/src/exos/README.md diff --git a/packages/fast-usdc/README.md b/packages/fast-usdc/README.md index 09810e873cd..fe1794a5872 100644 --- a/packages/fast-usdc/README.md +++ b/packages/fast-usdc/README.md @@ -56,3 +56,29 @@ sequenceDiagram A->>TF: notify(evidence) ``` + +# Status Manager + +### Contract state diagram + +*Transactions are qualified by the OCW and EventFeed before arriving to the Advancer.* + +```mermaid +stateDiagram-v2 + [*] --> Advanced: Advancer .advance() + Advanced --> Settled: Settler .settle() after fees + [*] --> Observed: Advancer .observed() + Observed --> Settled: Settler .settle() sans fees + Settled --> [*] +``` + +### Complete state diagram (starting from OCW) + +```mermaid +stateDiagram-v2 + Observed --> Qualified + Observed --> Unqualified + Qualified --> Advanced + Advanced --> Settled + Qualified --> Settled +``` diff --git a/packages/fast-usdc/src/exos/README.md b/packages/fast-usdc/src/exos/README.md deleted file mode 100644 index f65fb20a958..00000000000 --- a/packages/fast-usdc/src/exos/README.md +++ /dev/null @@ -1,26 +0,0 @@ -## **StatusManager** state diagram, showing different transitions - - -### Contract state diagram - -*Transactions are qualified by the OCW and EventFeed before arriving to the Advancer.* - -```mermaid -stateDiagram-v2 - [*] --> Advanced: Advancer .advance() - Advanced --> Settled: Settler .settle() after fees - [*] --> Observed: Advancer .observed() - Observed --> Settled: Settler .settle() sans fees - Settled --> [*] -``` - -### Complete state diagram (starting from OCW) - -```mermaid -stateDiagram-v2 - Observed --> Qualified - Observed --> Unqualified - Qualified --> Advanced - Advanced --> Settled - Qualified --> Settled -``` From bd8a2dbcd3ecb0fbc8efd40d9bae408521d17347 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Wed, 20 Nov 2024 17:48:57 -0500 Subject: [PATCH 4/6] chore: lint fix warnings --- packages/fast-usdc/src/fast-usdc.contract.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/fast-usdc/src/fast-usdc.contract.js b/packages/fast-usdc/src/fast-usdc.contract.js index 0d838534184..7673e68da87 100644 --- a/packages/fast-usdc/src/fast-usdc.contract.js +++ b/packages/fast-usdc/src/fast-usdc.contract.js @@ -111,7 +111,6 @@ export const contract = async (zcf, privateArgs, zone, tools) => { const creatorFacet = zone.exo('Fast USDC Creator', undefined, { /** @type {(operatorId: string) => Promise>} */ async makeOperatorInvitation(operatorId) { - // eslint-disable-next-line no-use-before-define return feedKit.creator.makeOperatorInvitation(operatorId); }, /** @@ -157,7 +156,6 @@ export const contract = async (zcf, privateArgs, zone, tools) => { * @param {CctpTxEvidence} evidence */ makeTestPushInvitation(evidence) { - // eslint-disable-next-line no-use-before-define void advancer.handleTransactionEvent(evidence); return makeTestInvitation(); }, From 6dc438eabbf8c0e474cd5c1254c31b4a7548ce41 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Wed, 20 Nov 2024 18:03:04 -0500 Subject: [PATCH 5/6] types: status-manager @throws annotations --- packages/fast-usdc/src/exos/status-manager.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/fast-usdc/src/exos/status-manager.js b/packages/fast-usdc/src/exos/status-manager.js index ff27dcdb553..5e774dff3b8 100644 --- a/packages/fast-usdc/src/exos/status-manager.js +++ b/packages/fast-usdc/src/exos/status-manager.js @@ -107,6 +107,7 @@ export const prepareStatusManager = zone => { /** * Add a new transaction with ADVANCED status * @param {CctpTxEvidence} evidence + * @throws {Error} if transaction was already seen */ advance(evidence) { recordPendingTx(evidence, PendingTxStatus.Advanced); @@ -115,6 +116,7 @@ export const prepareStatusManager = zone => { /** * Add a new transaction with OBSERVED status * @param {CctpTxEvidence} evidence + * @throws {Error} if transaction was already seen */ observe(evidence) { recordPendingTx(evidence, PendingTxStatus.Observed); @@ -135,10 +137,15 @@ export const prepareStatusManager = zone => { }, /** - * Mark an `ADVANCED` or `OBSERVED` transaction as `SETTLED` and remove it + * Mark an `ADVANCED` or `OBSERVED` transaction as `SETTLED` and remove + * it. + * + * If there are multiple EDU+amt matches, we are unable to know which tx + * the settlement is for, but we’ll act as if it’s the earliest one. * * @param {NobleAddress} address * @param {bigint} amount + * @throws {Error} if a pending settlement was not found for the address + amount */ settle(address, amount) { const key = makePendingTxKey(address, amount); From 2ba8051b305c8b5d7b7a69d7c4b16c04e4be813b Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Wed, 20 Nov 2024 18:26:54 -0500 Subject: [PATCH 6/6] feat(status-manager): delete empty `pendingTx` entries - if we settle and there's no additional entries for a key, delete it instead of setting to an empty array - ensure always return 'No unsettled entry' error instead of sometimes 'key not found' --- packages/fast-usdc/src/exos/status-manager.js | 20 +++++++++++-------- .../test/exos/status-manager.test.ts | 10 ++++------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/fast-usdc/src/exos/status-manager.js b/packages/fast-usdc/src/exos/status-manager.js index 5e774dff3b8..cd7e8d2082c 100644 --- a/packages/fast-usdc/src/exos/status-manager.js +++ b/packages/fast-usdc/src/exos/status-manager.js @@ -148,30 +148,34 @@ export const prepareStatusManager = zone => { * @throws {Error} if a pending settlement was not found for the address + amount */ settle(address, amount) { - const key = makePendingTxKey(address, amount); - const pending = pendingTxs.get(key); - + // @ts-expect-error this.self not recognized + const pending = this.self.lookupPending(address, amount); if (!pending.length) { - throw makeError(`No unsettled entry for ${q(key)}`); + throw makeError(`No unsettled entry for ${q([address, amount])}`); } const pendingCopy = [...pending]; pendingCopy.shift(); // TODO, vstorage update for `TxStatus.Settled` - pendingTxs.set(key, harden(pendingCopy)); + + const key = makePendingTxKey(address, amount); + if (pendingCopy.length) { + return pendingTxs.set(key, harden(pendingCopy)); + } + return pendingTxs.delete(key); }, /** - * Lookup all pending entries for a given address and amount + * Lookup all pending entries for a given address and amount. * * @param {NobleAddress} address * @param {bigint} amount - * @returns {PendingTx[]} + * @returns {PendingTx[]} pendingTxs or empty array if nothing is found */ lookupPending(address, amount) { const key = makePendingTxKey(address, amount); if (!pendingTxs.has(key)) { - throw makeError(`Key ${q(key)} not yet observed`); + return harden([]); } return pendingTxs.get(key); }, diff --git a/packages/fast-usdc/test/exos/status-manager.test.ts b/packages/fast-usdc/test/exos/status-manager.test.ts index 793b04d88d6..9f0e14bd2db 100644 --- a/packages/fast-usdc/test/exos/status-manager.test.ts +++ b/packages/fast-usdc/test/exos/status-manager.test.ts @@ -87,7 +87,7 @@ test('cannot SETTLE without an ADVANCED or OBSERVED entry', t => { statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount), { message: - 'key "pendingTx:[\\"noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd\\",\\"150000000\\"]" not found in collection "PendingTxs"', + 'No unsettled entry for ["noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd","[150000000n]"]', }, ); }); @@ -139,19 +139,17 @@ test('settle SETTLES first matched entry', t => { statusManager.settle(evidence.tx.forwardingAddress, evidence.tx.amount), { message: - 'No unsettled entry for "pendingTx:[\\"noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd\\",\\"150000000\\"]"', + 'No unsettled entry for ["noble1x0ydg69dh6fqvr27xjvp6maqmrldam6yfelqkd","[150000000n]"]', }, 'No more matches to settle', ); }); -test('lookup throws when presented a key it has not seen', t => { +test('lookingPending returns an empty array when presented a key it has not seen', t => { const zone = provideDurableZone('status-test'); const statusManager = prepareStatusManager(zone.subZone('status-manager')); - t.throws(() => statusManager.lookupPending('noble123', 1n), { - message: 'Key "pendingTx:[\\"noble123\\",\\"1\\"]" not yet observed', - }); + t.deepEqual(statusManager.lookupPending('noble123', 1n), []); }); test('StatusManagerKey logic handles addresses with hyphens', async t => {