-
Notifications
You must be signed in to change notification settings - Fork 220
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
CosmosOrchestrationAccount
's amountToCoin
returns incorrect denom
when presented a Brand
#10449
Comments
CosmosOrchestrationAccount
's amountToCoin
does not account for CosmosOrchestrationAccount
's amountToCoin
returns incorrect denom
when presented a Brand
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send` - filed Agoric#10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send` - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send` - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send` - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send` - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send` - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send` - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
Thinking about the impact of this bug... it impacts all APIs that call A quick audit shows lots of high-profile methods:
cc @mitdralla |
The impact is that users cannot present IST as a brand and have it mean I'm wondering if #10588 indirectly addresses the issue. Namely: This would indicate this is not the expected behavior: // expected
helper.amountToCoin(ISTbrand, holdingChain) => ({ denom: 'ibc/uisthash', amount: 1n }) Instead, it doesn't seem we have a good story about brands for |
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send`. marked as failing for now - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send`. marked as failing for now - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send`. marked as failing for now - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send`. marked as failing for now - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send`. marked as failing for now - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
this test can no longer rely on the IST brand being available, so we - added checks to ensure brands are accepted for `.transfer` and `.send`. marked as failing for now - filed #10449 since this surfaced a bug in `amountToCoin` - use Moolah issuer for "no denom for brand" failure path tests
Describe the bug
CosmosOrchestrationAccount
'samountToCoin
/coerceCoin
/coerceDenom
helpers do not account for issuing/host chain logic.Example:
Most tests use
DenomArg
instead ofAmountArg
, as we are waiting on #9752, #9966, #9967, so this path wasn't well trodden.To Reproduce
/packages/orchestration/test/exos/cosmos-orchestration-account.test.ts
has a test that mentions thisExpected behavior
The helper should return the denom from the perspective of the holding chain. IOW, the host chain for the ICA.
The text was updated successfully, but these errors were encountered: