Skip to content

Commit

Permalink
feat!: transfer(dest, amount, opts) (#10139)
Browse files Browse the repository at this point in the history
incidental

## Description

reorders `orchAccount.transfer` parameters to align with `.send` and `.delegate` methods, which roughly follow `fn(from, to, amounts)`

### Security Considerations
n/a

### Scaling Considerations
n/a

### Documentation Considerations
PR motivated by documenters who noticed this inconsistency.

### Testing Considerations
Updates existing tests

### Upgrade Considerations
This is a breaking change and will likely affect `dapp-orchestration-basics` cc @Jovonni
  • Loading branch information
mergify[bot] authored Sep 25, 2024
2 parents c0c733c + f60c24d commit b282e92
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 38 deletions.
11 changes: 4 additions & 7 deletions packages/orchestration/src/examples/auto-stake-it-tap-kit.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,10 @@ const prepareStakingTapKit = (zone, { watch }) => {

const { localAccount, localDenom, remoteChainAddress } = this.state;
return watch(
E(localAccount).transfer(
{
denom: localDenom,
value: BigInt(tx.amount),
},
remoteChainAddress,
),
E(localAccount).transfer(remoteChainAddress, {
denom: localDenom,
value: BigInt(tx.amount),
}),
this.facets.transferWatcher,
BigInt(tx.amount),
);
Expand Down
2 changes: 1 addition & 1 deletion packages/orchestration/src/examples/send-anywhere.flows.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ export const sendIt = async (

try {
await contractState.localAccount.transfer(
{ denom, value: amt.value },
{
value: destAddr,
encoding: 'bech32',
chainId,
},
{ denom, value: amt.value },
);
} catch (e) {
await withdrawToSeat(contractState.localAccount, seat, give);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const depositAndDelegate = async (

const address = account.getAddress();
try {
await contractState.localAccount.transfer(give.Stake, address);
await contractState.localAccount.transfer(address, give.Stake);
} catch (cause) {
await zoeTools.withdrawToSeat(contractState.localAccount, seat, give);
const errMsg = makeError(`ibc transfer failed ${q(cause)}`);
Expand Down Expand Up @@ -105,7 +105,7 @@ export const undelegateAndTransfer = async (
) => {
await account.undelegate(delegations);
for (const { amount } of delegations) {
await account.transfer(amount, destination);
await account.transfer(destination, amount);
}
};
harden(undelegateAndTransfer);
2 changes: 1 addition & 1 deletion packages/orchestration/src/examples/unbond.flows.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ export const unbondAndTransfer = async (orch, { zcfTools }) => {
const strideAccount = await stride.makeAccount();

const balance = await osmoAccount.getBalance(osmoDenom);
await osmoAccount.transfer(balance, strideAccount.getAddress());
await osmoAccount.transfer(strideAccount.getAddress(), balance);
};
harden(unbondAndTransfer);
Original file line number Diff line number Diff line change
Expand Up @@ -681,14 +681,14 @@ export const prepareCosmosOrchestrationAccountKit = (
* {
* amount: AmountArg;
* destination: ChainAddress;
* opts: IBCMsgTransferOptions;
* opts?: IBCMsgTransferOptions;
* }
* >}
*/
const offerHandler = (seat, { amount, destination, opts }) => {
seat.exit();
return watch(
this.facets.holder.transfer(amount, destination, opts),
this.facets.holder.transfer(destination, amount, opts),
);
};
return zcf.makeInvitation(offerHandler, 'Transfer');
Expand Down Expand Up @@ -881,8 +881,8 @@ export const prepareCosmosOrchestrationAccountKit = (
},

/** @type {HostOf<OrchestrationAccountI['transfer']>} */
transfer(amount, destination, opts) {
trace('transfer', amount, destination, opts);
transfer(destination, amount, opts) {
trace('transfer', destination, amount, opts);
return asVow(() => {
const { helper } = this.facets;
const token = helper.amountToCoin(amount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,14 @@ export const prepareLocalOrchestrationAccountKit = (
* {
* amount: AmountArg;
* destination: ChainAddress;
* opts: IBCMsgTransferOptions;
* opts?: IBCMsgTransferOptions;
* }
* >}
*/
const offerHandler = (seat, { amount, destination, opts }) => {
seat.exit();
return watch(
this.facets.holder.transfer(amount, destination, opts),
this.facets.holder.transfer(destination, amount, opts),
);
};
return zcf.makeInvitation(offerHandler, 'Transfer');
Expand Down Expand Up @@ -675,15 +675,15 @@ export const prepareLocalOrchestrationAccountKit = (
});
},
/**
* @param {ChainAddress} destination
* @param {AmountArg} amount an ERTP {@link Amount} or a
* {@link DenomAmount}
* @param {ChainAddress} destination
* @param {IBCMsgTransferOptions} [opts] if either timeoutHeight or
* timeoutTimestamp are not supplied, a default timeoutTimestamp will
* be set for 5 minutes in the future
* @returns {Vow<any>}
*/
transfer(amount, destination, opts) {
transfer(destination, amount, opts) {
return asVow(() => {
trace('Transferring funds from LCA over IBC');

Expand Down
2 changes: 1 addition & 1 deletion packages/orchestration/src/orchestration-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ export interface OrchestrationAccountI {
* TODO document the mapping from the address to the destination chain.
*/
transfer: (
amount: AmountArg,
destination: ChainAddress,
amount: AmountArg,
opts?: IBCMsgTransferOptions,
) => Promise<void>;

Expand Down
2 changes: 1 addition & 1 deletion packages/orchestration/src/utils/orchestrationAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const orchestrationAccountMethods = {
sendAll: M.call(ChainAddressShape, M.arrayOf(AmountArgShape)).returns(
VowShape,
),
transfer: M.call(AmountArgShape, ChainAddressShape)
transfer: M.call(ChainAddressShape, AmountArgShape)
.optional(IBCTransferOptionsShape)
.returns(VowShape),
transferSteps: M.call(AmountArgShape, M.any()).returns(VowShape),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ test('transfer', async t => {
encoding: 'bech32',
};
const mockAmountArg: AmountArg = { value: 10n, denom: 'ibc/uusdchash' };
const res = E(account).transfer(mockAmountArg, mockDestination);
const res = E(account).transfer(mockDestination, mockAmountArg);
await eventLoopIteration();

t.deepEqual(
Expand All @@ -220,7 +220,7 @@ test('transfer', async t => {
t.is(await res, undefined, 'transfer returns undefined');

t.log('transfer accepts custom memo');
await E(account).transfer(mockAmountArg, mockDestination, {
await E(account).transfer(mockDestination, mockAmountArg, {
memo: JSON.stringify({ custom: 'pfm memo' }),
});
t.like(
Expand All @@ -232,7 +232,7 @@ test('transfer', async t => {
);

t.log('transfer accepts custom timeoutHeight');
await E(account).transfer(mockAmountArg, mockDestination, {
await E(account).transfer(mockDestination, mockAmountArg, {
timeoutHeight: {
revisionHeight: 1000n,
revisionNumber: 1n,
Expand All @@ -251,7 +251,7 @@ test('transfer', async t => {
);

t.log('transfer accepts custom timeoutTimestamp');
await E(account).transfer(mockAmountArg, mockDestination, {
await E(account).transfer(mockDestination, mockAmountArg, {
timeoutTimestamp: 999n,
});
t.like(
Expand All @@ -267,7 +267,7 @@ test('transfer', async t => {
);

t.log('transfer accepts custom timeoutHeight and timeoutTimestamp');
await E(account).transfer(mockAmountArg, mockDestination, {
await E(account).transfer(mockDestination, mockAmountArg, {
timeoutHeight: {
revisionHeight: 5000n,
revisionNumber: 5n,
Expand All @@ -288,17 +288,20 @@ test('transfer', async t => {

t.log('transfer throws if connection is not in its chainHub');
await t.throwsAsync(
E(account).transfer(mockAmountArg, {
...mockDestination,
chainId: 'unknown-1',
}),
E(account).transfer(
{
...mockDestination,
chainId: 'unknown-1',
},
mockAmountArg,
),
{
message: 'connection not found: cosmoshub-4<->unknown-1',
},
);

t.log('transfer throws if asset is not in its chainHub');
await t.throwsAsync(E(account).transfer(ist.make(10n), mockDestination), {
await t.throwsAsync(E(account).transfer(mockDestination, ist.make(10n)), {
message: 'No denom for brand [object Alleged: IST brand]',
});
chainHub.registerAsset('uist', {
Expand All @@ -308,14 +311,14 @@ test('transfer', async t => {
chainName: 'agoric',
});
// uses uistTransfer mock above
await E(account).transfer(ist.make(10n), mockDestination);
await E(account).transfer(mockDestination, ist.make(10n));

t.log('transfer timeout error recieved and handled from the bridge');
await t.throwsAsync(
E(account).transfer(
{ ...mockAmountArg, value: SIMULATED_ERRORS.TIMEOUT },
mockDestination,
),
E(account).transfer(mockDestination, {
...mockAmountArg,
value: SIMULATED_ERRORS.TIMEOUT,
}),
{
message: 'ABCI code: 5: error handling packet: see events for details',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ test('transfer', async t => {
dest: ChainAddress,
opts = {},
) => {
const transferP = VE(account).transfer(amount, dest, opts);
const transferP = VE(account).transfer(dest, amount, opts);
sequence += 1n;
// Ensure the toBridge of the transferP happens before the fromBridge is awaited after this function returns
await eventLoopIteration();
Expand Down Expand Up @@ -194,7 +194,7 @@ test('transfer', async t => {
};
// XXX dev has to know not to startTransfer here
await t.throwsAsync(
VE(account).transfer({ denom: 'ubld', value: 1n }, unknownDestination),
VE(account).transfer(unknownDestination, { denom: 'ubld', value: 1n }),
{ message: /connection not found: agoric-3<->fakenet/ },
'cannot create transfer msg with unknown chainId',
);
Expand Down

0 comments on commit b282e92

Please sign in to comment.