-
Notifications
You must be signed in to change notification settings - Fork 206
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
9281 durable orchestrator #9532
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
chainId: localChainInfo.chainId, | ||
addressEncoding: 'bech32', | ||
}), | ||
// FIXME storage path |
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.
What ticket capture this? Maybe vstorage logging ?
// FIXME storage path | |
// FIXME storage path https://github.com/Agoric/agoric-sdk/issues/9066 |
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.
Thanks for looking these up. I expect we'll get to them soon so I'll let this merge as is to save the CI time. Maybe include the links on a next PR.
@@ -169,7 +169,7 @@ export const prepareChainAccountKit = zone => | |||
}, | |||
async onClose(_connection, reason) { | |||
trace(`ICA Channel closed. Reason: ${reason}`); | |||
// XXX handle connection closing | |||
// FIXME handle connection closing |
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.
// FIXME handle connection closing | |
// FIXME handle connection closing https://github.com/Agoric/agoric-sdk/issues/9192 |
@@ -67,18 +58,18 @@ const PUBLIC_TOPICS = { | |||
* @param {Remote<TimerService>} timerService | |||
* @param {ChainHub} chainHub | |||
*/ | |||
export const prepareLocalChainAccountKit = ( | |||
export const prepareLocalOrchestrationAccountKit = ( |
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.
Nice rename 👏
return V(this.state.account).deposit(payment, optAmountShape); | ||
/** @type {OrchestrationAccount<any>['deposit']} */ | ||
async deposit(payment) { | ||
await V(this.state.account).deposit(payment); |
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.
In a future PR we might consider updating OrchestrationAccountI['deposit']
to return Promise<Amount<'nat'>>
. I don't see a reason to swallow that response.
Tangentially - with the revised implementation plan for #9193 only LocalOrchestrationAccount
will have .deposit()
.
refs: #9281
Description
AsyncFlow requires that everything passing the membrane is durable. This makes the facade objects durable to conform.
Following #9529
Security Considerations
none
Scaling Considerations
Exo for each chain and each account
Documentation Considerations
none
Testing Considerations
Existing coverage
Upgrade Considerations
not yet deployed