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

Introduce DecimalAmount and refactor wallet controller to use it #1434

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

iljakuklic
Copy link
Contributor

@iljakuklic iljakuklic commented Jan 6, 2024

  • Add DecimalAmount which is like Amount with formatting information (number of decimals)
    • it can be printed and parsed to/from strings and supports serde
    • The printing/parsing of Amount was changed to use it under the hood
  • Use that in the wallet controller for querying balances
  • That will enable removing some duplication between the CLI frontend and RPC frontend

@iljakuklic iljakuklic requested a review from OBorce January 6, 2024 00:29
@iljakuklic iljakuklic force-pushed the decimal-amount branch 2 times, most recently from a7ce962 to e20503e Compare January 6, 2024 00:46
It can be used to present amounts to the user since it carries with
itself the number of decimal digits to display. It has the capability to
be displayed to string and parsed from string and has serde support.
Copy link
Contributor

@OBorce OBorce left a comment

Choose a reason for hiding this comment

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

looks good

Comment on lines +96 to +109
let mut tokens = BTreeMap::new();
for (currency, amount) in balances {
let token_id = match currency {
Currency::Coin => panic!("Removed just above"),
Currency::Token(token_id) => token_id,
};

let info = super::fetch_token_info(&self.rpc_client, token_id).await?;
let decimals = info.token_number_of_decimals();
tokens.insert(
token_id,
DecimalAmount::from_amount_minimal(amount, decimals),
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

One other option I used in the read controller for fetching multiple pool infos from the node was to use FuturesUnordered and try_collect().await to await all of them at once instead of one by one.

@iljakuklic iljakuklic merged commit 5695a1a into master Jan 9, 2024
22 checks passed
@iljakuklic iljakuklic deleted the decimal-amount branch January 9, 2024 08:53
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.

3 participants