chore: chain account kit cleanup #9818
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
58bdd14
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0a50a7b4.agoric-sdk.pages.dev |
| Branch Preview URL: | https://pc-chain-account-kit-cleanup.agoric-sdk.pages.dev |
0xpatrickdev
force-pushed
the
pc/chain-account-kit-cleanup
branch
2 times, most recently
from
87b6cc6 to
88e497d
Compare
0xpatrickdev
force-pushed
the
pc/chain-account-kit-cleanup
branch
from
88e497d to
8ef7968
Compare
| @@ -100,16 +97,6 @@ export const prepareChainAccountKit = (zone, { watch, asVow }) => | |||
| 'ICA channel creation acknowledgement not yet received.', | |||
| ); | |||
| }, | |||
| getBalance(_denom) { | |||
| // TODO https://github.com/Agoric/agoric-sdk/issues/9610 | |||
There was a problem hiding this comment.
re
no plans to implement them on IcaAccount|ChainAccount
#9610 seems like a plan to implement them. but yes, I suppose it's postponed.
There was a problem hiding this comment.
Yes there's a plan, but these would appear in CosmosOrchestrationAccount. IcaAccount is concerned with lower level things likes Ports and Connections
| @@ -163,10 +163,12 @@ export const prepareChainAccountKit = (zone, { watch, asVow }) => | |||
| this.state.connection = connection; | |||
| this.state.remoteAddress = remoteAddr; | |||
| this.state.localAddress = localAddr; | |||
| const address = findAddressField(remoteAddr); | |||
| if (!address) { | |||
| console.error('🚨 failed to parse chain address', remoteAddr); | |||
There was a problem hiding this comment.
the red light is for "page somebody right away; an invariant was violated" per https://github.com/Agoric/agoric-sdk/wiki/Logging
But this can be triggered if the remote end mis-behaves. Seems like this should be a warning.
⚠️ indicates something that is not expected but was recovered from.
| @@ -9,11 +9,11 @@ classDiagram | |||
| LCAKit --* LocalchainAccount | |||
| ICQConnectionKit --* Port | |||
| ICQConnectionKit --* Connection | |||
| ChainAccountKit --* Port | |||
| ChainAccountKit --* Connection | |||
| IcaAccountKit --* Port | |||
There was a problem hiding this comment.
line 3 says,
As of 2024-05-29…
This change means that's not true anymore, but it's not accurate as of today either.
Please change the message to something like "Last verified 2024-05-29"
To update it before release I've added an item to #9757
There was a problem hiding this comment.
I didn't look closely in my find and replace but it's pretty dated. Please see: 72eabf7
0xpatrickdev
force-pushed
the
pc/chain-account-kit-cleanup
branch
3 times, most recently
from
565afc7 to
50b96c7
Compare
- these methods are part of OrchestrationAccountI, with no plans to implement them on IcaAccount|ChainAccount
- whether an address is parsable or not is not a consideration for the vstorage story (#9066) - instead, this should be interpreted as an unexpected error if this path is reached - throwing here would not solve much - this is a notifier divorced from the callsite. instead, - the caller will experience an error when they try and use UNPARSABLE_CHAIN_ADDRESS
- ChainAccountKit was a poor name - it implies this kit works for multiple chains - renamed to IcaAccountKit to convey it follows the ics27-1 protocol
0xpatrickdev
force-pushed
the
pc/chain-account-kit-cleanup
branch
from
50b96c7 to
58bdd14
Compare
refs: #9064
Description
UNPARSABLE_CHAIN_ADDRESSChainAccountKit->IcaAccountKitfindAddressFieldshould treat an empty string as anUNPARSABLE_ADDRESSIcaAccount.getPort()Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
n/a
Testing Considerations
n/a
Upgrade Considerations
n/a