-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add neutron-carbon ibc #5887
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
base: master
Are you sure you want to change the base?
Add neutron-carbon ibc #5887
Conversation
@JeremyParish69 hi! may i seek your assistance to look into this PR! |
* Update carbon assetlist and chain config * Add and update token assets * Update IBC connections to carbon * Make some manual corrections to config files
Hi @JeremyParish69, I've resolved your comment alr. In addition, I accidentally merged an extra commit with several more changes to our master branch. Please let us know if we should close this PR and create a new one to better reflect the changes. Apologies for the inconvenience! 🙏 |
Fix/reviewer comments
Hi @JeremyParish69, I've just addressed your comments and the PR is ready for another review. Could you review this pull request when you're next able to? Thank you! 🙏 |
carbon/assetlist.json
Outdated
}, | ||
"provider": "PolyNetwork" | ||
"provider": "Axelar Bridge" |
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'm suspicious of this actually being via Axelar. Their API shows a different base denom (as ibc/...) for these assets, not starting with (brdg/). Are we sure that isn't a typo?
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.
this is a asset starting with brdg
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.
Ok, but why does it say Axelar here?
carbon/assetlist.json
Outdated
] | ||
} | ||
], | ||
"base": "bnb.1.6.773edb", |
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.
It looks like you may be removing this base_denom? Instead of removing, please mark as "deprecated": true
. you may update the name of symbol to avoid confusion with the replacement BNB). Same idea applies to all assets being removed
carbon/assetlist.json
Outdated
}, | ||
"provider": "PolyNetwork" | ||
"provider": "Tether" |
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.
It doesn't appear that Tether is issuing USDt here. Are there any docs from them that show this as one of their issuances?
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.
This synthetic asset comprises of usdt bridged in from other chains via axelar. These assets are those that are listed on https://tether.to/en/supported-protocols. Would there be a more appropriate provider that i could change to?
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.
The Provider should be the team that creates the synthetic token itself--not who issued the token it follows, nor who provides the backing/collateral. Who is it that is creating this synthetic token? Is it Carbon or Demex?
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.
got it. i amended all synthetic tokens to carbon as the provider as it is created by carbon.
carbon/assetlist.json
Outdated
"chain_name": "binancesmartchain", | ||
"base_denom": "0xe9e7cea3dedca5984780bafc599bd69add087d56" | ||
}, | ||
"provider": "Axelar Bridge" |
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.
Still suspicious of this provider...
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.
this should be Binance taking reference from their assetlist.
@@ -61,18 +94,18 @@ | |||
} | |||
}, | |||
{ | |||
"denom": "busd.1.6.754a80", | |||
"denom": "brdg/1768794901f8a19c2ec795a5402653cef6cbfe6b3ec6398d39fc37de963cb667", |
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.
From verification script output:
Error: Chain carbon does not have fee token brdg/1768794901f8a19c2ec795a5402653cef6cbfe6b3ec6398d39fc37de963cb667 defined in its Assetlist.
No description provided.