Skip to content

Commit

Permalink
refactor(orchestration,time,zoe): Avoid name convention conflict (#10751
Browse files Browse the repository at this point in the history
)

closes: #XXXX
refs: endojs/endo#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 endojs/endo#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 endojs/endo#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.
  • Loading branch information
erights authored Dec 20, 2024
1 parent 7b7ceb9 commit 1f25ee2
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 40 deletions.
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
22 changes: 11 additions & 11 deletions packages/orchestration/src/exos/cosmos-orchestration-account.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -138,7 +138,7 @@ const stakingAccountQueriesMethods = {
getRewards: M.call().returns(VowShape),
};

/** @see {OrchestrationAccountI} */
/** @see {OrchestrationAccountCommon} */
export const IcaAccountHolderI = M.interface('IcaAccountHolder', {
...orchestrationAccountMethods,
...stakingAccountActionsMethods,
Expand Down Expand Up @@ -702,7 +702,7 @@ export const prepareCosmosOrchestrationAccountKit = (
},
},
holder: {
/** @type {HostOf<OrchestrationAccountI['asContinuingOffer']>} */
/** @type {HostOf<OrchestrationAccountCommon['asContinuingOffer']>} */
asContinuingOffer() {
// @ts-expect-error XXX invitationMakers
// getPublicTopics resolves promptly (same run), so we don't need a watcher
Expand All @@ -724,7 +724,7 @@ export const prepareCosmosOrchestrationAccountKit = (
});
});
},
/** @type {HostOf<OrchestrationAccountI['getPublicTopics']>} */
/** @type {HostOf<OrchestrationAccountCommon['getPublicTopics']>} */
getPublicTopics() {
// getStoragePath resolves promptly (same run), so we don't need a watcher
// eslint-disable-next-line no-restricted-syntax
Expand All @@ -741,7 +741,7 @@ export const prepareCosmosOrchestrationAccountKit = (
});
},

/** @type {HostOf<OrchestrationAccountI['getAddress']>} */
/** @type {HostOf<OrchestrationAccountCommon['getAddress']>} */
getAddress() {
return this.state.chainAddress;
},
Expand Down Expand Up @@ -808,7 +808,7 @@ export const prepareCosmosOrchestrationAccountKit = (
return watch(results, this.facets.withdrawRewardWatcher);
});
},
/** @type {HostOf<OrchestrationAccountI['getBalance']>} */
/** @type {HostOf<OrchestrationAccountCommon['getBalance']>} */
getBalance(denom) {
return asVow(() => {
const { chainAddress, icqConnection } = this.state;
Expand All @@ -827,7 +827,7 @@ export const prepareCosmosOrchestrationAccountKit = (
});
},

/** @type {HostOf<OrchestrationAccountI['getBalances']>} */
/** @type {HostOf<OrchestrationAccountCommon['getBalances']>} */
getBalances() {
return asVow(() => {
const { chainAddress, icqConnection } = this.state;
Expand All @@ -845,7 +845,7 @@ export const prepareCosmosOrchestrationAccountKit = (
});
},

/** @type {HostOf<OrchestrationAccountI['send']>} */
/** @type {HostOf<OrchestrationAccountCommon['send']>} */
send(toAccount, amount) {
return asVow(() => {
trace('send', toAccount, amount);
Expand All @@ -866,7 +866,7 @@ export const prepareCosmosOrchestrationAccountKit = (
});
},

/** @type {HostOf<OrchestrationAccountI['sendAll']>} */
/** @type {HostOf<OrchestrationAccountCommon['sendAll']>} */
sendAll(toAccount, amounts) {
return asVow(() => {
trace('sendAll', toAccount, amounts);
Expand All @@ -887,7 +887,7 @@ export const prepareCosmosOrchestrationAccountKit = (
});
},

/** @type {HostOf<OrchestrationAccountI['transfer']>} */
/** @type {HostOf<OrchestrationAccountCommon['transfer']>} */
transfer(destination, amount, opts) {
trace('transfer', destination, amount, opts);
return asVow(() => {
Expand Down Expand Up @@ -921,7 +921,7 @@ export const prepareCosmosOrchestrationAccountKit = (
});
},

/** @type {HostOf<OrchestrationAccountI['transferSteps']>} */
/** @type {HostOf<OrchestrationAccountCommon['transferSteps']>} */
transferSteps(amount, msg) {
console.log('transferSteps got', amount, msg);
return asVow(() => Fail`not yet implemented`);
Expand Down
18 changes: 9 additions & 9 deletions packages/orchestration/src/exos/local-orchestration-account.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -476,7 +476,7 @@ export const prepareLocalOrchestrationAccountKit = (
},
},
holder: {
/** @type {HostOf<OrchestrationAccountI['asContinuingOffer']>} */
/** @type {HostOf<OrchestrationAccountCommon['asContinuingOffer']>} */
asContinuingOffer() {
// @ts-expect-error XXX invitationMakers
// getPublicTopics resolves promptly (same run), so we don't need a watcher
Expand All @@ -498,7 +498,7 @@ export const prepareLocalOrchestrationAccountKit = (
});
});
},
/** @type {HostOf<OrchestrationAccountI['getBalance']>} */
/** @type {HostOf<OrchestrationAccountCommon['getBalance']>} */
getBalance(denomArg) {
return asVow(() => {
const [brand, denom] =
Expand Down Expand Up @@ -529,7 +529,7 @@ export const prepareLocalOrchestrationAccountKit = (
);
});
},
/** @type {HostOf<OrchestrationAccountI['getBalances']>} */
/** @type {HostOf<OrchestrationAccountCommon['getBalances']>} */
getBalances() {
return watch(
E(localchain).query(
Expand All @@ -541,7 +541,7 @@ export const prepareLocalOrchestrationAccountKit = (
);
},

/** @type {HostOf<OrchestrationAccountI['getPublicTopics']>} */
/** @type {HostOf<OrchestrationAccountCommon['getPublicTopics']>} */
getPublicTopics() {
// getStoragePath resolves promptly (same run), so we don't need a watcher
// eslint-disable-next-line no-restricted-syntax
Expand Down Expand Up @@ -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<OrchestrationAccountI['send']>}
* @type {HostOf<OrchestrationAccountCommon['send']>}
*/
send(toAccount, amount) {
return asVow(() => {
Expand All @@ -646,7 +646,7 @@ export const prepareLocalOrchestrationAccountKit = (
/**
* XXX consider using ERTP to send if it's vbank asset
*
* @type {HostOf<OrchestrationAccountI['sendAll']>}
* @type {HostOf<OrchestrationAccountCommon['sendAll']>}
*/
sendAll(toAccount, amounts) {
return asVow(() => {
Expand Down Expand Up @@ -713,7 +713,7 @@ export const prepareLocalOrchestrationAccountKit = (
return resultV;
});
},
/** @type {HostOf<OrchestrationAccountI['transferSteps']>} */
/** @type {HostOf<OrchestrationAccountCommon['transferSteps']>} */
transferSteps(amount, msg) {
return asVow(() => {
console.log('transferSteps got', amount, msg);
Expand Down
2 changes: 1 addition & 1 deletion packages/orchestration/src/exos/portfolio-holder-kit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
*/

/**
Expand Down
15 changes: 8 additions & 7 deletions packages/orchestration/src/orchestration-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,13 @@ export type ChainAddress = {
*
* The methods available depend on the chain and its capabilities.
*/
export type OrchestrationAccount<CI extends ChainInfo> = OrchestrationAccountI &
(CI extends CosmosChainInfo
? CI['chainId'] extends `agoric${string}`
? LocalAccountMethods
: CosmosChainAccountMethods<CI>
: {});
export type OrchestrationAccount<CI extends ChainInfo> =
OrchestrationAccountCommon &
(CI extends CosmosChainInfo
? CI['chainId'] extends `agoric${string}`
? LocalAccountMethods
: CosmosChainAccountMethods<CI>
: {});

/**
* An object for access the core functions of a remote chain.
Expand Down Expand Up @@ -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
*/
Expand Down
4 changes: 2 additions & 2 deletions packages/orchestration/src/utils/orchestrationAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion packages/orchestration/test/fixtures/zoe-tools.flows.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
*/

Expand Down
6 changes: 3 additions & 3 deletions packages/orchestration/tools/ibc-mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
interface EncoderCommon<T> {
encode: (message: T) => {
finish: () => Uint8Array;
};
Expand All @@ -45,7 +45,7 @@ const toPacket = (obj: Record<string, any>): string =>
* @param message
*/
export function buildMsgResponseString<T>(
Encoder: EncoderI<T>,
Encoder: EncoderCommon<T>,
message: Partial<T>,
): string {
const encodedMsg = Encoder.encode(Encoder.fromPartial(message)).finish();
Expand Down Expand Up @@ -90,7 +90,7 @@ export function buildMsgErrorString(
* @param opts
*/
export function buildQueryResponseString<T>(
Encoder: EncoderI<T>,
Encoder: EncoderCommon<T>,
query: Partial<T>,
opts: Omit<ResponseQuery, 'key'> = {
value: new Uint8Array(),
Expand Down
6 changes: 3 additions & 3 deletions packages/time/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/zoe/tools/manualTimer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
*/
Expand All @@ -31,7 +31,7 @@ const nolog = (..._args) => {};
* @property {(nTimes: number, msg?: string) => Promise<void>} tickN
*/

/** @typedef {ReturnType<typeof buildManualTimer> & RemotableBrand<ManualTimerAdmin, TimerServiceI & ManualTimerAdmin> & ManualTimerAdmin} ZoeManualTimer */
/** @typedef {ReturnType<typeof buildManualTimer> & RemotableBrand<ManualTimerAdmin, TimerServiceCommon & ManualTimerAdmin> & ManualTimerAdmin} ZoeManualTimer */

/**
* A fake TimerService, for unit tests that do not use a real
Expand Down

0 comments on commit 1f25ee2

Please sign in to comment.