Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(orchestration): remove cancelToken + rename createAccount -> makeAccount #9312

Merged
merged 7 commits into from
May 2, 2024
Merged

fix(orchestration): remove cancelToken + rename createAccount -> makeAccount #9312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
be14fa5
fix(orchestration): remove cancelToken logic from undelegate
0xpatrickdev May 1, 2024
330ebea
refactor(orchestration): rename createAccount -> makeAccount
0xpatrickdev May 1, 2024
f951cde
fix(orchestration): rename `.getAccountAddress` -> `.getAddress`
0xpatrickdev May 1, 2024
05cce52
refactor(localchain): createAccount -> makeAccount
0xpatrickdev May 1, 2024
f2a6d41
refactor(orchestration): prefer ChainAddress object to string
0xpatrickdev May 1, 2024
678f21f
feat(orchestration): align ChainAccount spec with current implementation
0xpatrickdev May 2, 2024
39a980a
refactor(chainAccountKit): move to own file
0xpatrickdev May 2, 2024
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
6 changes: 3 additions & 3 deletions packages/boot/test/bootstrapTests/test-orchestration.ts
View file
Open in desktop
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
View file
Open in desktop
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(
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need EV?

Copy link
Member Author

Choose a reason for hiding this comment

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

EV is a testing utility for bootstrap tests

Copy link
Member

Choose a reason for hiding this comment

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

It's like E but works through RunUtils.

agoric-sdk/packages/SwingSet/tools/run-utils.js

Lines 55 to 61 in 231416e

// IMPORTANT WARNING TO USERS OF `EV`
//
// `EV` presents an abstraction that can be used (within tests only!) to get
// much of the convenience with respect to messaging that `E` provides in
// normal code. However, this convenience comes with a huge caveat that all
// users of this convenience feature MUST keep in mind.
//

'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
View file
Open in desktop
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();
Copy link
Member

Choose a reason for hiding this comment

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

btw feel free to name the variable accountAddress if that seems more informative

Copy link
Member Author

@0xpatrickdev 0xpatrickdev May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

👍 going to keep it as address for consistency. I might lobby to change getRemoteAddress() to getRemoteChannelAddress() (and getLocalAddress() -> getLocalChannelAddress()) so no one gets confused by these.

Copy link
Member Author

@0xpatrickdev 0xpatrickdev May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

On second thought, since I am going to change things to use ChainAddress, i will call the local variable chainAddress. Was awkward typing this.state.address.address to form a message.

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading