-
Notifications
You must be signed in to change notification settings - Fork 121
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
feat: fetching known ibc channels from github chain-registry repository #1545
Conversation
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.
Nice! Very good!
@@ -3,3 +3,6 @@ | |||
#rpc_url = "" | |||
#masp_indexer_url = "" | |||
#localnet_enabled = false | |||
|
|||
github_chain_registry_base_url = "https://raw.githubusercontent.com/anoma/namada-chain-registry/refs/heads/main" | |||
github_namada_interface_url = "https://raw.githubusercontent.com/anoma/namada-interface/refs/heads/main" |
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.
I didn't understand the use case of this 🤔
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.
Just in case someone would want to point to their own chain registry for some reason. Probably won't be used
); | ||
] as Chain[]; | ||
|
||
cosmosRegistry.chains.push(...namadaTestnetChainList, namadaChain); |
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.
I vaguely remember having a find()
in the code that captures the first item on array, and it needs to be the mainnet, maybe an asset related?
Do you think we could invert parameters to be .push(namadaChain, ...namadaTestnetChainList)
?
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.
Hmmm, I can do it, but this find()
is not right :/ if we find this case, we should refactor
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.
LGTM!
Tries to fetch known IBC channels from namada-chain-registry repository, instead of relying on the ibc channels added to Namadillo build. Maybe in the future we can revert these changes, but now we'll need a more dynamic way to fetch the latest changes and info from relayers.
This PR also introduce new config.toml entries:
github_chain_registry_base_url = "https://raw.githubusercontent.com/anoma/namada-chain-registry/refs/heads/main"
github_namada_interface_url = "https://raw.githubusercontent.com/anoma/namada-interface/refs/heads/main"