Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/portfolio-contract/src/planner.exo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ export const preparePlanner = (
resolvePlan: M.call(M.number(), M.number(), planCompatShape, M.number())
.optional(M.number())
.returns(),
rejectPlan: M.call(M.number(), M.number(), M.string())
.optional(M.number(), M.number())
.returns(),
});

return zone.exoClass(
Expand Down Expand Up @@ -109,6 +112,20 @@ export const preparePlanner = (
portfolioPlanner.submitVersion(policyVersion, rebalanceCount);
portfolioPlanner.resolveFlowPlan(flowId, planOrSteps);
},
rejectPlan(
portfolioId: number,
flowId: number,
Comment on lines +116 to +117
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somewhat tangential but is the nomenclature that flow/portfolio "ID" is a number and the string form (e.g. portfolio3) is a key?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I've done that consistently.

I sort of think of the id being a number, and sometimes it's passed around as a js number and sometimes it's wrapped in a portfolioN form.

reason: string,
policyVersion?: number,
rebalanceCount?: number,
) {
trace('TODO: reject plan', { portfolioId, flowId, reason });
const { planner: portfolioPlanner } = getPortfolio(portfolioId);
if (policyVersion !== undefined && rebalanceCount !== undefined) {
portfolioPlanner.submitVersion(policyVersion, rebalanceCount);
}
portfolioPlanner.rejectFlowPlan(flowId, reason);
},
},
{
stateShape: { etc: M.any() },
Expand Down
12 changes: 12 additions & 0 deletions packages/portfolio-contract/src/portfolio.exo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,18 @@ export const preparePortfolioKit = (
const detail = flowsRunning.get(flowId);
detail.sync.resolver.resolve(steps);
},
rejectFlowPlan(flowId: number, reason: string) {
const { flowsRunning } = this.state;
if (!flowsRunning.has(flowId)) {
const traceFlow = trace
.sub(`portfolio${this.state.portfolioId}`)
.sub(`flow${flowId}`);
traceFlow('flowsRunning has nothing to reject');
return;
}
const detail = flowsRunning.get(flowId);
detail.sync.resolver.reject(new Error(reason));
},
},
manager: {
reserveAccount<C extends SupportedChain>(
Expand Down
19 changes: 15 additions & 4 deletions packages/portfolio-contract/src/portfolio.flows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1167,13 +1167,24 @@ export const executePlan = (async (
startedFlow ?? pKit.manager.startFlow(flowDetail, offerArgs.flow);
const traceFlow = traceP.sub(`flow${flowId}`);
if (!offerArgs.flow) traceFlow('waiting for steps from planner');
// idea: race with seat.getSubscriber()
const plan = await (stepsP as unknown as Promise<
MovementDesc[] | FundsFlowPlan
>); // XXX Guest/Host types UNTIL #9822
await null;
try {
// idea: race with seat.getSubscriber()
const plan = await (stepsP as unknown as Promise<
MovementDesc[] | FundsFlowPlan
>); // XXX Guest/Host types UNTIL #9822
await stepFlow(orch, ctx, seat, plan, pKit, traceP, flowId, flowDetail);
return `flow${flowId}`;
} catch (err) {
if (!seat.hasExited()) seat.fail(err);
pKit.reporter.publishFlowStatus(flowId, {
state: 'fail',
step: 0,
error: errmsg(err),
how: `await plan`,
...flowDetail,
});
throw err;
} finally {
// The seat must be exited no matter what to avoid leaks
if (!seat.hasExited()) seat.exit();
Expand Down
117 changes: 102 additions & 15 deletions packages/portfolio-contract/test/planner.exo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';

import { makeIssuerKit } from '@agoric/ertp';
import { makeFakeStorageKit } from '@agoric/internal/src/storage-test-utils.js';
import { eventLoopIteration } from '@agoric/internal/src/testing-utils.js';
import { makeFakeBoard } from '@agoric/vats/tools/board-utils.js';
import { prepareVowTools } from '@agoric/vow';
import type { ZCF } from '@agoric/zoe';
Expand All @@ -13,7 +12,7 @@ import {
makeOfferArgsShapes,
type MovementDesc,
} from '../src/type-guards-steps.ts';
import type { StatusFor } from '../src/type-guards.ts';
import { makeStorageTools } from './supports.ts';

const { brand: USDC } = makeIssuerKit('USDC');

Expand All @@ -35,17 +34,14 @@ test('planner exo submit method', async t => {

const board = makeFakeBoard();
const storage = makeFakeStorageKit('published', { sequence: true });
const readPublished = async path => {
await eventLoopIteration();
return marshaller.fromCapData(
JSON.parse(storage.getValues(`published.${path}`).at(-1) || ''),
) as StatusFor['portfolio'];
};
const { getPortfolioStatus } = makeStorageTools(storage);
const marshaller = board.getReadonlyMarshaller();
const makePortfolio = preparePortfolioKit(zone, {
usdcBrand: USDC,
marshaller,
portfoliosNode: storage.rootNode.makeChildNode('portfolios'),
portfoliosNode: storage.rootNode
.makeChildNode('ymax0')
.makeChildNode('portfolios'),
vowTools: vt,
...({} as any),
});
Expand All @@ -67,9 +63,7 @@ test('planner exo submit method', async t => {
aPortfolio.manager.setTargetAllocation({ USDN: 100n });

{
const { policyVersion, rebalanceCount } = await readPublished(
'portfolios.portfolio1',
);
const { policyVersion, rebalanceCount } = await getPortfolioStatus(1);
t.log('targetAllocation', aPortfolio.reader.getTargetAllocation(), {
policyVersion,
rebalanceCount,
Expand All @@ -96,9 +90,7 @@ test('planner exo submit method', async t => {
await vt.when(planner.submit(portfolioId, plan, 1, 0));

{
const { policyVersion, rebalanceCount } = await readPublished(
'portfolios.portfolio1',
);
const { policyVersion, rebalanceCount } = await getPortfolioStatus(1);
t.log({ policyVersion, rebalanceCount });
t.deepEqual(
{ policyVersion, rebalanceCount },
Expand All @@ -118,3 +110,98 @@ test('planner exo submit method', async t => {
vt.when(planner.resolvePlan(portfolioId, 1, plan, 1, 2)),
);
});

test('planner can reject a plan due to insufficient funds', async t => {
const zone = makeHeapZone();

const vt = prepareVowTools(zone);
// Mock dependencies with minimal implementation
const mockRebalance = (_seat, offerArgs, _kit) => {
t.log('rebalance called with', offerArgs);
return vt.asVow(() => undefined);
};

const mockZcf = {
makeEmptySeatKit: () => ({
zcfSeat: null as any,
}),
} as ZCF;

const board = makeFakeBoard();
const storage = makeFakeStorageKit('published', { sequence: true });
const { getPortfolioStatus } = makeStorageTools(storage);
const marshaller = board.getReadonlyMarshaller();
const makePortfolio = preparePortfolioKit(zone, {
usdcBrand: USDC,
marshaller,
portfoliosNode: storage.rootNode
.makeChildNode('ymax0')
.makeChildNode('portfolios'),
vowTools: vt,
...({} as any),
});
const aPortfolio = makePortfolio({ portfolioId: 1 });
const mockGetPortfolio = _id => aPortfolio;

// Create planner exo
const makePlanner = preparePlanner(zone, {
rebalance: mockRebalance,
zcf: mockZcf,
getPortfolio: mockGetPortfolio,
shapes: makeOfferArgsShapes(USDC),
vowTools: vt,
});

const planner = makePlanner();

// Set up portfolio state
aPortfolio.manager.setTargetAllocation({ USDN: 100n });

const portfolioId = 0;
const amount = { brand: USDC, value: 100n };

// Start a flow that will be waiting for a plan and simulate proper cleanup
const { stepsP, flowId } = aPortfolio.manager.startFlow({
type: 'withdraw',
amount,
});

{
const {
policyVersion,
rebalanceCount,
flowsRunning = {},
} = await getPortfolioStatus(1);
t.log('before reject:', { policyVersion, rebalanceCount, flowsRunning });
t.is(Object.keys(flowsRunning).length, 1, 'should have one running flow');
t.is(rebalanceCount, 0, 'rebalanceCount should start at 0');
}

// Planner rejects the plan due to insufficient funds
t.notThrows(() => planner.rejectPlan(portfolioId, 1, 'insufficient funds'));

// Verify the flow's promise gets rejected with the expected error
await t.throwsAsync(() => vt.when(stepsP), { message: 'insufficient funds' });

// Simulate proper cleanup that would happen in production flows
aPortfolio.reporter.finishFlow(flowId);

{
const {
policyVersion,
rebalanceCount,
flowsRunning = {},
} = await getPortfolioStatus(1);
t.log('after reject and cleanup:', {
policyVersion,
rebalanceCount,
flowsRunning,
});
t.is(
Object.keys(flowsRunning).length,
0,
'flow should be cleaned up after finishFlow',
);
t.is(rebalanceCount, 0, 'rebalanceCount should remain 0 after rejection');
}
});
69 changes: 69 additions & 0 deletions packages/portfolio-contract/test/portfolio.flows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1632,3 +1632,72 @@ test('parallel execution with scheduler', async t => {

t.snapshot(flowHistory, 'parallel flow history');
});

test('planner rejects plan and flow fails gracefully', async t => {
const { orch, ctx, offer, storage } = mocks({});

const { getPortfolioStatus } = makeStorageTools(storage);

const kit = await ctx.makePortfolioKit();
const portfolioId = kit.reader.getPortfolioId();

// Set up portfolio with initial allocation
kit.manager.setTargetAllocation({ USDN: 10000n }); // 100% USDN

const webUiDone = (async () => {
const Cash = make(USDC, 1_000_000n);
const dSeat = makeMockSeat({}, { Cash }, offer.log);

// This should fail when planner rejects the plan
await t.throwsAsync(
() =>
executePlan(orch, ctx, dSeat, {}, kit, {
type: 'withdraw',
amount: Cash,
}),
{ message: 'insufficient funds for this operation' },
);
})();

const plannerP = (async () => {
const { flowsRunning = {} } = await getPortfolioStatus(portfolioId);
const [[flowId, detail]] = Object.entries(flowsRunning);
t.log('planner found running flow', { portfolioId, flowId, detail });

if (detail.type !== 'withdraw')
throw t.fail(`Expected withdraw, got ${detail.type}`);

// Planner rejects the plan due to insufficient funds
const flowIdNum = Number(flowId.replace('flow', ''));

kit.planner.rejectFlowPlan(
flowIdNum,
'insufficient funds for this operation',
);
})();

await Promise.all([webUiDone, plannerP]);

const { log } = offer;
t.log('calls:', log.map(msg => msg._method).join(', '));

// Verify the seat failed rather than exited successfully
const seatCalls = log.filter(entry => entry._cap === 'seat');
const failCall = seatCalls.find(call => call._method === 'fail');
const exitCall = seatCalls.find(call => call._method === 'exit');

t.truthy(failCall, 'seat.fail() should be called when plan is rejected');
t.falsy(exitCall, 'seat.exit() should not be called when plan is rejected');
t.deepEqual(
`${failCall?.reason}`,
'Error: insufficient funds for this operation',
'failure reason should match rejection message',
);

// Verify flow is cleaned up from running flows
const { flowsRunning = {} } = await getPortfolioStatus(portfolioId);
t.deepEqual(flowsRunning, {}, 'flow should be cleaned up after rejection');

t.snapshot(log, 'call log');
await documentStorageSchema(t, storage, docOpts);
});
Original file line number Diff line number Diff line change
Expand Up @@ -3705,3 +3705,60 @@ Generated by [AVA](https://avajs.dev).
},
],
}

## planner rejects plan and flow fails gracefully

> call log

[
{
_cap: 'seat',
_method: 'fail',
reason: Error {
message: 'insufficient funds for this operation',
},
},
]

> Under "published", the "ymax0" node is delegated to ymax.
> The example below illustrates the schema of the data published there.
>
> See also board marshalling conventions (_to appear_).

[
[
'published.ymax0.portfolios',
{
addPortfolio: 'portfolio1',
},
],
[
'published.ymax0.portfolios.portfolio1',
{
accountIdByChain: {},
accountsPending: [],
flowCount: 1,
flowsRunning: {},
policyVersion: 1,
positionKeys: [],
rebalanceCount: 0,
targetAllocation: {
USDN: 10000n,
},
},
],
[
'published.ymax0.portfolios.portfolio1.flows.flow1',
{
amount: {
brand: Object @Alleged: USDC brand {},
value: 1000000n,
},
error: 'insufficient funds for this operation',
how: 'await plan',
state: 'fail',
step: 0,
type: 'withdraw',
},
],
]
Binary file not shown.
Loading
Loading