-
Notifications
You must be signed in to change notification settings - Fork 212
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
chore(fast-usdc): limited operation before connecting to noble #10641
Changes from all commits
046779f
eb82ae3
508a3e0
c9ba09e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
#!/bin/bash | ||
set -euo pipefail | ||
|
||
yarn ava | ||
|
||
# TODO get CLI test passing and part of CI | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,7 @@ const AdvancerVowCtxShape = M.splitRecord( | |
const AdvancerKitI = harden({ | ||
advancer: M.interface('AdvancerI', { | ||
handleTransactionEvent: M.callWhen(CctpTxEvidenceShape).returns(), | ||
setIntermediateRecipient: M.call(ChainAddressShape).returns(), | ||
}), | ||
depositHandler: M.interface('DepositHandlerI', { | ||
onFulfilled: M.call(M.undefined(), AdvancerVowCtxShape).returns(VowShape), | ||
|
@@ -95,7 +96,7 @@ export const prepareAdvancerKit = ( | |
usdc, | ||
vowTools: { watch, when }, | ||
zcf, | ||
} = /** @type {AdvancerKitPowers} */ ({}), | ||
}, | ||
) => { | ||
assertAllDefined({ | ||
chainHub, | ||
|
@@ -116,10 +117,15 @@ export const prepareAdvancerKit = ( | |
* notifyFacet: import('./settler.js').SettlerKit['notify']; | ||
* borrowerFacet: LiquidityPoolKit['borrower']; | ||
* poolAccount: HostInterface<OrchestrationAccount<{chainId: 'agoric'}>>; | ||
* intermediateRecipient: ChainAddress; | ||
* intermediateRecipient?: ChainAddress; | ||
* }} config | ||
*/ | ||
config => harden(config), | ||
config => | ||
harden({ | ||
...config, | ||
// make sure the state record has this property, perhaps with an undefined value | ||
intermediateRecipient: config.intermediateRecipient, | ||
}), | ||
{ | ||
advancer: { | ||
/** | ||
|
@@ -181,6 +187,10 @@ export const prepareAdvancerKit = ( | |
statusManager.observe(evidence); | ||
} | ||
}, | ||
/** @param {ChainAddress} intermediateRecipient */ | ||
setIntermediateRecipient(intermediateRecipient) { | ||
this.state.intermediateRecipient = intermediateRecipient; | ||
}, | ||
}, | ||
depositHandler: { | ||
/** | ||
|
@@ -192,15 +202,8 @@ export const prepareAdvancerKit = ( | |
const { destination, advanceAmount, ...detail } = ctx; | ||
const transferV = E(poolAccount).transfer( | ||
destination, | ||
{ | ||
denom: usdc.denom, | ||
value: advanceAmount.value, | ||
}, | ||
{ | ||
forwardOpts: { | ||
intermediateRecipient, | ||
}, | ||
}, | ||
{ denom: usdc.denom, value: advanceAmount.value }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised prettier change the second argument too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it didn't. I tried a helper for the 1st arg and then took it out, and used the more concise form when I took it out. I realized the second arg didn't need the line-breaks either. |
||
{ forwardOpts: { intermediateRecipient } }, | ||
); | ||
return watch(transferV, this.facets.transferHandler, { | ||
destination, | ||
|
@@ -259,7 +262,7 @@ export const prepareAdvancerKit = ( | |
notifyFacet: M.remotable(), | ||
borrowerFacet: M.remotable(), | ||
poolAccount: M.remotable(), | ||
intermediateRecipient: ChainAddressShape, | ||
intermediateRecipient: M.opt(ChainAddressShape), | ||
}), | ||
}, | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,7 @@ export const prepareSettler = ( | |
{ | ||
creator: M.interface('SettlerCreatorI', { | ||
monitorMintingDeposits: M.callWhen().returns(M.any()), | ||
setIntermediateRecipient: M.call(ChainAddressShape).returns(), | ||
}), | ||
tap: M.interface('SettlerTapI', { | ||
receiveUpcall: M.call(M.record()).returns(M.promise()), | ||
|
@@ -94,13 +95,15 @@ export const prepareSettler = ( | |
* remoteDenom: Denom; | ||
* repayer: LiquidityPoolKit['repayer']; | ||
* settlementAccount: HostInterface<OrchestrationAccount<{ chainId: 'agoric' }>> | ||
* intermediateRecipient: ChainAddress; | ||
* intermediateRecipient?: ChainAddress; | ||
* }} config | ||
*/ | ||
config => { | ||
log('config', config); | ||
return { | ||
...config, | ||
// make sure the state record has this property, perhaps with an undefined value | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider using "the state schema" to suggest the implications There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The schema is below in |
||
intermediateRecipient: config.intermediateRecipient, | ||
/** @type {HostInterface<TargetRegistration>|undefined} */ | ||
registration: undefined, | ||
/** @type {SetStore<ReturnType<typeof makeMintedEarlyKey>>} */ | ||
|
@@ -117,6 +120,10 @@ export const prepareSettler = ( | |
assert.typeof(registration, 'object'); | ||
this.state.registration = registration; | ||
}, | ||
/** @param {ChainAddress} intermediateRecipient */ | ||
setIntermediateRecipient(intermediateRecipient) { | ||
this.state.intermediateRecipient = intermediateRecipient; | ||
}, | ||
}, | ||
tap: { | ||
/** @param {VTransferIBCEvent} event */ | ||
|
@@ -265,11 +272,7 @@ export const prepareSettler = ( | |
const txfrV = E(settlementAccount).transfer( | ||
dest, | ||
AmountMath.make(USDC, fullValue), | ||
{ | ||
forwardOpts: { | ||
intermediateRecipient, | ||
}, | ||
}, | ||
{ forwardOpts: { intermediateRecipient } }, | ||
); | ||
void vowTools.watch(txfrV, this.facets.transferHandler, { | ||
txHash, | ||
|
@@ -312,7 +315,7 @@ export const prepareSettler = ( | |
sourceChannel: M.string(), | ||
remoteDenom: M.string(), | ||
mintedEarly: M.remotable('mintedEarly'), | ||
intermediateRecipient: ChainAddressShape, | ||
intermediateRecipient: M.opt(ChainAddressShape), | ||
}), | ||
}, | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ import type { | |
} from '@agoric/orchestration'; | ||
import type { IBCChannelID } from '@agoric/vats'; | ||
import type { Amount } from '@agoric/ertp'; | ||
import type { Passable } from '@endo/pass-style'; | ||
import type { CopyRecord, Passable } from '@endo/pass-style'; | ||
import type { PendingTxStatus } from './constants.js'; | ||
import type { FastUsdcTerms } from './fast-usdc.contract.js'; | ||
|
||
|
@@ -78,14 +78,15 @@ export interface FeedPolicy { | |
eventFilter?: string; | ||
} | ||
|
||
export type FastUSDCConfig = Passable & { | ||
export type FastUSDCConfig = { | ||
terms: FastUsdcTerms; | ||
oracles: Record<string, string>; | ||
feeConfig: FeeConfig; | ||
feedPolicy: FeedPolicy & Passable; | ||
noNoble: boolean; // support a3p-integration, which has no noble chain | ||
chainInfo: Record<string, CosmosChainInfo & Passable>; | ||
assetInfo: [Denom, DenomDetail & { brandKey?: string }][]; | ||
}; | ||
} & CopyRecord; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider putting it at the front (like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would we want it to be noticeable? It's only here due to limitations in tsc or the Passable type or something. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because it's unexpected |
||
|
||
export type * from './constants.js'; | ||
export type { LiquidityPoolKit } from './exos/liquidity-pool.js'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,11 @@ export const defaultAssetInfo = [ | |
}, | ||
], | ||
]; | ||
harden(defaultAssetInfo); | ||
|
||
const agoricAssetInfo = defaultAssetInfo.filter( | ||
([_d, i]) => i.chainName === 'agoric', | ||
); | ||
|
||
/** | ||
* @type {Record<string, Pick<FastUSDCConfig, 'oracles' | 'feedPolicy' | 'chainInfo' | 'assetInfo' >>} | ||
|
@@ -43,14 +48,19 @@ export const defaultAssetInfo = [ | |
* meanwhile, use price oracle addresses (from updatePriceFeeds.js). | ||
*/ | ||
export const configurations = { | ||
/** | ||
* NOTE: The a3p-integration runtime does _not_ include | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
* a noble chain; this limits functionality to advancing | ||
* to the Agoric chain. | ||
*/ | ||
A3P_INTEGRATION: { | ||
oracles: { | ||
gov1: 'agoric1ee9hr0jyrxhy999y755mp862ljgycmwyp4pl7q', | ||
gov2: 'agoric1wrfh296eu2z34p6pah7q04jjuyj3mxu9v98277', | ||
gov3: 'agoric1ydzxwh6f893jvpaslmaz6l8j2ulup9a7x8qvvq', | ||
}, | ||
feedPolicy: { | ||
nobleAgoricChannelId: 'TODO', | ||
nobleAgoricChannelId: 'channel-does-not-exist', | ||
nobleDomainId: 4, | ||
chainPolicies: { | ||
Arbitrum: { | ||
|
@@ -63,9 +73,13 @@ export const configurations = { | |
}, | ||
}, | ||
chainInfo: /** @type {Record<string, CosmosChainInfo & Passable>} */ ( | ||
withChainCapabilities(fetchedChainInfo) | ||
withChainCapabilities({ | ||
agoric: fetchedChainInfo.agoric, | ||
// registering USDC-on-agoric requires registering the noble chain | ||
noble: fetchedChainInfo.noble, | ||
}) | ||
), | ||
assetInfo: defaultAssetInfo, | ||
assetInfo: agoricAssetInfo, | ||
}, | ||
MAINNET: { | ||
oracles: { | ||
|
@@ -139,3 +153,4 @@ export const configurations = { | |
assetInfo: defaultAssetInfo, // TODO: use emerynet values | ||
}, | ||
}; | ||
harden(configurations); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider making the option phrased positively. e.g.
connectNoble
.I think the default could still be false. connecting to Noble has strictly more dependencies on the environment so makes sense to be opt-in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's more dependencies, but without a connection to noble, the whole notion of "fast usdc" goes away. It's supported only for a limited form of testing. If somebody forgot to set the flag, the normal condition should be that the product works.