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

add token supply query #17

Closed
wants to merge 1 commit into from
Closed

add token supply query #17

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 17, 2022

Pretty much self explanatory, I need to be able to query for the supply of a native token during testing.

I don't know how to get around the non_exhaustive issue, that's why I've redefined the SupplyResponse struct.

@chipshort
Copy link
Contributor

About that non-exhaustive issue: In newer versions of cosmwasm-std, SupplyResponse implements Default, so you can use that to get one and adjust the amount on that:

let mut res = SupplyResponse::default();
res.amount = amount;

@NoahSaso
Copy link

NoahSaso commented Aug 8, 2023

What is left to fix here? I'm blocked on this, happy to finish up the PR.

Copy link
Contributor

@chipshort chipshort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments here to improve.
Also would like to see some tests

Comment on lines +31 to +40
// TODO: remove this when I figure out how non_exhaustive works
#[cfg(feature = "cosmwasm_1_1")]
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub struct SupplyResponse {
/// Always returns a Coin with the requested denom.
/// This will be of zero amount if the denom does not exist.
pub amount: Coin,
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cosmwasm-std version should be bumped and then this can be removed. Instead the SupplyResponse from cosmwasm-std should be used (using either the Default impl or the recently introduced constructor)

fn get_supply(&self, bank_storage: &dyn Storage, denom: String) -> AnyResult<Coin> {
let supply: Uint128 = BALANCES
.range(bank_storage, None, None, Order::Ascending)
.into_iter()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to call into_iter() here, it's a noop

let supply: Uint128 = BALANCES
.range(bank_storage, None, None, Order::Ascending)
.into_iter()
.map(|a| a.unwrap().1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of this unwrap, the error should be bubbled up

@ghost ghost closed this by deleting the head repository Aug 15, 2023
JakeHartnell pushed a commit to JakeHartnell/cw-multi-test that referenced this pull request Aug 17, 2023
JakeHartnell pushed a commit to JakeHartnell/cw-multi-test that referenced this pull request Aug 17, 2023
This pull request was closed.
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.

2 participants