Fix: Remove automatic flow unwinding (#12129)

## Description
For Beta, re: automated recovery by the contract is unnecessary complexity
    
And error in a step would trigger reverse steps for all the successful transactions. This is slow and potentially expensive (since steps may cost time and money). Instead, we should stop, and a future rebalance operation will compute from the new state.  This changes the failure mode to just record the failure and exit.

### Security Considerations
None.

### Scaling Considerations
Better because it does less thrashing if there is for example a temporary outage.

### Documentation Considerations
Users will see  the failure, but will not necessarily know what to do.  We should explain that a future rebalance will complete the deployment.

 - #12131

### Testing Considerations
This skips the old tests that verified recovery execution, since that's not longer appropriate.  It is a separate task to revisit these and see whether they are interesting. 

### Upgrade Considerations
No change to stored state.

Author: mergify[bot]

packages/portfolio-contract/src/portfolio.flows.ts

@@ -119,18 +119,16 @@ type AssetMovement = {
     accounts: AccountsByChain,
     tracer: TraceLogger,
   ) => Promise<{ srcPos?: Position; destPos?: Position }>;
-  recover: (accounts: AccountsByChain, tracer: TraceLogger) => Promise<void>;
 };
 
-const moveStatus = ({ apply: _a, recover: _r, ...data }: AssetMovement) => data;
+const moveStatus = ({ apply: _a, ...data }: AssetMovement) => data;
 const errmsg = (err: any) => ('message' in err ? err.message : `${err}`);
 
 export type TransportDetail<
   How extends string,
   S extends SupportedChain,
   D extends SupportedChain,
   CTX = unknown,
-  RecoverCTX = CTX,
 > = {
   how: How;
   connections: { src: S; dest: D }[];
@@ -140,12 +138,6 @@ export type TransportDetail<
     src: AccountInfoFor[S],
     dest: AccountInfoFor[D],
   ) => Promise<void>;
-  recover: (
-    ctx: RecoverCTX,
-    amount: NatAmount,
-    src: AccountInfoFor[S],
-    dest: AccountInfoFor[D],
-  ) => Promise<void>;
 };
 
 export type ProtocolDetail<
@@ -169,9 +161,9 @@ export type ProtocolDetail<
 };
 
 /**
- * **Failure Handling**: Attempts to unwind failed operations, but recovery
- * itself can fail. In that case, publishes final asset location to vstorage
- * and gives up. Clients must manually rebalance to recover.
+ * **Failure Handling**: Logs failures and publishes status without attempting
+ * to unwind already-completed steps. Operators must resolve any partial
+ * effects manually.
  */
 const trackFlow = async (
   reporter: GuestInterface<PortfolioKit['reporter']>,
@@ -203,35 +195,15 @@ const trackFlow = async (
     reporter.publishFlowStatus(flowId, { state: 'done' });
     // TODO(#NNNN): delete the flow storage node
   } catch (err) {
-    traceFlow('⚠️ step', step, ' failed', err);
+    traceFlow('⚠️ step', step, 'failed', err);
     const failure = moves[step - 1];
-    const errStep = step;
-    while (step > 1) {
-      step -= 1;
-      const traceStep = traceFlow.sub(`step${step}`);
-      const move = moves[step - 1];
-      const how = `unwind: ${move.how}`;
-      reporter.publishFlowStatus(flowId, { state: 'undo', step, how });
-      try {
-        await move.recover(accounts, traceStep);
-      } catch (errInUnwind) {
-        traceStep('⚠️ unwind failed', errInUnwind);
-        // if a recover fails, we just give up and report `where` the assets are
-        const { dest: where } = move;
-        reporter.publishFlowStatus(flowId, {
-          state: 'fail',
-          step,
-          how,
-          error: errmsg(errInUnwind),
-          where,
-        });
-        throw errInUnwind;
-      }
+    if (failure) {
+      traceFlow('failed movement details', moveStatus(failure));
     }
     reporter.publishFlowStatus(flowId, {
       state: 'fail',
-      step: errStep,
-      how: failure.how,
+      step,
+      how: failure?.how ?? 'unknown',
       error: errmsg(err),
     });
     throw err;
@@ -517,24 +489,6 @@ const stepFlow = async (
           return { srcPos: pos };
         }
       },
-      recover: async ({ [evmChain]: gInfo }) => {
-        assert(gInfo, evmChain);
-        await null;
-        if ('src' in way) {
-          assert.fail('last step. cannot recover');
-        } else {
-          const { lca } = agoric;
-          const { poolKey } = way;
-          const evmCtx = await makeEVMPoolCtx(
-            evmChain,
-            move,
-            lca,
-            poolKey,
-            ctx.transferChannels.noble.counterPartyChannelId,
-          );
-          await pImpl.supply(evmCtx, amount, gInfo);
-        }
-      },
     });
   };
 
@@ -544,8 +498,8 @@ const stepFlow = async (
   traceFlow('checking', moves.length, 'moves');
   for (const [i, move] of entries(moves)) {
     const traceMove = traceFlow.sub(`move${i}`);
-    // @@@ traceMove('wayFromSrcToDesc?', move);
     const way = wayFromSrcToDesc(move);
+    traceMove('plan', { move, way });
     const { amount } = move;
     switch (way.how) {
       case 'localTransfer': {
@@ -562,11 +516,6 @@ const stepFlow = async (
             await ctx.zoeTools.localTransfer(src.seat, account, amounts);
             return {};
           },
-          recover: async ({ agoric }) => {
-            const { lca, lcaIn } = agoric;
-            const account = move.dest === '+agoric' ? lcaIn : lca;
-            await ctx.zoeTools.withdrawToSeat(account, seat, amounts);
-          },
         });
         break;
       }
@@ -582,9 +531,6 @@ const stepFlow = async (
             await ctx.zoeTools.withdrawToSeat(agoric.lca, seat, amounts);
             return {};
           },
-          recover: async ({ agoric }) => {
-            await ctx.zoeTools.localTransfer(seat, agoric.lca, amounts);
-          },
         });
         break;
       }
@@ -600,9 +546,6 @@ const stepFlow = async (
             await lcaIn.send(lca.getAddress(), amount);
             return {};
           },
-          recover: async () => {
-            traceMove('recover send is noop; not sending back to deposit LCA');
-          },
         });
         break;
 
@@ -629,15 +572,6 @@ const stepFlow = async (
             }
             return {};
           },
-          recover: async ({ agoric, noble }) => {
-            assert(noble); // per nobleMentioned below
-            await null;
-            if (way.src === 'agoric') {
-              await agoricToNoble.recover(ctxI, amount, agoric, noble);
-            } else {
-              await nobleToAgoric.recover(ctxI, amount, noble, agoric);
-            }
-          },
         });
 
         break;
@@ -671,15 +605,6 @@ const stepFlow = async (
             }
             return {};
           },
-          recover: async ({ [evmChain]: gInfo, agoric, noble }) => {
-            assert(gInfo && noble, evmChain);
-            await null;
-            if (outbound) {
-              await CCTP.recover(ctx, amount, noble, gInfo);
-            } else {
-              await CCTPfromEVM.recover(ctx, amount, gInfo, agoric);
-            }
-          },
         });
 
         break;
@@ -709,15 +634,6 @@ const stepFlow = async (
               return { srcPos: pos };
             }
           },
-          recover: async ({ noble }) => {
-            assert(noble); // per nobleMentioned below
-            await null;
-            if (isSupply) {
-              Fail`no recovery from supply (final step)`;
-            } else {
-              await protocolUSDN.supply(ctxU, amount, noble);
-            }
-          },
         });
 
         break;
@@ -759,7 +675,7 @@ const stepFlow = async (
   });
   reporter.publishFlowSteps(
     flowId,
-    todo.map(({ apply: _a, recover: _r, ...data }) => data),
+    todo.map(({ apply: _a, ...data }) => data),
   );
 
   const agoric = await provideCosmosAccount(orch, 'agoric', kit, traceFlow);
@@ -840,11 +756,12 @@ const stepFlow = async (
  * More generally: move assets as instructed by client.
  *
  * **Non-Atomic Operations**: Cross-chain flows are not atomic. If operations
- * fail partway through, assets may be left in intermediate accounts.
- * Recovery is attempted but can also fail, leaving assets "stranded".
+ * fail partway through, assets may be left in intermediate accounts and must
+ * be reconciled manually.
  *
  * **Client Recovery**: If rebalancing fails, check flow status in vstorage
- * and call rebalance() again to move assets to desired destinations.
+ * and call rebalance() again (or another corrective flow) to move assets to
+ * desired destinations.
  *
  * **Input Validation**: ASSUME caller validates args
  *

packages/portfolio-contract/src/pos-gmp.flows.ts

@@ -168,16 +168,7 @@ export const CCTPfromEVM = {
     await Promise.all([result, contractCallP]);
     traceTransfer('transfer complete.');
   },
-  recover: async (_ctx, _amount, _src, _dest) => {
-    throw Error('TODO: do away with recover. see #NNNN');
-  },
-} as const satisfies TransportDetail<
-  'CCTP',
-  AxelarChain,
-  'agoric',
-  EVMContext,
-  PortfolioInstanceContext
->;
+} as const satisfies TransportDetail<'CCTP', AxelarChain, 'agoric', EVMContext>;
 harden(CCTPfromEVM);
 
 export const CCTP = {
@@ -205,11 +196,6 @@ export const CCTP = {
     ]);
     traceTransfer('transfer complete.');
   },
-  recover: async (_ctx, _amount, _src, _dest) => {
-    // XXX evmCtx needs a GMP fee
-    // return CCTPfromEVM.apply(evmCtx, amount, dest, src);
-    throw Error('TODO(Luqi): how to recover from CCTP transfer?');
-  },
 } as const satisfies TransportDetail<'CCTP', 'noble', AxelarChain>;
 harden(CCTP);
 

packages/portfolio-contract/src/pos-usdn.flows.ts

@@ -149,10 +149,6 @@ export const agoricToNoble = {
     const denomAmount = { value: amount.value, denom };
     await src.lca.transfer(dest.ica.getAddress(), denomAmount);
   },
-  recover: async (ctx, amount, src, dest) => {
-    const nobleAmount = { value: amount.value, denom: 'uusdc' };
-    await dest.ica.transfer(src.lca.getAddress(), nobleAmount);
-  },
 } as const satisfies TransportDetail<
   'IBC to Noble',
   'agoric',
@@ -168,11 +164,6 @@ export const nobleToAgoric = {
     const nobleAmount = { value: amount.value, denom: 'uusdc' };
     await src.ica.transfer(dest.lca.getAddress(), nobleAmount);
   },
-  recover: async (ctx, amount, src, dest) => {
-    const { denom } = ctx.usdc;
-    const denomAmount = { value: amount.value, denom };
-    await dest.lca.transfer(src.ica.getAddress(), denomAmount);
-  },
 } as const satisfies TransportDetail<
   'IBC from Noble',
   'noble',

packages/portfolio-contract/src/type-guards.ts

@@ -333,7 +333,7 @@ export const makeFlowStepsPath = (parent: number, id: number) => [
 
 export const FlowStatusShape: TypedPattern<StatusFor['flow']> = M.or(
   { state: 'run', step: M.number(), how: M.string() },
-  { state: 'undo', step: M.number(), how: M.string() },
+  { state: 'undo', step: M.number(), how: M.string() }, // XXX Not currently used
   { state: 'done' },
   M.splitRecord(
     { state: 'fail', step: M.number(), how: M.string(), error: M.string() },

packages/portfolio-contract/test/offer-shapes.test.ts

@@ -156,11 +156,6 @@ test('vstorage flow type matches shape', t => {
       step: 1,
       how: 'deposit',
     },
-    undoingFlow: {
-      state: 'undo',
-      step: 2,
-      how: 'withdraw',
-    },
     completedFlow: {
       state: 'done',
     },

packages/portfolio-contract/test/portfolio.flows.test.ts

@@ -704,7 +704,7 @@ test('open portfolio with Compound position', async t => {
   await documentStorageSchema(t, storage, docOpts);
 });
 
-test('handle failure in localTransfer from seat to local account', async t => {
+test.skip('handle failure in localTransfer from seat to local account', async t => {
   const amount = make(USDC, 100n);
   const { orch, ctx, offer, storage } = mocks(
     { localTransfer: Error('localTransfer from seat failed') },
@@ -754,7 +754,7 @@ test.skip('handle failure in IBC transfer', async t => {
   await documentStorageSchema(t, storage, docOpts);
 });
 
-test('handle failure in executeEncodedTx', async t => {
+test.skip('handle failure in executeEncodedTx', async t => {
   const { give, steps } = await makePortfolioSteps({ USDN: make(USDC, 100n) });
   const { orch, ctx, offer, storage } = mocks(
     { executeEncodedTx: Error('exec swaplock went kerflewey') },
@@ -781,7 +781,7 @@ test('handle failure in executeEncodedTx', async t => {
   await documentStorageSchema(t, storage, docOpts);
 });
 
-test('handle failure in recovery from executeEncodedTx', async t => {
+test.skip('handle failure in recovery from executeEncodedTx', async t => {
   const amount = make(USDC, 100n);
   const { orch, ctx, offer, storage } = mocks(
     {
@@ -868,7 +868,7 @@ test(
   ],
 );
 
-test('rebalance handles stepFlow failure correctly', async t => {
+test.skip('rebalance handles stepFlow failure correctly', async t => {
   const { orch, ctx, offer } = mocks(
     {
       // Mock a failure in IBC transfer