Skip to content
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.

Sign up for GitHub

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

Jump to bottom

normalize (cosmos) ChainInfo schema #9807

Open
Tracked by #10230
0xpatrickdev opened this issue Jul 30, 2024 · 3 comments · May be fixed by #10109
Open
Tracked by #10230

normalize (cosmos) ChainInfo schema #9807

0xpatrickdev opened this issue Jul 30, 2024 · 3 comments · May be fixed by #10109
Labels
bug Something isn't working

Comments

@0xpatrickdev
Copy link
Member

What is the Problem Being Solved?

To facilitate contracts for @agoric/orchestration, chain and connection info will be published to vstorage for clients and contracts to access.

The ChainInfo schema is inconsistent. Namely:

  1. a mixture of snake_case and camelCase is used
  2. some keys use counterParty* while others use counterparty*
  3. Sometimes counterparty is an object, other times its flatted to counterPartyPortId and counterPartyChannelId
  4. All prefix.key_prefix values say 'FIXME'

Description of the Design

  1. use camelCase to comply with js eco conventions
  2. counterparty is one word not two
  3. Propose to flatten, but if nested is preferred we should do it consistently
  4. remove prefix.key_prefix from the schema - we can append later if needed

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations
@0xpatrickdev 0xpatrickdev added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Jul 30, 2024
@dckc
Copy link
Member

dckc commented Jul 31, 2024 •
edited
Loading

internal consistency doesn't seem as valuable as consistency with whatever external database we're mirroring.

what do the chain registry npm packages do?

a mixture of snake_case and camelCase is used

got an example?

p.s. I see denom_units in https://www.npmjs.com/package/chain-registry

@dckc
Copy link
Member

dckc commented Jul 31, 2024 •
edited
Loading

counterparty is one word not two
remove prefix.key_prefix

concur

@0xpatrickdev
Copy link
Member Author

0xpatrickdev commented Aug 7, 2024 •
edited
Loading

A point for not using a counterparty object and instead flattening the data structure - it might be common to destructure these values and the developer might need to alias to something like rPortID, rChannelID, rConnectionID. (Would be nice to avoid this if we anticipate it as a common pattern)

0xpatrickdev added a commit that referenced this issue Sep 18, 2024
@0xpatrickdev
feat: remove prefix.key_prefix from IBCConnectionInfo
78e701a 
- the field is both unused by any code and unpopulated ('FIXME')
- refs: #9807
@0xpatrickdev 0xpatrickdev mentioned this issue Sep 18, 2024
Merged
mergify bot added a commit that referenced this issue Sep 18, 2024
@mergify
chore: cleanup chain info (#10108)
f6ca68a 
refs: #9807

## Description
- Removes `counterparty.prefix.key_prefix: 'FIXME'` from every `agoricNames.chainConnection` entry in vstorage
- Removes `agoricNames.chain.agoriclocal` from vstorage

### Security Considerations
n/a

### Scaling Considerations
Incidentally reduces storage used on chain by writing less data to vstorage

### Documentation Considerations
n/a

### Testing Considerations
Existing tests were modified to accommodate these deletions.

### Upgrade Considerations
We've yet to run `init-orchestration.js`, so this does not have upgrade considerations. It should be part of upgrade-17, though.
@0xpatrickdev 0xpatrickdev linked a pull request Sep 18, 2024 that will close this issue
Draft
@0xpatrickdev 0xpatrickdev mentioned this issue Oct 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: normalize chain info Agoric/agoric-sdk
2 participants
@dckc @0xpatrickdev