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.

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

feat!: makeOsmosisSwap() using getDenomOn #9814

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
cf7f043
feat: getVBankAssetInfo() on orc.getChain('agoric')
dckc Jul 29, 2024
09928c8
chore: prune unused makeResumableAgoricNamesHack
dckc Jul 29, 2024
590dd03
chore(sendAnywhere): use agoric.getVBankAssetInfo()
dckc Jul 29, 2024
a0c3950
test: addInterchainAsset() test support
dckc Jul 31, 2024
8041ea7
chore(types): refine CosmosAssetInfo
dckc Jul 31, 2024
59dc885
chore(assets.fixture): add omniflixhub, noble
dckc Jul 31, 2024
308e6eb
feat: Orchestrator.getDenomOn (WIP)
dckc Jul 31, 2024
b046968
feat!: makeOsmosisSwap() using getDenomOn()
dckc Jul 31, 2024
9a48822
feat(LocalOrchestrationAccount): getBalance by brand
dckc Jul 31, 2024
dff557e
refactor(swapExample): factor out flows
dckc Jul 31, 2024
0890182
chore(swapExample): complete makeOsmosisSwap
dckc Jul 31, 2024
c37bcca
WIP: meeting note about PFM routing to issuing chain
dckc Aug 1, 2024
b555881
feat(ChainHub): getHoldingDenom
dckc Aug 1, 2024
66ad3c9
WIP: punt Orchestrator.getDenomOn impl
dckc Aug 1, 2024
60b30cb
WIP: remote-chain-facade getLocalDenom impl
dckc Aug 1, 2024
f7dd521
WIP: local-chain-facade: getLocalDenom impl (UNTESTED)
dckc Aug 1, 2024
46ac601
WIP: orch API: getLocalDenom (never mind getDenomOn)
dckc Aug 1, 2024
a06c29b
WIP: typeGuards: getLocalDenom
dckc Aug 1, 2024
6a213e9
WIP: makeOsmosisSwap: getDenomOn -> getLocalDenom
dckc Aug 1, 2024
7a6ba9b
fixup! WIP: makeOsmosisSwap: getDenomOn -> getLocalDenom
dckc Aug 1, 2024
f9e7f4d
WIP: remote chain facade gets chainHub too
dckc Aug 1, 2024
82e2040
WIP: always traces[0]???
dckc Aug 1, 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
24 changes: 23 additions & 1 deletion packages/orchestration/src/exos/orchestrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
DenomShape,
LocalChainAccountShape,
} from '../typeGuards.js';
import { denomHash } from '../utils/denomHash.js';

/**
* @import {Zone} from '@agoric/base-zone';
Expand All @@ -39,6 +40,11 @@ export const OrchestratorI = M.interface('Orchestrator', {
getChain: M.call(M.string()).returns(Vow$(ChainInfoShape)),
makeLocalAccount: M.call().returns(Vow$(LocalChainAccountShape)),
getBrandInfo: M.call(DenomShape).returns(BrandInfoShape),
getDenomOn: M.call(
DenomShape,
M.remotable('Chain'),
Copy link
Member

Choose a reason for hiding this comment

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

it is remotable but it lives in the same vat. can we have a stricter type in that case? (curious, not blocking)

Copy link
Member Author

Choose a reason for hiding this comment

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

no; patterns are pass-invariant, and "in the same vat" is the antithesis of pass-invariant.

M.remotable('Chain'),
).returns(Vow$(DenomShape)),
asAmount: M.call(DenomAmountShape).returns(AmountShape),
});

Expand Down Expand Up @@ -67,7 +73,7 @@ const prepareOrchestratorKit = (
chainByName,
makeLocalChainFacade,
makeRemoteChainFacade,
vowTools: { watch, asVow },
vowTools: { watch, asVow, when }, // TODO: no when
Copy link
Member

Choose a reason for hiding this comment

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

will we need retriable?

Copy link
Member Author

Choose a reason for hiding this comment

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

no idea; I guess I should look into it

},
) =>
zone.exoClassKit(
Expand Down Expand Up @@ -156,6 +162,22 @@ const prepareOrchestratorKit = (
// @ts-expect-error XXX HostOf<> not quite right?
return harden({ chain, base, brand, baseDenom });
},

/** @type {HostOf<Orchestrator['getDenomOn']>} */
getDenomOn(baseDenom, base, chain) {
// TODO: when -> watch
return asVow(async () => {
const baseInfo = await when(base.getChainInfo()); // XXX when
const chainInfo = await when(chain.getChainInfo());
const { transferChannel } = await when(
chainHub.getConnectionInfo(chainInfo.chainId, baseInfo.chainId),
);
const { channelId, portId } = transferChannel;
const hash = denomHash({ portId, channelId, denom: baseDenom });
return `ibc/${hash}`;
});
},

/** @type {HostOf<Orchestrator['asAmount']>} */
asAmount: () => Fail`not yet implemented`,
},
Expand Down
20 changes: 18 additions & 2 deletions packages/orchestration/src/orchestration-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ export interface Chain<CI extends ChainInfo> {
*/
makeAccount: () => Promise<OrchestrationAccount<CI>>;
// FUTURE supply optional port object; also fetch port object

// TODO provide a way to get the local denom/brand/whatever for this chain
Copy link
Member Author

Choose a reason for hiding this comment

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

getDenomOn() is intended to provide this.

It's on Orchestrator instead of Chain because the Chain implementation doesn't have ready access to chainHub. Perhaps we shouldn't let internal details like that drive the external API, but it was the easiest thing that seems to work.

Copy link
Member Author

Choose a reason for hiding this comment

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

intended design is/was:

osmosis.getLocalDenom(baseDemon)

}

/**
Expand Down Expand Up @@ -124,6 +122,24 @@ export interface Orchestrator {
/** the Denom for the underlying asset on its issuer chain */
baseDenom: Denom;
};

/**
* Get denom on a holding chain.
*
* @param baseDenom denom on issuing chain
* @param base issuing chain
* @param chain holding chain - connection to base chain must be regitered in chainHub
Copy link
Member

Choose a reason for hiding this comment

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

typo

* @returns denom on holding chain
*/
getDenomOn: <
Copy link
Member

Choose a reason for hiding this comment

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

new top level interface method will need @tribble review

Copy link
Member Author

Choose a reason for hiding this comment

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

right...
+cc @mitdralla

Copy link
Member Author

@dckc dckc Jul 31, 2024

Choose a reason for hiding this comment

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

the name getDenomOn is sort of... uninspired

different arg names might help... orch.getDenomOn(baseDenom, issuingChain, holdingChain)

which suggests orch.getHoldingChainDenom(baseDenom, issuingChain, holdingChain)

HoldingChain extends keyof KnownChains,
Copy link
Member

Choose a reason for hiding this comment

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

not strictly necessary that it's a known chain. but we can tackle that when we have more use of contract-specified chains

Copy link
Member Author

Choose a reason for hiding this comment

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

I followed the structure of getBrandInfo(). I guess the comment applies to both?

IssuingChain extends keyof KnownChains,
>(
baseDenom: Denom,
base: Chain<KnownChains[IssuingChain]>,
chain: Chain<KnownChains[HoldingChain]>,
) => Promise<Denom>;

// TODO preload the mapping so this can be synchronous
/**
* Convert an amount described in native data to a local, structured Amount.
Expand Down