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.

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

Added osmosis #559

Merged
merged 16 commits into from
Nov 16, 2022
Merged

Added osmosis #559

merged 16 commits into from
Nov 16, 2022

Conversation

tonymorony
Copy link

@tonymorony tonymorony commented Nov 12, 2022

image
image

coins Show resolved Hide resolved
@yurii-khi
Copy link

@tonymorony I took the liberty of adding api_ids and explorer url data, hope you don't mind.

@smk762 Please check out f2ea188 . The goal was to have "type": "TENDERMINT" for OSMO in coins_config.json

@KomodoPlatform KomodoPlatform deleted a comment from smk762 Nov 12, 2022
@smk762
Copy link
Collaborator

smk762 commented Nov 12, 2022

@tonymorony I took the liberty of adding api_ids and explorer url data, hope you don't mind.

@smk762 Please check out f2ea188 . The goal was to have "type": "TENDERMINT" for OSMO in coins_config.json

The generator should apply type to each coin in output as it is processed, though multiple "parent coins" with same coin type was not considered with initial script. I assume in the current script IRIS should be TENDERMINT not TENDERMINTTOKEN
https://github.com/KomodoPlatform/coins/blob/add_osmosis/utils/generate_app_configs.py#L83 also.

If this is working for your purposes I am happy to approve, but Id like to revisit the script to make sure there are no unintended consequences from the duplication at a glance it looks ok).

api_ids/nomics_ids.json Outdated Show resolved Hide resolved
@smk762
Copy link
Collaborator

smk762 commented Nov 14, 2022

@tonymorony I took the liberty of adding api_ids and explorer url data, hope you don't mind.

@smk762 Please check out f2ea188 . The goal was to have "type": "TENDERMINT" for OSMO in coins_config.json

Is there any requirement to delineate between TENDERMINT and TENDERMINTOKEN?
We dont do this for any other protocol - I'd prefer to tag all as "TENDERMINT"

@tonymorony
Copy link
Author

Is there any requirement to delineate between TENDERMINT and TENDERMINTOKEN?
We dont do this for any other protocol - I'd prefer to tag all as "TENDERMINT"

We do have protocol type for all protocols, TENDERMINT is type for "parental" tendermint assets (it's not a same thing as a token, like a eth and erc20)

    "protocol": {
      "type": "ERC20",
      "protocol_data": {
        "platform": "AVAX"

e.g. for ERC20 on ethereum

for TENDERMINTTOKEN I think it's a bit confusing now since platform flag is missing (and potentially tenderminttokens might be not only on IRIS), imo it should be like a :

    "protocol": {
      "type": "TENDERMINTTOKEN",
      "protocol_data": {
        "platform": "IRIS"

@tonymorony
Copy link
Author

Ah, nvm - for IBC tokens it's already like this so everything is fine:

    "protocol": {
      "type": "TENDERMINTTOKEN",
      "protocol_data": {
        "platform": "IRIS",

@smk762
Copy link
Collaborator

smk762 commented Nov 14, 2022

Just to confirm:

@tonymorony
Copy link
Author

IRIS, OSMO, ATOM -> "type": "TENDERMINT",
ATOM-IBC_IRIS -> "type": "TENDERMINTTOKEN",

yes, it's correct. Native blockchain assets marked as TENDERMINT, token on one of these blockchains (on IRIS) marked as TENDERMINTTOKEN

IRIS, OSMO and ATOM it's 3 different blockchains

so everything is quite consistent atm

@smk762
Copy link
Collaborator

smk762 commented Nov 14, 2022

IRIS, OSMO, ATOM -> "type": "TENDERMINT",
ATOM-IBC_IRIS -> "type": "TENDERMINTTOKEN",

yes, it's correct. Native blockchain assets marked as TENDERMINT, token on one of these blockchains (on IRIS) marked as TENDERMINTTOKEN

IRIS, OSMO and ATOM it's 3 different blockchains

so everything is quite consistent atm

ok - looks like config gen needs a fix, that is where i saw the inconsistency.
https://github.com/KomodoPlatform/coins/blob/add_osmosis/utils/coins_config.json#L24275-L24277

@smk762
Copy link
Collaborator

smk762 commented Nov 14, 2022

I've update so generated output will no longer incorrectly tag IRIS as TENDERMINTTOKEN
Output now also includes parent_coin field where applicable for future convenience when we add tendermint tokens under IRIS/OSMO etc.
I've also renamed the ethereum/COSMOS file to ethereum/ATOM as all others in this folder use ticker, not name.

@smk762
Copy link
Collaborator

smk762 commented Nov 14, 2022

I noticed the urls for tendermint not working as expected in desktop, I've have added the req path suffixes here.

@tonymorony tonymorony merged commit da5a6d6 into master Nov 16, 2022
@cipig cipig deleted the add_osmosis branch June 27, 2023 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants