From 1f25ee2ec66788523594e25f020ccf3b6103d259 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Fri, 20 Dec 2024 09:58:56 -0800 Subject: [PATCH] refactor(orchestration,time,zoe): Avoid name convention conflict (#10751) closes: #XXXX refs: https://github.com/endojs/endo/pull/2666 ## Description We generally use names of the form `^([A-Z]\w*)I$`, i.e., identifiers beginning with a capital letter and ending with a lone capital "I", for interface guards. However there were some uses of the same form of name for typescript interfaces. Because interfaces and interface guards are adjacent concepts, this could be confusing. This PR fixes this for agoric-sdk. The companion https://github.com/endojs/endo/pull/2666 fixes it for endo. Because the endo names in question were not exported, there is clearly no dependency between these two PRs. ### Security Considerations Consistent naming conventions makes code more reviewable, which is good for security. ### Scaling Considerations none ### Documentation Considerations Consistent naming conventions helps documentation and its readers. ### Testing Considerations none ### Compatibility Considerations Unlike https://github.com/endojs/endo/pull/2666, the type names in question here are exported by their respective modules (and likely packages), so in theory it is possible to break code outside agoric-sdk that depends on these type names. Because these are only type names that are erased prior to execution, this mismatch could only cause static problems, not runtime problems. Further, the names in question ***seem*** only intended as helper types for defining other types. Likely, and dependence outside agoric-sdk will only be to those outer types, not to these helper types, in which case an actual compat problem is unlikely. But possible. ### Upgrade Considerations Because these are only type names that are erased prior to execution, none. --- .../examples/staking-combinations.flows.js | 2 +- .../src/exos/cosmos-orchestration-account.js | 22 +++++++++---------- .../src/exos/local-orchestration-account.js | 18 +++++++-------- .../src/exos/portfolio-holder-kit.js | 2 +- .../orchestration/src/orchestration-api.ts | 15 +++++++------ .../src/utils/orchestrationAccount.js | 4 ++-- .../test/fixtures/zoe-tools.flows.js | 2 +- packages/orchestration/tools/ibc-mocks.ts | 6 ++--- packages/time/src/types.ts | 6 ++--- packages/zoe/tools/manualTimer.js | 4 ++-- 10 files changed, 41 insertions(+), 40 deletions(-) diff --git a/packages/orchestration/src/examples/staking-combinations.flows.js b/packages/orchestration/src/examples/staking-combinations.flows.js index 794d13099f8..2df03f35102 100644 --- a/packages/orchestration/src/examples/staking-combinations.flows.js +++ b/packages/orchestration/src/examples/staking-combinations.flows.js @@ -1,6 +1,6 @@ /** * @import {GuestInterface} from '@agoric/async-flow'; - * @import {Orchestrator, OrchestrationFlow, AmountArg, CosmosValidatorAddress, ChainAddress, LocalAccountMethods, OrchestrationAccountI, ChainHub} from '../types.js' + * @import {Orchestrator, OrchestrationFlow, AmountArg, CosmosValidatorAddress, ChainAddress, LocalAccountMethods, OrchestrationAccountCommon, ChainHub} from '../types.js' * @import {ContinuingOfferResult, InvitationMakers} from '@agoric/smart-wallet/src/types.js'; * @import {LocalOrchestrationAccountKit} from '../exos/local-orchestration-account.js'; * @import {MakeCombineInvitationMakers} from '../exos/combine-invitation-makers.js'; diff --git a/packages/orchestration/src/exos/cosmos-orchestration-account.js b/packages/orchestration/src/exos/cosmos-orchestration-account.js index a787ff8e39d..1e1ae810283 100644 --- a/packages/orchestration/src/exos/cosmos-orchestration-account.js +++ b/packages/orchestration/src/exos/cosmos-orchestration-account.js @@ -67,7 +67,7 @@ import { makeTimestampHelper } from '../utils/time.js'; /** * @import {HostOf} from '@agoric/async-flow'; - * @import {AmountArg, IcaAccount, ChainAddress, CosmosValidatorAddress, ICQConnection, StakingAccountActions, StakingAccountQueries, OrchestrationAccountI, CosmosRewardsResponse, IBCConnectionInfo, IBCMsgTransferOptions, ChainHub, CosmosDelegationResponse} from '../types.js'; + * @import {AmountArg, IcaAccount, ChainAddress, CosmosValidatorAddress, ICQConnection, StakingAccountActions, StakingAccountQueries, OrchestrationAccountCommon, CosmosRewardsResponse, IBCConnectionInfo, IBCMsgTransferOptions, ChainHub, CosmosDelegationResponse} from '../types.js'; * @import {RecorderKit, MakeRecorderKit} from '@agoric/zoe/src/contractSupport/recorder.js'; * @import {Coin} from '@agoric/cosmic-proto/cosmos/base/v1beta1/coin.js'; * @import {Remote} from '@agoric/internal'; @@ -138,7 +138,7 @@ const stakingAccountQueriesMethods = { getRewards: M.call().returns(VowShape), }; -/** @see {OrchestrationAccountI} */ +/** @see {OrchestrationAccountCommon} */ export const IcaAccountHolderI = M.interface('IcaAccountHolder', { ...orchestrationAccountMethods, ...stakingAccountActionsMethods, @@ -702,7 +702,7 @@ export const prepareCosmosOrchestrationAccountKit = ( }, }, holder: { - /** @type {HostOf} */ + /** @type {HostOf} */ asContinuingOffer() { // @ts-expect-error XXX invitationMakers // getPublicTopics resolves promptly (same run), so we don't need a watcher @@ -724,7 +724,7 @@ export const prepareCosmosOrchestrationAccountKit = ( }); }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ getPublicTopics() { // getStoragePath resolves promptly (same run), so we don't need a watcher // eslint-disable-next-line no-restricted-syntax @@ -741,7 +741,7 @@ export const prepareCosmosOrchestrationAccountKit = ( }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ getAddress() { return this.state.chainAddress; }, @@ -808,7 +808,7 @@ export const prepareCosmosOrchestrationAccountKit = ( return watch(results, this.facets.withdrawRewardWatcher); }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ getBalance(denom) { return asVow(() => { const { chainAddress, icqConnection } = this.state; @@ -827,7 +827,7 @@ export const prepareCosmosOrchestrationAccountKit = ( }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ getBalances() { return asVow(() => { const { chainAddress, icqConnection } = this.state; @@ -845,7 +845,7 @@ export const prepareCosmosOrchestrationAccountKit = ( }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ send(toAccount, amount) { return asVow(() => { trace('send', toAccount, amount); @@ -866,7 +866,7 @@ export const prepareCosmosOrchestrationAccountKit = ( }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ sendAll(toAccount, amounts) { return asVow(() => { trace('sendAll', toAccount, amounts); @@ -887,7 +887,7 @@ export const prepareCosmosOrchestrationAccountKit = ( }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ transfer(destination, amount, opts) { trace('transfer', destination, amount, opts); return asVow(() => { @@ -921,7 +921,7 @@ export const prepareCosmosOrchestrationAccountKit = ( }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ transferSteps(amount, msg) { console.log('transferSteps got', amount, msg); return asVow(() => Fail`not yet implemented`); diff --git a/packages/orchestration/src/exos/local-orchestration-account.js b/packages/orchestration/src/exos/local-orchestration-account.js index 41d151475fd..8b5a04d9a4e 100644 --- a/packages/orchestration/src/exos/local-orchestration-account.js +++ b/packages/orchestration/src/exos/local-orchestration-account.js @@ -28,7 +28,7 @@ import { TransferRouteShape } from './chain-hub.js'; /** * @import {HostOf} from '@agoric/async-flow'; * @import {LocalChain, LocalChainAccount} from '@agoric/vats/src/localchain.js'; - * @import {AmountArg, ChainAddress, DenomAmount, IBCMsgTransferOptions, IBCConnectionInfo, OrchestrationAccountI, LocalAccountMethods, TransferRoute} from '@agoric/orchestration'; + * @import {AmountArg, ChainAddress, DenomAmount, IBCMsgTransferOptions, IBCConnectionInfo, OrchestrationAccountCommon, LocalAccountMethods, TransferRoute} from '@agoric/orchestration'; * @import {RecorderKit, MakeRecorderKit} from '@agoric/zoe/src/contractSupport/recorder.js'. * @import {Zone} from '@agoric/zone'; * @import {Remote} from '@agoric/internal'; @@ -476,7 +476,7 @@ export const prepareLocalOrchestrationAccountKit = ( }, }, holder: { - /** @type {HostOf} */ + /** @type {HostOf} */ asContinuingOffer() { // @ts-expect-error XXX invitationMakers // getPublicTopics resolves promptly (same run), so we don't need a watcher @@ -498,7 +498,7 @@ export const prepareLocalOrchestrationAccountKit = ( }); }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ getBalance(denomArg) { return asVow(() => { const [brand, denom] = @@ -529,7 +529,7 @@ export const prepareLocalOrchestrationAccountKit = ( ); }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ getBalances() { return watch( E(localchain).query( @@ -541,7 +541,7 @@ export const prepareLocalOrchestrationAccountKit = ( ); }, - /** @type {HostOf} */ + /** @type {HostOf} */ getPublicTopics() { // getStoragePath resolves promptly (same run), so we don't need a watcher // eslint-disable-next-line no-restricted-syntax @@ -618,14 +618,14 @@ export const prepareLocalOrchestrationAccountKit = ( executeTx(messages) { return watch(E(this.state.account).executeTx(messages)); }, - /** @type {OrchestrationAccountI['getAddress']} */ + /** @type {OrchestrationAccountCommon['getAddress']} */ getAddress() { return this.state.address; }, /** * XXX consider using ERTP to send if it's vbank asset * - * @type {HostOf} + * @type {HostOf} */ send(toAccount, amount) { return asVow(() => { @@ -646,7 +646,7 @@ export const prepareLocalOrchestrationAccountKit = ( /** * XXX consider using ERTP to send if it's vbank asset * - * @type {HostOf} + * @type {HostOf} */ sendAll(toAccount, amounts) { return asVow(() => { @@ -713,7 +713,7 @@ export const prepareLocalOrchestrationAccountKit = ( return resultV; }); }, - /** @type {HostOf} */ + /** @type {HostOf} */ transferSteps(amount, msg) { return asVow(() => { console.log('transferSteps got', amount, msg); diff --git a/packages/orchestration/src/exos/portfolio-holder-kit.js b/packages/orchestration/src/exos/portfolio-holder-kit.js index a5c03eceacd..0b8f213796d 100644 --- a/packages/orchestration/src/exos/portfolio-holder-kit.js +++ b/packages/orchestration/src/exos/portfolio-holder-kit.js @@ -13,7 +13,7 @@ const { fromEntries } = Object; * @import {VowTools} from '@agoric/vow'; * @import {ResolvedPublicTopic} from '@agoric/zoe/src/contractSupport/topics.js'; * @import {Zone} from '@agoric/zone'; - * @import {OrchestrationAccount, OrchestrationAccountI, MakeCombineInvitationMakers} from '@agoric/orchestration'; + * @import {OrchestrationAccount, OrchestrationAccountCommon, MakeCombineInvitationMakers} from '@agoric/orchestration'; */ /** diff --git a/packages/orchestration/src/orchestration-api.ts b/packages/orchestration/src/orchestration-api.ts index cb97cadcc56..4bd51994d26 100644 --- a/packages/orchestration/src/orchestration-api.ts +++ b/packages/orchestration/src/orchestration-api.ts @@ -69,12 +69,13 @@ export type ChainAddress = { * * The methods available depend on the chain and its capabilities. */ -export type OrchestrationAccount = OrchestrationAccountI & - (CI extends CosmosChainInfo - ? CI['chainId'] extends `agoric${string}` - ? LocalAccountMethods - : CosmosChainAccountMethods - : {}); +export type OrchestrationAccount = + OrchestrationAccountCommon & + (CI extends CosmosChainInfo + ? CI['chainId'] extends `agoric${string}` + ? LocalAccountMethods + : CosmosChainAccountMethods + : {}); /** * An object for access the core functions of a remote chain. @@ -160,7 +161,7 @@ export interface Orchestrator { /** * An object that supports high-level operations for an account on a remote chain. */ -export interface OrchestrationAccountI { +export interface OrchestrationAccountCommon { /** * @returns the address of the account on the remote chain */ diff --git a/packages/orchestration/src/utils/orchestrationAccount.js b/packages/orchestration/src/utils/orchestrationAccount.js index 23f5337d3dc..992510d23f1 100644 --- a/packages/orchestration/src/utils/orchestrationAccount.js +++ b/packages/orchestration/src/utils/orchestrationAccount.js @@ -10,11 +10,11 @@ import { IBCTransferOptionsShape, } from '../typeGuards.js'; -/** @import {OrchestrationAccountI} from '../orchestration-api.js'; */ +/** @import {OrchestrationAccountCommon} from '../orchestration-api.js'; */ const { Vow$ } = NetworkShape; // TODO #9611 -/** @see {OrchestrationAccountI} */ +/** @see {OrchestrationAccountCommon} */ export const orchestrationAccountMethods = { getAddress: M.call().returns(ChainAddressShape), getBalance: M.call(M.or(BrandShape, M.string())).returns( diff --git a/packages/orchestration/test/fixtures/zoe-tools.flows.js b/packages/orchestration/test/fixtures/zoe-tools.flows.js index 4994984bafc..c8e7d829105 100644 --- a/packages/orchestration/test/fixtures/zoe-tools.flows.js +++ b/packages/orchestration/test/fixtures/zoe-tools.flows.js @@ -13,7 +13,7 @@ const { values } = Object; * @import {GuestInterface} from '@agoric/async-flow'; * @import {AtomicProvider} from '@agoric/store/src/stores/store-utils.js'; * @import {LocalOrchestrationAccountKit} from '../../src/exos/local-orchestration-account.js'; - * @import {Orchestrator, LocalAccountMethods, OrchestrationAccountI, OrchestrationFlow, ChainAddress} from '@agoric/orchestration'; + * @import {Orchestrator, LocalAccountMethods, OrchestrationAccountCommon, OrchestrationFlow, ChainAddress} from '@agoric/orchestration'; * @import {ZoeTools} from '../../src/utils/zoe-tools.js'; */ diff --git a/packages/orchestration/tools/ibc-mocks.ts b/packages/orchestration/tools/ibc-mocks.ts index 96995b9e60c..4482edf93c0 100644 --- a/packages/orchestration/tools/ibc-mocks.ts +++ b/packages/orchestration/tools/ibc-mocks.ts @@ -24,7 +24,7 @@ import { atob, btoa, decodeBase64, encodeBase64 } from '@endo/base64'; import type { ChainAddress } from '../src/orchestration-api.js'; import { makeQueryPacket, makeTxPacket } from '../src/utils/packet.js'; -interface EncoderI { +interface EncoderCommon { encode: (message: T) => { finish: () => Uint8Array; }; @@ -45,7 +45,7 @@ const toPacket = (obj: Record): string => * @param message */ export function buildMsgResponseString( - Encoder: EncoderI, + Encoder: EncoderCommon, message: Partial, ): string { const encodedMsg = Encoder.encode(Encoder.fromPartial(message)).finish(); @@ -90,7 +90,7 @@ export function buildMsgErrorString( * @param opts */ export function buildQueryResponseString( - Encoder: EncoderI, + Encoder: EncoderCommon, query: Partial, opts: Omit = { value: new Uint8Array(), diff --git a/packages/time/src/types.ts b/packages/time/src/types.ts index b890f2853ec..06eaf1c3dca 100644 --- a/packages/time/src/types.ts +++ b/packages/time/src/types.ts @@ -105,7 +105,7 @@ export type CancelToken = object; * schedule a single wake() call, create a repeater that will allow scheduling * of events at regular intervals, or remove scheduled calls. */ -export interface TimerServiceI { +export interface TimerServiceCommon { /** * Retrieve the latest timestamp */ @@ -180,9 +180,9 @@ export interface TimerServiceI { getTimerBrand: () => TimerBrand; } // XXX copied from Remotable helper return type -export type TimerService = TimerServiceI & +export type TimerService = TimerServiceCommon & RemotableObject<'TimerService'> & - RemotableBrand<{}, TimerServiceI>; + RemotableBrand<{}, TimerServiceCommon>; /** * Read-only access to a TimeService's current time. This allows reading the diff --git a/packages/zoe/tools/manualTimer.js b/packages/zoe/tools/manualTimer.js index f833e2e2e83..cf05c45b07a 100644 --- a/packages/zoe/tools/manualTimer.js +++ b/packages/zoe/tools/manualTimer.js @@ -5,7 +5,7 @@ import { buildManualTimer } from '@agoric/swingset-vat/tools/manual-timer.js'; import { TimeMath } from '@agoric/time'; /** - * @import {TimerServiceI} from '@agoric/time'; + * @import {TimerServiceCommon} from '@agoric/time'; * @import {RemotableObject} from '@endo/pass-style'; * @import {RemotableBrand} from '@endo/eventual-send'; */ @@ -31,7 +31,7 @@ const nolog = (..._args) => {}; * @property {(nTimes: number, msg?: string) => Promise} tickN */ -/** @typedef {ReturnType & RemotableBrand & ManualTimerAdmin} ZoeManualTimer */ +/** @typedef {ReturnType & RemotableBrand & ManualTimerAdmin} ZoeManualTimer */ /** * A fake TimerService, for unit tests that do not use a real