Skip to content

Commit

Permalink
fix(orchestration): remove cancelToken + rename createAccount -> …
Browse files Browse the repository at this point in the history
…`makeAccount` (#9312)

<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v                               ✰  Thanks for creating a PR! ✰
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

<!-- Most PRs should close a specific Issue. All PRs should at least
reference one or more Issues. Edit and/or delete the following lines as
appropriate (note: you don't need both `refs` and `closes` for the same
one): -->

closes: #XXXX
refs: #9207 

## Description

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review.
-->

- removes `cancelToken` logic from `.undelegate()`
- renames `createAccount` to `makeAccount` for `orchestration` and
`localchain` ([Method Naming
Structure](https://docs.agoric.com/guides/ertp/#method-naming-structure))
- `make<Foo>()`: Creates a new Foo object and returns only that object.
   - `create<Foo>()`: Creates a new Foo, but doesn't return it.
 - renames `getChainAddress`  -> `getAddress` on `orchestration`


### Security Considerations

<!-- Does this change introduce new assumptions or dependencies that, if
violated, could introduce security vulnerabilities? How does this PR
change the boundaries between mutually-suspicious components? What new
authorities are introduced by this change, perhaps by new API calls?
-->

### Scaling Considerations

<!-- Does this change require or encourage significant increase in
consumption of CPU cycles, RAM, on-chain storage, message exchanges, or
other scarce resources? If so, can that be prevented or mitigated? -->

### Documentation Considerations

<!-- Give our docs folks some hints about what needs to be described to
downstream users.

Backwards compatibility: what happens to existing data or deployments
when this code is shipped? Do we need to instruct users to do something
to upgrade their saved data? If there is no upgrade path possible, how
bad will that be for users?

-->

### Testing Considerations

<!-- Every PR should of course come with tests of its own functionality.
What additional tests are still needed beyond those unit tests? How does
this affect CI, other test automation, or the testnet?
-->

### Upgrade Considerations

<!-- What aspects of this PR are relevant to upgrading live production
systems, and how should they be addressed? -->
  • Loading branch information
mergify[bot] authored May 2, 2024
2 parents 8e82183 + 39a980a commit 5a4779a
Show file tree
Hide file tree
Showing 12 changed files with 316 additions and 267 deletions.
6 changes: 3 additions & 3 deletions packages/boot/test/bootstrapTests/test-orchestration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ test.serial('stakeAtom - repl-style', async t => {
const publicFacet = await EV(zoe).getPublicFacet(instance);
t.truthy(publicFacet, 'stakeAtom publicFacet is available');

const account = await EV(publicFacet).createAccount();
const account = await EV(publicFacet).makeAccount();
t.log('account', account);
t.truthy(account, 'createAccount returns an account on ATOM connection');
t.truthy(account, 'makeAccount returns an account on ATOM connection');
t.truthy(
matches(account, M.remotable('ChainAccount')),
'account is a remotable',
Expand All @@ -140,7 +140,7 @@ test.serial('stakeAtom - smart wallet', async t => {
invitationSpec: {
source: 'agoricContract',
instancePath: ['stakeAtom'],
callPipe: [['makeCreateAccountInvitation']],
callPipe: [['makeAcountInvitationMaker']],
},
proposal: {},
});
Expand Down
42 changes: 21 additions & 21 deletions packages/boot/test/bootstrapTests/test-vat-orchestration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
MsgDelegateResponse,
} from '@agoric/cosmic-proto/cosmos/staking/v1beta1/tx.js';
import { Any } from '@agoric/cosmic-proto/google/protobuf/any.js';
import type { ChainAccount, OrchestrationService } from '@agoric/orchestration';
import type { OrchestrationService } from '@agoric/orchestration';
import { decodeBase64 } from '@endo/base64';
import { M, matches } from '@endo/patterns';
import { makeWalletFactoryContext } from './walletFactory.ts';
Expand Down Expand Up @@ -60,38 +60,38 @@ test.before(async t => {

test.after.always(t => t.context.shutdown?.());

test('createAccount returns an ICA connection', async t => {
test('makeAccount returns an ICA connection', async t => {
const {
runUtils: { EV },
} = t.context;

const orchestration = await EV.vat('bootstrap').consumeItem('orchestration');
const orchestration: OrchestrationService =
await EV.vat('bootstrap').consumeItem('orchestration');

const account = await EV(orchestration).createAccount(
const account = await EV(orchestration).makeAccount(
'connection-0',
'connection-0',
);
t.truthy(account, 'createAccount returns an account');
t.truthy(account, 'makeAccount returns an account');
t.truthy(
matches(account, M.remotable('ChainAccount')),
'account is a remotable',
);
const [remoteAddress, localAddress, accountAddress, port] = await Promise.all(
[
EV(account).getRemoteAddress(),
EV(account).getLocalAddress(),
EV(account).getAccountAddress(),
EV(account).getPort(),
],
);
const [remoteAddress, localAddress, chainAddress, port] = await Promise.all([
EV(account).getRemoteAddress(),
EV(account).getLocalAddress(),
EV(account).getAddress(),
EV(account).getPort(),
]);
t.regex(remoteAddress, /icahost/);
t.regex(localAddress, /icacontroller/);
t.regex(accountAddress, /cosmos1/);
t.regex(chainAddress.address, /cosmos1/);
t.regex(chainAddress.chainId, /FIXME/); // TODO, use a real chainId #9063
t.truthy(matches(port, M.remotable('Port')));
t.log('ICA Account Addresses', {
remoteAddress,
localAddress,
accountAddress,
chainAddress,
});
});

Expand All @@ -103,11 +103,11 @@ test('ICA connection can be closed', async t => {
const orchestration: OrchestrationService =
await EV.vat('bootstrap').consumeItem('orchestration');

const account = await EV(orchestration).createAccount(
const account = await EV(orchestration).makeAccount(
'connection-0',
'connection-0',
);
t.truthy(account, 'createAccount returns an account');
t.truthy(account, 'makeAccount returns an account');

await EV(account).close();

Expand All @@ -121,13 +121,14 @@ test('ICA connection can send msg with proto3', async t => {
runUtils: { EV },
} = t.context;

const orchestration = await EV.vat('bootstrap').consumeItem('orchestration');
const orchestration: OrchestrationService =
await EV.vat('bootstrap').consumeItem('orchestration');

const account: ChainAccount = await EV(orchestration).createAccount(
const account = await EV(orchestration).makeAccount(
'connection-0',
'connection-0',
);
t.truthy(account, 'createAccount returns an account');
t.truthy(account, 'makeAccount returns an account');

// @ts-expect-error intentional
await t.throwsAsync(EV(account).executeEncodedTx('malformed'), {
Expand All @@ -149,7 +150,6 @@ test('ICA connection can send msg with proto3', async t => {

const txWithOptions = await EV(account).executeEncodedTx(
[delegateMsgSuccess],
// @ts-expect-error XXX TxBody interface
{
memo: 'TESTING',
timeoutHeight: 1_000_000_000n,
Expand Down
24 changes: 12 additions & 12 deletions packages/orchestration/src/examples/stakeAtom.contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ export const start = async (zcf, privateArgs, baggage) => {
zcf,
);

async function createAccount() {
const account = await E(orchestration).createAccount(
async function makeAccount() {
const account = await E(orchestration).makeAccount(
hostConnectionId,
controllerConnectionId,
);
const accountAddress = await E(account).getAccountAddress();
trace('account address', accountAddress);
const address = await E(account).getAddress();
trace('chain address', address);
const { holder, invitationMakers } = makeStakingAccountKit(
account,
storageNode,
accountAddress,
address,
);
return {
publicSubscribers: holder.getPublicTopics(),
Expand All @@ -69,20 +69,20 @@ export const start = async (zcf, privateArgs, baggage) => {
const publicFacet = zone.exo(
'StakeAtom',
M.interface('StakeAtomI', {
createAccount: M.callWhen().returns(M.remotable('ChainAccount')),
makeCreateAccountInvitation: M.call().returns(M.promise()),
makeAccount: M.callWhen().returns(M.remotable('ChainAccount')),
makeAcountInvitationMaker: M.call().returns(M.promise()),
}),
{
async createAccount() {
trace('createAccount');
return createAccount().then(({ account }) => account);
async makeAccount() {
trace('makeAccount');
return makeAccount().then(({ account }) => account);
},
makeCreateAccountInvitation() {
makeAcountInvitationMaker() {
trace('makeCreateAccountInvitation');
return zcf.makeInvitation(
async seat => {
seat.exit();
return createAccount();
return makeAccount();
},
'wantStakingAccount',
undefined,
Expand Down
2 changes: 1 addition & 1 deletion packages/orchestration/src/examples/stakeBld.contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const start = async (zcf, privateArgs, baggage) => {
const { give } = seat.getProposal();
trace('makeStakeBldInvitation', give);
// XXX type appears local but it's remote
const account = await E(privateArgs.localchain).createAccount();
const account = await E(privateArgs.localchain).makeAccount();
const lcaSeatKit = zcf.makeEmptySeatKit();
atomicTransfer(zcf, seat, lcaSeatKit.zcfSeat, give);
seat.exit();
Expand Down
6 changes: 3 additions & 3 deletions packages/orchestration/src/examples/swapExample.contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ export const start = async (zcf, privateArgs) => {
const agoric = await orch.getChain('agoric');

const [celestiaAccount, localAccount] = await Promise.all([
celestia.createAccount(),
agoric.createAccount(),
celestia.makeAccount(),
agoric.makeAccount(),
]);

const tiaAddress = await celestiaAccount.getChainAddress();
const tiaAddress = await celestiaAccount.getAddress();

// deposit funds from user seat to LocalChainAccount
const seatKit = zcf.makeEmptySeatKit();
Expand Down
10 changes: 4 additions & 6 deletions packages/orchestration/src/examples/unbondExample.contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,19 @@ export const start = async (zcf, privateArgs) => {
// ??? could these be passed in? It would reduce the size of this handler,
// keeping it focused on long-running operations.
const celestia = await orch.getChain('celestia');
const celestiaAccount = await celestia.createAccount();
const celestiaAccount = await celestia.makeAccount();

const delegations = await celestiaAccount.getDelegations();
const [undelegation] = await celestiaAccount.undelegate(delegations);

// wait for the undelegations to be complete (may take weeks)
await undelegation.completion;
await celestiaAccount.undelegate(delegations);

// ??? should this be synchronous? depends on how names are resolved.
const stride = await orch.getChain('stride');
const strideAccount = await stride.createAccount();
const strideAccount = await stride.makeAccount();

// TODO the `TIA` string actually needs to be the Brand from AgoricNames
const tiaAmt = await celestiaAccount.getBalance('TIA');
await celestiaAccount.transfer(tiaAmt, strideAccount.getChainAddress());
await celestiaAccount.transfer(tiaAmt, strideAccount.getAddress());

await strideAccount.liquidStake(tiaAmt);
},
Expand Down
Loading

0 comments on commit 5a4779a

Please sign in to comment.