fix(fast-usdc): recover from bad lp proposal (#10787)

closes https://github.com/Agoric/agoric-private/issues/234

## Description

See issue for context. It also seems related to #10684 because the code path that was triggering the bad state was this:

https://github.com/Agoric/agoric-sdk/blob/ca25dd59f43f27451fad685207086a9be87860c7/packages/fast-usdc/src/exos/liquidity-pool.js#L272-L285

As you can see, we were updating the pool state, but then the `atomicRearrange` failed, so the pool state was left invalid. This PR makes it so that the bad proposal shape is caught by the type guard earlier, so this code path never happens.

The withdraw path was already being handled correctly because the typeguard was specific enough.

If the proposal shape is correct, but the amounts are incorrect, the contract already handles that fine by failing before the state update.

### Security Considerations
The bug would allow anyone to send an offer that breaks the liquidity pool.

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
Added a bootstrap test that fails accordingly without the fix.

### Upgrade Considerations
None

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

@@ -200,47 +200,49 @@ test.serial('writes account addresses to vstorage', async t => {
   await documentStorageSchema(t, storage, doc);
 });
 
-test.serial('makes usdc advance', async t => {
-  const {
-    walletFactoryDriver: wd,
-    storage,
-    agoricNamesRemotes,
-    harness,
-  } = t.context;
-  const oracles = await Promise.all([
-    wd.provideSmartWallet('agoric19uscwxdac6cf6z7d5e26e0jm0lgwstc47cpll8'),
-    wd.provideSmartWallet('agoric1krunjcqfrf7la48zrvdfeeqtls5r00ep68mzkr'),
-    wd.provideSmartWallet('agoric1n4fcxsnkxe4gj6e24naec99hzmc4pjfdccy5nj'),
-  ]);
-  await Promise.all(
-    oracles.map(wallet =>
-      wallet.sendOffer({
-        id: 'claim-oracle-invitation',
-        invitationSpec: {
-          source: 'purse',
-          instance: agoricNamesRemotes.instance.fastUsdc,
-          description: 'oracle operator invitation',
-        },
-        proposal: {},
-      }),
-    ),
+test.serial('LP deposits', async t => {
+  const { walletFactoryDriver: wd, agoricNamesRemotes } = t.context;
+  const lp = await wd.provideSmartWallet(
+    'agoric19uscwxdac6cf6z7d5e26e0jm0lgwstc47cpll8',
   );
 
-  const lp = oracles[0]; // somewhat arbitrary
-
-  // @ts-expect-error it doesnt recognize usdc as a Brand type
+  // @ts-expect-error it doesnt recognize USDC as a Brand type
   const usdc = agoricNamesRemotes.vbankAsset.USDC.brand as Brand<'nat'>;
+  // @ts-expect-error it doesnt recognize FastLP as a Brand type
+  const fastLP = agoricNamesRemotes.vbankAsset.FastLP.brand as Brand<'nat'>;
+
+  // Send a bad proposal first to make sure it's recoverable.
   await lp.sendOffer({
     id: 'deposit-lp-0',
     invitationSpec: {
       source: 'agoricContract',
       instancePath: ['fastUsdc'],
       callPipe: [['makeDepositInvitation', []]],
     },
+    proposal: {
+      give: {
+        USDC: { brand: usdc, value: 98_000_000n },
+      },
+      want: {
+        BADPROPOSAL: { brand: fastLP, value: 567_000_000n },
+      },
+    },
+  });
+
+  await lp.sendOffer({
+    id: 'deposit-lp-1',
+    invitationSpec: {
+      source: 'agoricContract',
+      instancePath: ['fastUsdc'],
+      callPipe: [['makeDepositInvitation', []]],
+    },
     proposal: {
       give: {
         USDC: { brand: usdc, value: 150_000_000n },
       },
+      want: {
+        PoolShare: { brand: fastLP, value: 150_000_000n },
+      },
     },
   });
   await eventLoopIteration();
@@ -252,15 +254,45 @@ test.serial('makes usdc advance', async t => {
       obj.denom === 'ufastlp' &&
       obj.recipient === lp.getAddress(),
   );
-  t.log('LP vbank deposit', lpBankDeposit);
-  t.true(BigInt(lpBankDeposit.amount) > 1_000_000n, 'vbank GIVEs shares to LP');
+  t.log('LP vbank deposits', lpBankDeposit);
+  t.true(
+    BigInt(lpBankDeposit.amount) === 150_000_000n,
+    'vbank GIVEs shares to LP',
+  );
 
   const { purses } = lp.getCurrentWalletRecord();
   // XXX #10491 should not need to resort to string match on brand
   t.falsy(
     purses.find(p => `${p.brand}`.match(/FastLP/)),
     'FastLP balance not in wallet record',
   );
+});
+
+test.serial('makes usdc advance', async t => {
+  const {
+    walletFactoryDriver: wd,
+    storage,
+    agoricNamesRemotes,
+    harness,
+  } = t.context;
+  const oracles = await Promise.all([
+    wd.provideSmartWallet('agoric19uscwxdac6cf6z7d5e26e0jm0lgwstc47cpll8'),
+    wd.provideSmartWallet('agoric1krunjcqfrf7la48zrvdfeeqtls5r00ep68mzkr'),
+    wd.provideSmartWallet('agoric1n4fcxsnkxe4gj6e24naec99hzmc4pjfdccy5nj'),
+  ]);
+  await Promise.all(
+    oracles.map(wallet =>
+      wallet.sendOffer({
+        id: 'claim-oracle-invitation',
+        invitationSpec: {
+          source: 'purse',
+          instance: agoricNamesRemotes.instance.fastUsdc,
+          description: 'oracle operator invitation',
+        },
+        proposal: {},
+      }),
+    ),
+  );
 
   const EUD = 'dydx1anything';
   const lastNodeValue = storage.getValues('published.fastUsdc').at(-1);
@@ -361,6 +393,69 @@ test.serial('skips usdc advance when risks identified', async t => {
   await documentStorageSchema(t, storage, doc);
 });
 
+test.serial('LP withdraws', async t => {
+  const { walletFactoryDriver: wd, agoricNamesRemotes } = t.context;
+  const lp = await wd.provideSmartWallet(
+    'agoric19uscwxdac6cf6z7d5e26e0jm0lgwstc47cpll8',
+  );
+
+  // @ts-expect-error it doesnt recognize USDC as a Brand type
+  const usdc = agoricNamesRemotes.vbankAsset.USDC.brand as Brand<'nat'>;
+  // @ts-expect-error it doesnt recognize FastLP as a Brand type
+  const fastLP = agoricNamesRemotes.vbankAsset.FastLP.brand as Brand<'nat'>;
+
+  // Send a bad proposal first to make sure it's recoverable.
+  await lp.sendOffer({
+    id: 'withdraw-lp-bad-shape',
+    invitationSpec: {
+      source: 'agoricContract',
+      instancePath: ['fastUsdc'],
+      callPipe: [['makeWithdrawInvitation', []]],
+    },
+    proposal: {
+      give: {
+        PoolShare: { brand: fastLP, value: 777_000n },
+      },
+      want: {
+        BADPROPOSALSHAPE: { brand: usdc, value: 777_000n },
+      },
+    },
+  });
+
+  await lp.sendOffer({
+    id: 'withdraw-lp-1',
+    invitationSpec: {
+      source: 'agoricContract',
+      instancePath: ['fastUsdc'],
+      callPipe: [['makeWithdrawInvitation', []]],
+    },
+    proposal: {
+      give: {
+        PoolShare: { brand: fastLP, value: 369_000n },
+      },
+      want: {
+        USDC: { brand: usdc, value: 369_000n },
+      },
+    },
+  });
+  await eventLoopIteration();
+
+  const { denom: usdcDenom } = agoricNamesRemotes.vbankAsset.USDC;
+  const { getOutboundMessages } = t.context.bridgeUtils;
+  const lpBankDeposits = getOutboundMessages(BridgeId.BANK).filter(
+    obj =>
+      obj.type === 'VBANK_GIVE' &&
+      obj.denom === usdcDenom &&
+      obj.recipient === lp.getAddress(),
+  );
+  t.log('LP vbank deposits', lpBankDeposits);
+  // Check index 2. Indexes 0 and 1 would be from the deposit offers in prior testcase.
+  t.true(
+    BigInt(lpBankDeposits[2].amount) >= 369_000n,
+    'vbank GIVEs USDC back to LP',
+  );
+});
+
 test.serial('restart contract', async t => {
   const { EV } = t.context.runUtils;
   await null;

packages/fast-usdc/src/cli/lp-commands.js

@@ -14,6 +14,7 @@ import { AmountMath } from '@agoric/ertp';
 import {
   assertParsableNumber,
   ceilDivideBy,
+  floorDivideBy,
   multiplyBy,
   parseRatio,
 } from '@agoric/zoe/src/contractSupport/ratio.js';
@@ -81,18 +82,32 @@ export const addLPCommands = (
     .action(async opts => {
       swkP ||= loadSwk();
       const swk = await swkP;
+
       /** @type {Brand<'nat'>} */
       // @ts-expect-error it doesnt recognize usdc as a Brand type
       const usdc = swk.agoricNames.brand.USDC;
       assert(usdc, 'USDC brand not in agoricNames');
 
+      /** @type {Brand<'nat'>} */
+      // @ts-expect-error it doesnt recognize FastLP as a Brand type
+      const poolShare = swk.agoricNames.brand.FastLP;
+      assert(poolShare, 'FastLP brand not in agoricNames');
+
       const usdcAmount = parseUSDCAmount(opts.amount, usdc);
 
+      /** @type {import('../types.js').PoolMetrics} */
+      // @ts-expect-error it treats this as "unknown"
+      const metrics = await swk.readPublished('fastUsdc.poolMetrics');
+      const fastLPAmount = floorDivideBy(usdcAmount, metrics.shareWorth);
+
       /** @type {USDCProposalShapes['deposit']} */
       const proposal = {
         give: {
           USDC: usdcAmount,
         },
+        want: {
+          PoolShare: fastLPAmount,
+        },
       };
 
       /** @type {OfferSpec} */
@@ -125,7 +140,6 @@ export const addLPCommands = (
     .requiredOption('--amount <number>', 'USDC amount', parseDecimal)
     .option('--offerId <string>', 'Offer id', String, `lpWithdraw-${now()}`)
     .action(async opts => {
-      swkP ||= loadSwk();
       swkP ||= loadSwk();
       const swk = await swkP;
 

packages/fast-usdc/src/pool-share-math.js

@@ -38,7 +38,7 @@ export const makeParity = (numerator, denominatorBrand) => {
  * @typedef {{
  *   deposit: {
  *     give: { USDC: Amount<'nat'> },
- *     want?: { PoolShare: Amount<'nat'> }
+ *     want: { PoolShare: Amount<'nat'> }
  *   },
  *   withdraw: {
  *     give: { PoolShare: Amount<'nat'> }

packages/fast-usdc/src/type-guards.js

@@ -19,10 +19,10 @@ export const makeNatAmountShape = (brand, min) =>
 /** @param {Record<'PoolShares' | 'USDC', Brand<'nat'>>} brands */
 export const makeProposalShapes = ({ PoolShares, USDC }) => {
   /** @type {TypedPattern<USDCProposalShapes['deposit']>} */
-  const deposit = M.splitRecord(
-    { give: { USDC: makeNatAmountShape(USDC, 1n) } },
-    { want: M.splitRecord({}, { PoolShare: makeNatAmountShape(PoolShares) }) },
-  );
+  const deposit = M.splitRecord({
+    give: { USDC: makeNatAmountShape(USDC, 1n) },
+    want: { PoolShare: makeNatAmountShape(PoolShares) },
+  });
   /** @type {TypedPattern<USDCProposalShapes['withdraw']>} */
   const withdraw = M.splitRecord({
     give: { PoolShare: makeNatAmountShape(PoolShares, 1n) },

packages/fast-usdc/test/cli/lp-commands.test.ts

@@ -58,7 +58,7 @@ const test = anyTest as TestFn<Awaited<ReturnType<typeof makeTestContext>>>;
 test.beforeEach(async t => (t.context = await makeTestContext()));
 
 test('fast-usdc deposit command', async t => {
-  const { program, marshaller, out, err, USDC } = t.context;
+  const { program, marshaller, out, err, USDC, FastLP } = t.context;
   const amount = 100.05;
   const argv = [...`node fast-usdc deposit`.split(' '), ...flags({ amount })];
   t.log(...argv);
@@ -78,6 +78,9 @@ test('fast-usdc deposit command', async t => {
         give: {
           USDC: { brand: USDC, value: 100_050_000n },
         },
+        want: {
+          PoolShare: { brand: FastLP, value: 90_954_545n },
+        },
       },
     },
   });

packages/fast-usdc/test/pool-share-math.test.ts

@@ -58,8 +58,12 @@ test('initial withdrawal fails', t => {
 test('withdrawal after deposit OK', t => {
   const { PoolShares, USDC } = brands;
   const state0 = makeParity(make(USDC, 1n), PoolShares);
+  const emptyShares = makeEmpty(PoolShares);
 
-  const pDep = { give: { USDC: make(USDC, 100n) } };
+  const pDep = {
+    give: { USDC: make(USDC, 100n) },
+    want: { PoolShare: emptyShares },
+  };
   const { shareWorth: state1 } = depositCalc(state0, pDep);
 
   const proposal = harden({
@@ -81,8 +85,12 @@ test('withdrawal after deposit OK', t => {
 
 test('deposit offer underestimates value of share', t => {
   const { PoolShares, USDC } = brands;
+  const emptyShares = makeEmpty(PoolShares);
 
-  const pDep = { give: { USDC: make(USDC, 100n) } };
+  const pDep = {
+    give: { USDC: make(USDC, 100n) },
+    want: { PoolShare: emptyShares },
+  };
   const { shareWorth: state1 } = depositCalc(parity, pDep);
   const state2 = withFees(state1, make(USDC, 20n));
 
@@ -119,9 +127,13 @@ test('deposit offer overestimates value of share', t => {
 
 test('withdrawal offer underestimates value of share', t => {
   const { PoolShares, USDC } = brands;
+  const emptyShares = makeEmpty(PoolShares);
   const state0 = makeParity(make(USDC, 1n), PoolShares);
 
-  const proposal1 = harden({ give: { USDC: make(USDC, 100n) } });
+  const proposal1 = harden({
+    give: { USDC: make(USDC, 100n) },
+    want: { PoolShare: emptyShares },
+  });
   const { shareWorth: state1 } = depositCalc(state0, proposal1);
 
   const proposal = harden({
@@ -143,9 +155,13 @@ test('withdrawal offer underestimates value of share', t => {
 
 test('withdrawal offer overestimates value of share', t => {
   const { PoolShares, USDC } = brands;
+  const emptyShares = makeEmpty(PoolShares);
   const state0 = makeParity(make(USDC, 1n), PoolShares);
 
-  const d100 = { give: { USDC: make(USDC, 100n) } };
+  const d100 = {
+    give: { USDC: make(USDC, 100n) },
+    want: { PoolShare: emptyShares },
+  };
   const { shareWorth: state1 } = depositCalc(state0, d100);
 
   const proposal = harden({
@@ -196,7 +212,12 @@ testProp(
   'deposit properties',
   [arbShareWorth, arbUSDC],
   (t, shareWorth, In) => {
-    const actual = depositCalc(shareWorth, { give: { USDC: In } });
+    const { PoolShares } = brands;
+    const emptyShares = makeEmpty(PoolShares);
+    const actual = depositCalc(shareWorth, {
+      give: { USDC: In },
+      want: { PoolShare: emptyShares },
+    });
     const {
       payouts: { PoolShare },
       shareWorth: post,
@@ -234,7 +255,10 @@ testProp(
 
     for (const { party, action } of actions) {
       if ('In' in action) {
-        const d = depositCalc(shareWorth, { give: { USDC: action.In } });
+        const d = depositCalc(shareWorth, {
+          give: { USDC: action.In },
+          want: { PoolShare: emptyShares },
+        });
         myShares[party] = add(
           myShares[party] || emptyShares,
           d.payouts.PoolShare,