Skip to content

Commit

Permalink
fixup Host membrane types (#9797)
Browse files Browse the repository at this point in the history
incidental

## Description
Fix the `HostOf` type of async-flow so `orchestrator.js` doesn't to suppress it.

Makes some refactors and fixes in support of that.

Punts on handling the `subscriber` aspect of `invitationMakers`. Basically, `getUpdatesSince` can't be used in a membrane because it returns a promise, not a vow. This is okay for how `invitationMakers` are supposed to be used, in a smart-wallet which is not in an async-flow. 

### Security Considerations
none

### Scaling Considerations
none

### Documentation Considerations
none

### Testing Considerations
New types tests

### Upgrade Considerations
not yet deployed
  • Loading branch information
mergify[bot] authored Jul 31, 2024
2 parents b1d610d + 37ea93f commit 92f2510
Show file tree
Hide file tree
Showing 15 changed files with 82 additions and 27 deletions.
2 changes: 1 addition & 1 deletion packages/async-flow/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@
"workerThreads": false
},
"typeCoverage": {
"atLeast": 77.01
"atLeast": 77.1
}
}
17 changes: 14 additions & 3 deletions packages/async-flow/src/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,18 @@ export type GuestOf<F extends HostAsyncFuncWrapper> = F extends (
? (...args: A) => Promise<R>
: F;

// from https://github.com/sindresorhus/type-fest/blob/main/source/simplify.d.ts
type Simplify<T> = { [KeyType in keyof T]: T[KeyType] } & {};

/**
* Convert an entire Guest interface into what the host will implement.
*/
type HostInterface<T> = {
[K in keyof T]: HostOf<T[K]>;
[K in keyof T]: T[K] extends CallableFunction
? HostOf<T[K]>
: T[K] extends Record<string, any>
? Simplify<HostInterface<T[K]>>
: T[K];
};

/**
Expand All @@ -71,8 +78,12 @@ export type GuestInterface<T> = {
*
* Specifically, Promise return values are converted to Vows.
*/
export type HostOf<F> = F extends (...args: infer A) => Promise<infer R>
? (...args: A) => Vow<R extends Passable ? R : HostInterface<R>>
export type HostOf<F extends CallableFunction> = F extends (
...args: infer A
) => infer R
? R extends Promise<infer T>
? (...args: A) => Vow<T extends Passable ? T : HostInterface<T>>
: (...args: A) => HostInterface<R>
: F;

export type HostArgs<GA extends any[]> = { [K in keyof GA]: HostOf<GA[K]> };
Expand Down
3 changes: 2 additions & 1 deletion packages/notifier/src/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { makePublishKit } from './publish-kit.js';
import { subscribeLatest } from './subscribe.js';

/**
* @import {Remote} from '@agoric/internal';
* @import {LatestTopic, Notifier, NotifierRecord, PublishKit, Subscriber, UpdateRecord} from './types.js';
*/

Expand Down Expand Up @@ -41,7 +42,7 @@ export const makeNotifier = sharableInternalsP => {

/**
* @template T
* @param {ERef<Subscriber<T>>} subscriber
* @param {ERef<Subscriber<T>> | Remote<Subscriber<T>>} subscriber
* @returns {Notifier<T>}
*/
export const makeNotifierFromSubscriber = subscriber => {
Expand Down
2 changes: 1 addition & 1 deletion packages/orchestration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,6 @@
"access": "public"
},
"typeCoverage": {
"atLeast": 98.05
"atLeast": 98.1
}
}
8 changes: 7 additions & 1 deletion packages/orchestration/src/examples/stakeIca.contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const trace = makeTracer('StakeIca');
* @import {Baggage} from '@agoric/vat-data';
* @import {IBCConnectionID} from '@agoric/vats';
* @import {TimerService} from '@agoric/time';
* @import {ResolvedContinuingOfferResult} from '../utils/zoe-tools.js';
* @import {ICQConnection, CosmosInterchainService} from '../types.js';
*/

Expand Down Expand Up @@ -129,10 +130,15 @@ export const start = async (zcf, privateArgs, baggage) => {
makeAccountInvitationMaker() {
trace('makeCreateAccountInvitation');
return zcf.makeInvitation(
// XXX use `orchestrate` membrane for vow?
/**
* @param {ZCFSeat} seat
* @returns {Promise<ResolvedContinuingOfferResult>}
*/
async seat => {
seat.exit();
const holder = await makeAccountKit();
return holder.asContinuingOffer();
return vowTools.when(holder.asContinuingOffer());
},
'wantStakingAccount',
undefined,
Expand Down
4 changes: 3 additions & 1 deletion packages/orchestration/src/exos/chain-hub.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ import { CosmosChainInfoShape, IBCConnectionInfoShape } from '../typeGuards.js';
*/

/**
* If K matches a known chain, narrow the type from generic ChainInfo
*
* @template {string} K
* @typedef {K extends keyof KnownChains
* ? Omit<KnownChains[K], 'connections'>
* ? ChainInfo & Omit<KnownChains[K], 'connections'>
* : ChainInfo} ActualChainInfo
*/

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ export const prepareCosmosOrchestrationAccountKit = (
holder: {
/** @type {HostOf<OrchestrationAccountI['asContinuingOffer']>} */
asContinuingOffer() {
// @ts-expect-error XXX invitationMakers
// getPublicTopics resolves promptly (same run), so we don't need a watcher
// eslint-disable-next-line no-restricted-syntax
return asVow(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ export const prepareLocalOrchestrationAccountKit = (
holder: {
/** @type {HostOf<OrchestrationAccountI['asContinuingOffer']>} */
asContinuingOffer() {
// @ts-expect-error XXX invitationMakers
// getPublicTopics resolves promptly (same run), so we don't need a watcher
// eslint-disable-next-line no-restricted-syntax
return asVow(async () => {
Expand Down
15 changes: 6 additions & 9 deletions packages/orchestration/src/exos/orchestrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {

/**
* @import {Zone} from '@agoric/base-zone';
* @import {ChainHub} from './chain-hub.js';
* @import {ActualChainInfo, ChainHub} from './chain-hub.js';
* @import {AsyncFlowTools, HostInterface, HostOf} from '@agoric/async-flow';
* @import {Vow, VowTools} from '@agoric/vow';
* @import {TimerService} from '@agoric/time';
Expand Down Expand Up @@ -75,14 +75,14 @@ const prepareOrchestratorKit = (
{
orchestrator: OrchestratorI,
makeLocalChainFacadeWatcher: M.interface('makeLocalChainFacadeWatcher', {
onFulfilled: M.call(M.record(), M.string()).returns(M.any()), // FIXME narrow
onFulfilled: M.call(M.record()).returns(M.remotable()),
}),
makeRemoteChainFacadeWatcher: M.interface(
'makeRemoteChainFacadeWatcher',
{
onFulfilled: M.call(M.any(), M.string())
.optional(M.arrayOf(M.undefined())) // XXX needed?
.returns(M.any()), // FIXME narrow
.returns(M.remotable()),
},
),
},
Expand All @@ -94,12 +94,11 @@ const prepareOrchestratorKit = (
/** Waits for `chainInfo` and returns a LocalChainFacade */
makeLocalChainFacadeWatcher: {
/**
* @param {ChainInfo} agoricChainInfo
* @param {string} name
* @param {ActualChainInfo<'agoric'>} agoricChainInfo
*/
onFulfilled(agoricChainInfo, name) {
onFulfilled(agoricChainInfo) {
const it = makeLocalChainFacade(agoricChainInfo);
chainByName.init(name, it);
chainByName.init('agoric', it);
return it;
},
},
Expand Down Expand Up @@ -131,7 +130,6 @@ const prepareOrchestratorKit = (
return watch(
chainHub.getChainInfo('agoric'),
this.facets.makeLocalChainFacadeWatcher,
name,
);
}
return watch(
Expand All @@ -153,7 +151,6 @@ const prepareOrchestratorKit = (
chainByName.has(baseName) ||
Fail`use getChain(${q(baseName)}) before getBrandInfo(${q(denom)})`;
const base = chainByName.get(baseName);
// @ts-expect-error XXX HostOf<> not quite right?
return harden({ chain, base, brand, baseDenom });
},
/** @type {HostOf<Orchestrator['asAmount']>} */
Expand Down
7 changes: 4 additions & 3 deletions packages/orchestration/src/exos/portfolio-holder-kit.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { VowShape } from '@agoric/vow';
const { fromEntries } = Object;

/**
* @import {HostOf} from '@agoric/async-flow';
* @import {HostInterface, HostOf} from '@agoric/async-flow';
* @import {MapStore} from '@agoric/store';
* @import {VowTools} from '@agoric/vow';
* @import {ResolvedPublicTopic} from '@agoric/zoe/src/contractSupport/topics.js';
Expand All @@ -18,7 +18,7 @@ const { fromEntries } = Object;

/**
* @typedef {{
* accounts: MapStore<string, OrchestrationAccount<any>>;
* accounts: MapStore<string, HostInterface<OrchestrationAccount<any>>>;
* publicTopics: MapStore<string, ResolvedPublicTopic<unknown>>;
* }} PortfolioHolderState
*/
Expand Down Expand Up @@ -99,6 +99,7 @@ const preparePortfolioHolderKit = (zone, { asVow, when }) => {
const { accounts } = this.state;
accounts.has(chainName) || Fail`no account found for ${chainName}`;
const account = accounts.get(chainName);
// @ts-expect-error XXX invitationMakers
return when(E(account).asContinuingOffer(), ({ invitationMakers }) =>
E(invitationMakers)[action](...invitationArgs),
);
Expand All @@ -125,7 +126,7 @@ const preparePortfolioHolderKit = (zone, { asVow, when }) => {
},
/**
* @param {string} chainName key where the account is stored
* @param {OrchestrationAccount<any>} account
* @param {HostInterface<OrchestrationAccount<any>>} account
* @param {ResolvedPublicTopic<unknown>} publicTopic
*/
addAccount(chainName, account, publicTopic) {
Expand Down
10 changes: 6 additions & 4 deletions packages/orchestration/test/exos/portfolio-holder-kit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ test('portfolio holder kit behaviors', async t => {
const cosmosAccount = await E(holder).getAccount('cosmoshub');
t.is(
cosmosAccount,
// @ts-expect-error type mismatch between kit and OrchestrationAccountI
accounts.cosmoshub,
'same account holder kit provided is returned',
);
Expand Down Expand Up @@ -109,12 +108,15 @@ test('portfolio holder kit behaviors', async t => {

const osmosisTopic = (await E(osmosisAccount).getPublicTopics()).account;

// @ts-expect-error type mismatch between kit and OrchestrationAccountI
await E(holder).addAccount('osmosis', osmosisAccount, osmosisTopic);
await E(holder).addAccount(
'osmosis',
osmosisAccount,
// @ts-expect-error the promise from `subscriber.getUpdateSince` can't be used in a flow
osmosisTopic,
);

t.is(
await E(holder).getAccount('osmosis'),
// @ts-expect-error type mismatch between kit and OrchestrationAccountI
osmosisAccount,
'new accounts can be added',
);
Expand Down
1 change: 1 addition & 0 deletions packages/orchestration/test/staking-ops.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ test('makeAccount() writes to storage', async t => {
});
const { publicSubscribers } = await E.when(holder.asContinuingOffer());
const accountNotifier = makeNotifierFromSubscriber(
// @ts-expect-error the promise from `subscriber.getUpdateSince` can't be used in a flow
publicSubscribers.account.subscriber,
);
const storageUpdate = await E(accountNotifier).getUpdateSince();
Expand Down
28 changes: 28 additions & 0 deletions packages/orchestration/test/types.test-d.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
/**
* @file pure types types, no runtime, ignored by Ava
*/

import { expectNotType, expectType } from 'tsd';
import { typedJson } from '@agoric/cosmic-proto';
import type { MsgDelegateResponse } from '@agoric/cosmic-proto/cosmos/staking/v1beta1/tx.js';
import type { QueryAllBalancesResponse } from '@agoric/cosmic-proto/cosmos/bank/v1beta1/query.js';
import type { Vow, VowTools } from '@agoric/vow';
import type { GuestAsyncFunc, HostInterface, HostOf } from '@agoric/async-flow';
import type { ResolvedPublicTopic } from '@agoric/zoe/src/contractSupport/topics.js';
import type {
ChainAddress,
CosmosValidatorAddress,
StakingAccountActions,
OrchestrationAccount,
Orchestrator,
Chain,
ChainInfo,
} from '../src/types.js';
import type { LocalOrchestrationAccountKit } from '../src/exos/local-orchestration-account.js';
import { prepareCosmosOrchestrationAccount } from '../src/exos/cosmos-orchestration-account.js';
Expand Down Expand Up @@ -94,6 +98,30 @@ expectNotType<CosmosValidatorAddress>(chainAddr);

// Negative test
expectNotType<() => Promise<number>>(vowFn);

const getBrandInfo: HostOf<Orchestrator['getBrandInfo']> = null as any;
const chainHostOf = getBrandInfo('uatom').chain;
expectType<Vow<any>>(chainHostOf.getChainInfo());
}

{
// HostInterface

const chain: Chain<ChainInfo> = null as any;
expectType<Promise<ChainInfo>>(chain.getChainInfo());
const chainHostInterface: HostInterface<Chain<ChainInfo>> = null as any;
expectType<Vow<ChainInfo>>(chainHostInterface.getChainInfo());

const publicTopicRecord: HostInterface<
Record<string, ResolvedPublicTopic<unknown>>
> = {
someTopic: {
subscriber: null as any,
storagePath: 'published.somewhere',
},
};
// @ts-expect-error the promise from `subscriber.getUpdateSince` can't be used in a flow
expectType<Record<string, ResolvedPublicTopic<unknown>>>(publicTopicRecord);
}

// HostOf with TransferSteps
Expand Down
2 changes: 1 addition & 1 deletion packages/vats/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,6 @@
"workerThreads": false
},
"typeCoverage": {
"atLeast": 91.41
"atLeast": 91.52
}
}
8 changes: 6 additions & 2 deletions packages/zoe/src/contractSupport/topics.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { SubscriberShape } from '@agoric/notifier';
import { M } from '@agoric/store';
import { E } from '@endo/far';

/**
* @import {Remote} from '@agoric/internal';
*/

export { SubscriberShape };

export const PublicTopicShape = M.splitRecord(
Expand All @@ -22,14 +26,14 @@ export const PublicTopicShape = M.splitRecord(
*/

/**
* A {PublicTopic} in which the `storagePath` is always a resolved string.
* A {PublicTopic} in which the `storagePath` is always a resolved string and the `subscriber is remote.
*
* Useful when working with Vows and async-flow.
*
* @template {object} T topic value
* @typedef {{
* description?: string,
* subscriber: Subscriber<T>,
* subscriber: Remote<Subscriber<T>>,
* storagePath: string,
* }} ResolvedPublicTopic
*/
Expand Down

0 comments on commit 92f2510

Please sign in to comment.