-
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
9281 chainHub exo #9538
9281 chainHub exo #9538
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
@@ -54,7 +54,38 @@ export const connectionKey = (chainId1, chainId2) => { | |||
return [chainId1, chainId2].sort().join(CHAIN_ID_SEPARATOR); | |||
}; | |||
|
|||
const ChainIdArgShape = M.or( |
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:
const ChainIdArgShape = M.or( | |
/** accepts `chainId` or `ChainAddress` object */ | |
const ChainIdArgShape = M.or( |
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.
good idea. I'll make a note to get that into a future PR
M.string(), | ||
IBCConnectionInfoShape, | ||
).returns(), | ||
getConnectionInfo: M.callWhen(ChainIdArgShape, ChainIdArgShape).returns( |
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.
callWhen
is OK in asyncFlow after all?
I can imagine waiting for tests that show that it's not, but in that case, why not wait for tests that show that chainHub has to be an exo in the 1st place?
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.
Nope, I learned last night it's not.
We did hit a test failure with the object itself not being durable but only in another branch that had a higher fidelity asyncFlow
refs: #9281
Description
AsyncFlow requires that everything passing the membrane is durable. This makes the ChainHub durable by making it an Exo.
Security Considerations
none
Scaling Considerations
It's a singleton, because there will only be one per vat.
Documentation Considerations
none
Testing Considerations
Existing coverage
Upgrade Considerations
Not yet deployed.
Now that it's an Exo, we have to consider its state an upgradability. We expect to make a new one on each vat invocation. The only state it has is cache of AgoricNames and data supplied by the contract.