RPC: Take scaling into account for ui amount #4663
Conversation
#### Problem The UI amount reported by RPC does not take into account the recently-added scaled-ui-amount extension from token-2022. #### Summary of changes Very similar to solana-labs#1549, except also including the scaled ui amount. Deprecates structs and functions as appropriate, just like in the previous PR.
|
If this PR represents a change to the public RPC API:
Thank you for keeping the RPC clients in sync with the server API @joncinque. |
| #[test_case(spl_token::id(), None, None; "spl_token")] | ||
| #[test_case(spl_token_2022::id(), Some(InterestBearingConfig { pre_update_average_rate: 500.into(), current_rate: 500.into(),..Default::default() }), None; "spl_token_2022_with _interest")] | ||
| #[test_case(spl_token_2022::id(), None, Some(ScaledUiAmountConfig { new_multiplier: 2.0f64.into(), ..Default::default() }); "spl-token-2022 with multiplier")] | ||
| fn test_token_parsing( |
There was a problem hiding this comment.
It'll be nicer to review this test with "hide whitespace"
joncinque
requested review from
steveluscher
and removed request for
samkim-crypto
|
For the uninitiated (me): solana-labs/solana-program-library#7511 |
| }; | ||
| let token_amount = token_amount_to_ui_amount_v2(u64::MAX, &additional_data); | ||
| let token_amount = token_amount_to_ui_amount_v3(u64::MAX, &additional_data); | ||
| assert_eq!(token_amount.ui_amount, Some(f64::INFINITY)); | ||
| assert_eq!(token_amount.ui_amount_string, "inf"); |
| let mut value = json!({ | ||
| "source": account_keys[instruction.accounts[0] as usize].to_string(), | ||
| "mint": account_keys[instruction.accounts[1] as usize].to_string(), | ||
| "destination": account_keys[instruction.accounts[2] as usize].to_string(), | ||
| "tokenAmount": token_amount_to_ui_amount_v2(amount, &additional_data), | ||
| "tokenAmount": token_amount_to_ui_amount_v3(amount, &additional_data), |
There was a problem hiding this comment.
Presume a token with 2 decimals and a multiplier of 0.99.
I fetch a transaction with jsonParsed and this output results:
{
// ...
"tokenAmount": "0.99" // This PR would make this the post-multiplier value
}Which of these is true?
- This instruction represents a transfer of 1.00 underlying tokens (1.00 * 0.99 = 0.99 truncated to
"0.99"UI amount) - This instruction represents a transfer of 1.01 underlying tokens (1.01 * 0.99 = 0.9999 truncated to
"0.99"UI amount)
Edit: OK right, this is actually a TokenAmount struct, so what you'll see is:
{
// ...
"tokenAmount": {
"amount": "100", // ...or "amount": "101",
"decimals": 2,
"uiAmount": 0.99,
"uiAmountString": "0.99"
}
}Is that right?
There was a problem hiding this comment.
The transaction parsers don't have access to account data, only transaction data. This means that we don't take any scaling into account for past transactions.
But if this were an account that you were fetching, then for an amount of 100 it would give you what you exactly what you put in, and for an amount of 101 it would give you uiAmountString: "1" and uiAmount: 1.0
There was a problem hiding this comment.
…for an amount of 101 it would give you uiAmountString: "1" and uiAmount: 1.0
I don't agree with that, so maybe we have to chat about this. It would compute 101 * 0.99 = 99.99 ergo uiAmountString: "0.99" and uiAmount: 0.99
There was a problem hiding this comment.
There's nothing to agree or disagree about, that's literally what the code produces when you run it 😅
Edit: unless you mean that the behavior should always floor rather than round. We can certainly have a discussion about that. Here's the source code for reference, which uses Rust's float formatting with decimals when outputting the final string: https://github.com/solana-program/token-2022/blob/4fd35b214acfafe21269667009f16aedfad312e4/program/src/extension/scaled_ui_amount/mod.rs#L78
There was a problem hiding this comment.
Quick summary of IRL chat.
- I was under the impression that the truncation strategy for the display number was well defined, and furthermore that it was ‘round toward zero’ as defined by IEEE 754. It is not; the rounding strategy is ‘whatever
formatdoes in Rust,’ which happens to be ‘round to nearest, ties to even’ as defined by IEEE 724. - We agreed that we need to be explicit about the rounding strategy for display, and @joncinque will follow up on that in a subsequent PR. Furthermore, because the number under consideration can represent an asset, the rounding strategy must be ‘round toward zero’ as defined by IEEE 724. The quantity of an asset owned must at no point be overstated.
|
Merging this, will fix the token-2022 issue in https://github.com/solana-program/token-2022 |
Problem
The UI amount reported by RPC does not take into account the recently-added scaled-ui-amount extension from token-2022.
Summary of changes
Very similar to #1549, except also including the scaled ui amount. Deprecates structs and functions as appropriate, just like in the previous PR.