getConnectionInfo in the view of the primary chain #9718
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
646243b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f0bff9a7.agoric-sdk.pages.dev |
| Branch Preview URL: | https://ta-fix-chainhub.agoric-sdk.pages.dev |
| * @param {string | { chainId: string }} primary | ||
| * @param {string | { chainId: string }} counter |
There was a problem hiding this comment.
| * @param {string | { chainId: string }} primary | |
| * @param {string | { chainId: string }} counter | |
| * @param {string | { chainId: string }} primary the primary chain | |
| * @param {string | { chainId: string }} counter the counterparty chain |
counter == counterparty may not be immediately obvious
| export const ChainInfoShape = M.any(); | ||
| /** @type {TypedPattern<ChainInfo>} */ | ||
| export const ChainInfoShape = M.splitRecord({ | ||
| chainId: M.string(), |
There was a problem hiding this comment.
Maybe out of scope, but consider including a CosmosChainInfoShape for the second argument here. It seems we might be expecting a certain shape for IBCConnectionInfo.
icqEnabled: M.boolean() and stakingTokens: M.arrayOf({ denom: M.string() }) seem straight forward to incldue as well.
There was a problem hiding this comment.
Out of scope and I think it won't work
| @@ -223,38 +293,48 @@ export const makeChainHub = (agoricNames, vowTools) => { | |||
| * @param {IBCConnectionInfo} connectionInfo | |||
| */ | |||
| registerConnection(chainId1, chainId2, connectionInfo) { | |||
| const key = connectionKey(chainId1, chainId2); | |||
| connectionInfos.init(key, connectionInfo); | |||
| const [key, normalized] = normalizeConnectionInfo( | |||
There was a problem hiding this comment.
This I am a little confused by - is it necessary for the hotnewchain scenario, since we can't be certain they will give us noramlizedConnInfo? It would seem orchestration-proposal.js has everything normalized already at this point.
I wonder if it's better to throw if chainId1 > chainId2 so the user knows how they need to specify the perspective of connectionInfo. Alternatively, maybe a parameter name change would do the trick.
There was a problem hiding this comment.
I wonder if it's better to throw if
chainId1>chainId2so the user knows how they need to specify the perspective ofconnectionInfo.
That would not be reliable.
Alternatively, maybe a parameter name change would do the trick.
Agree, I'll improve the docs.
|
Forgot to attached this to my ✅ - Approving assuming feedback & CI failure will be addressed. CI failure seems to be due to stale snapshot, now that |
incidental
Description
Motivated by #9712 (comment)
This fixes up ChainHub to return IBCConnectionInfo in the perspective of the directed edge implied by the chain arguments.
The data is the same in either direction, just reversed, so we store it just once. This adds the logic to normalize on save and denormalize on read.
Security Considerations
none
Scaling Considerations
Some extra compute when saving or reading a chain. Low call rate.
Documentation Considerations
none
Testing Considerations
New tests
Upgrade Considerations
Not yet deployed