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

bidirectional lookup of denoms / brands for IBC #9211

Closed
Tracked by #8863
turadg opened this issue Apr 9, 2024 · 4 comments · Fixed by #9754 or #9983
Closed
Tracked by #8863

bidirectional lookup of denoms / brands for IBC #9211

turadg opened this issue Apr 9, 2024 · 4 comments · Fixed by #9754 or #9983
Assignees
@turadg
Labels
enhancement New feature or request

Comments

@turadg
Copy link
Member

turadg commented Apr 9, 2024 •
edited
Loading

What is the Problem Being Solved?

#8879 manages IBC paths but developers need a way to establish a correspondence between a denom and its issuing chain, and optionally an ERTP Brand.

Description of the Design

  • Treat channel info as immutable
  • We provide a lookup service to get the info for a particular chain
  • The set is append-only (the value for a particular chain name is immutable)
  • The right to append is held only by BLD Stakers
  • If a developer needs the info before it’s on-chain (between staker votes) they look it up through some other trusted source (e.g. Google for the chain’s docs)

With the above requirements we landed on each contract having a ChainHub. That can load from agoricNames and/or be filled manually by contract code.

Security Considerations

Scaling Considerations

Test Plan

getBrandInfo works with ChainHub (done in #9754)

All exos look up denoms using ChainHub

Not in scope: filling ChainHub using agoricNames, which is

Upgrade Considerations

not yet deployed but needs an upgrade test
@turadg turadg added the enhancement New feature or request label Apr 9, 2024
@turadg turadg self-assigned this Apr 9, 2024
@dckc
Copy link
Member

dckc commented Apr 12, 2024

some recent relevant work: agoric-labs/ag-power-tools#8 (comment)

see also:

@dckc
Copy link
Member

dckc commented May 3, 2024

btw "petname" is a bit of a stretch here. It suggests that each person can have their own name for the tokens and that the names can be changed. I suppose those are true from the perspective of the BLD stakers as one aggregate "person", but... it's a stretch.

@dckc
Copy link
Member

dckc commented May 3, 2024

"well known name handling" might be better

@dckc dckc mentioned this issue May 3, 2024
Closed
@turadg turadg changed the title pet name handling for IBC denoms well known name handling for IBC denoms May 5, 2024
0xpatrickdev added a commit that referenced this issue May 6, 2024
@0xpatrickdev
test(orchestration): chain NameHub
9aff733 
- refs #9211
This was referenced May 6, 2024
Closed
Merged
Closed
@dckc dckc mentioned this issue May 23, 2024
Merged
@aj-agoric aj-agoric assigned dckc and unassigned turadg May 28, 2024
@dckc dckc mentioned this issue Jun 12, 2024
Closed
1 task
@dckc dckc mentioned this issue Jun 24, 2024
Merged
mergify bot added a commit that referenced this issue Jul 12, 2024
@mergify
feat: vat-safe denomHash using noble sha256 (#9576)
24528f1 
refs: #9211

## Description

Prototyping for #9211 suggests we'll want to be [derive IBC denoms](https://tutorials.cosmos.network/tutorials/6-ibc-dev/#how-are-ibc-denoms-derived) using a hash of a path such as `transfer/channel-0/uatom` -> `ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2`.

I tried to use the sha256 function from `@cosmjs/crypto`, but I got error's about node's `crypto` module. `@cosmjs/crypto` imports an old version of `@noble/hashes`. The current version seems to be vat-safe.

### Scaling Considerations

Compute performance: Doing a sha256 computation on XS in pure JS seems a little whacky. We could consider dropping down to C like we do for base64. But since this is a constant time operation, it doesn't seem worthwhile.

Bundle size: A base64 encoded bundle with just this module seems to be around 500k.

```
┌──────────────────────────────┬────────┐
│           (index)            │ Values │
├──────────────────────────────┼────────┤
│ @agoric/orchestration-v0.1.0 │  1702  │
│     @noble/hashes-v1.4.0     │ 18820  │
└──────────────────────────────┴────────┘
total size: 36623
```

### Security Considerations

supply chain: this adds a dependency on the current [@noble/hashes](https://www.npmjs.com/package/@noble/hashes), which is reasonably popular (3m weekly downloads) and seems to be well audited.

### Documentation Considerations

I'm not sure where this shows up in the orchestration API yet.

### Testing Considerations

A test that it works in a compartment is included.

### Upgrade Considerations

Unrelated to any deployed code.
@0xpatrickdev 0xpatrickdev mentioned this issue Jul 15, 2024
Open
@turadg turadg mentioned this issue Jul 22, 2024
Open
This was referenced Jul 25, 2024
Closed
Merged
@mergify mergify bot closed this as completed in #9754 Jul 26, 2024
mergify bot added a commit that referenced this issue Jul 26, 2024
@mergify
feat: getBrandInfo using ChainHub (#9754)
77fcd1a 
closes #9211

## Description

  - feat: chainHub.registerAsset / lookupAsset
  - feat: Orchestrator.getBrandInfo
    - getChain() is now backed by a store
  - groundwork for asset info in agoricNames  - #9752

### Testing Considerations

unit tests cover happy path.

Low code-complexity doesn't seem to merit more.

### Security / Scaling Considerations

none, AFAICT

### Documentation Considerations

make `getBrandInfo()` work as documented, provided assets are registered with the ChainHub

### Upgrade Considerations

not yet deployed
@dckc dckc mentioned this issue Jul 31, 2024
Draft
3 tasks
0xpatrickdev added a commit that referenced this issue Aug 9, 2024
@0xpatrickdev
feat: amountToCoin accepts any denom
bfe4cb1 
- still does not support brand lookups (see #9211)
- throws error if a brand is provided instead of warning in the console
- moves bondDenom check to delegate method to preserve functionality
- also includes this helper on LocalOrchestrationAccount
0xpatrickdev added a commit that referenced this issue Aug 9, 2024
@0xpatrickdev
feat: amountToCoin accepts any denom
a3ffe1e 
- still does not support brand lookups (see #9211)
- throws error if a brand is provided instead of warning in the console
- moves bondDenom check to delegate method to preserve functionality
- also includes this helper on LocalOrchestrationAccount
0xpatrickdev added a commit that referenced this issue Aug 9, 2024
@0xpatrickdev
feat: amountToCoin accepts any denom
99b1704 
- still does not support brand lookups (see #9211)
- throws error if a brand is provided instead of warning in the console
- moves bondDenom check to delegate method to preserve functionality
- also includes this helper on LocalOrchestrationAccount
@turadg
Copy link
Member Author

turadg commented Aug 26, 2024

Reopening until the "#9211" comments (8 files) are resolved.

@turadg turadg reopened this Aug 26, 2024
kriskowal pushed a commit that referenced this issue Aug 27, 2024
@0xpatrickdev @kriskowal
feat: amountToCoin accepts any denom
554edbd 
- still does not support brand lookups (see #9211)
- throws error if a brand is provided instead of warning in the console
- moves bondDenom check to delegate method to preserve functionality
- also includes this helper on LocalOrchestrationAccount
@turadg turadg mentioned this issue Aug 27, 2024
Merged
@turadg turadg changed the title well known name handling for IBC denoms bidirectional lookup of denoms / brands for IBC Aug 28, 2024
@turadg turadg assigned turadg and unassigned dckc Aug 28, 2024
@mergify mergify bot closed this as completed in #9983 Aug 30, 2024
@mergify mergify bot closed this as completed in 42d1d4f Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@turadg turadg

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: getBrandInfo using ChainHub Agoric/agoric-sdk
9211 lookup denom Agoric/agoric-sdk
2 participants
@turadg @dckc