From 95dfeb3629d84232799b56745f80d7ea93aa98df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Chabowski?= <88321181+rafal-ch@users.noreply.github.com> Date: Thu, 5 Dec 2024 23:58:06 +0100 Subject: [PATCH] Revert balances amount to `U64` and introduce new `amountU128` getter (#2472) ## Description This PR partially reverts the changes introduced in the [balances indexation PR](https://github.com/FuelLabs/fuel-core/pull/2383). In particular: * reverts the type of `amount` in the `Balance` GraphQL type back to `U64` * introduces a new field `amountU128` in the `Balance` GraphQL type * reverts the `FuelClient::balance()` function to return `u64`. There is no `balance_u128()` function added to the `FuelClient` for now. ![image](https://github.com/user-attachments/assets/621db3b9-54f8-46c9-8bdd-c29f65b01c7c) This is done to maintain the full backward compatibility on the GraphQL level, internally the total balance is still processed using `u128`. ### Before requesting review - [X] I have reviewed the code myself --- CHANGELOG.md | 2 +- bin/e2e-test-client/src/test_context.rs | 2 +- crates/client/assets/schema.sdl | 3 ++- crates/client/src/client.rs | 4 ++-- crates/client/src/client/schema/balance.rs | 4 ++-- crates/client/src/client/types/balance.rs | 5 ++++- crates/fuel-core/src/schema/balance.rs | 9 ++++++++- tests/tests/balances.rs | 2 +- tests/tests/chain.rs | 4 ++-- tests/tests/fee_collection_contract.rs | 2 +- 10 files changed, 24 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6dfcb33e77..48b6d73162d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [2327](https://github.com/FuelLabs/fuel-core/pull/2327): Add more services tests and more checks of the pool. Also add an high level documentation for users of the pool and contributors. - [2416](https://github.com/FuelLabs/fuel-core/issues/2416): Define the `GasPriceServiceV1` task. - [2033](https://github.com/FuelLabs/fuel-core/pull/2033): Remove `Option` in favor of `BlockHeightQuery` where applicable. +- [2472](https://github.com/FuelLabs/fuel-core/pull/2472): Added the `amountU128` field to the `Balance` GraphQL schema, providing the total balance as a `U128`. The existing `amount` field clamps any balance exceeding `U64` to `u64::MAX`. ### Fixed - [2365](https://github.com/FuelLabs/fuel-core/pull/2365): Fixed the error during dry run in the case of race condition. @@ -44,7 +45,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). #### Breaking - [2389](https://github.com/FuelLabs/fuel-core/pull/2258): Updated the `messageProof` GraphQL schema to return a non-nullable `MessageProof`. -- [2383](https://github.com/FuelLabs/fuel-core/pull/2383): Asset balance queries now return U128 instead of U64. - [2154](https://github.com/FuelLabs/fuel-core/pull/2154): Transaction graphql endpoints use `TransactionType` instead of `fuel_tx::Transaction`. - [2446](https://github.com/FuelLabs/fuel-core/pull/2446): Use graphiql instead of graphql-playground due to known vulnerability and stale development. - [2379](https://github.com/FuelLabs/fuel-core/issues/2379): Change `kv_store::Value` to be `Arc<[u8]>` instead of `Arc>`. diff --git a/bin/e2e-test-client/src/test_context.rs b/bin/e2e-test-client/src/test_context.rs index d5e98c121d4..1cb6bcb8b07 100644 --- a/bin/e2e-test-client/src/test_context.rs +++ b/bin/e2e-test-client/src/test_context.rs @@ -99,7 +99,7 @@ impl Wallet { } /// returns the balance associated with a wallet - pub async fn balance(&self, asset_id: Option) -> anyhow::Result { + pub async fn balance(&self, asset_id: Option) -> anyhow::Result { self.client .balance(&self.address, Some(&asset_id.unwrap_or_default())) .await diff --git a/crates/client/assets/schema.sdl b/crates/client/assets/schema.sdl index 0cd31fdb34f..1c291a567fd 100644 --- a/crates/client/assets/schema.sdl +++ b/crates/client/assets/schema.sdl @@ -4,7 +4,8 @@ scalar AssetId type Balance { owner: Address! - amount: U128! + amount: U64! + amountU128: U128! assetId: AssetId! } diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index aa41567b384..48973564e17 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -1069,7 +1069,7 @@ impl FuelClient { &self, owner: &Address, asset_id: Option<&AssetId>, - ) -> io::Result { + ) -> io::Result { let owner: schema::Address = (*owner).into(); let asset_id: schema::AssetId = match asset_id { Some(asset_id) => (*asset_id).into(), @@ -1077,7 +1077,7 @@ impl FuelClient { }; let query = schema::balance::BalanceQuery::build(BalanceArgs { owner, asset_id }); let balance: types::Balance = self.query(query).await?.balance.into(); - Ok(balance.amount) + Ok(balance.amount.try_into().unwrap_or(u64::MAX)) } // Retrieve a page of balances by their owner diff --git a/crates/client/src/client/schema/balance.rs b/crates/client/src/client/schema/balance.rs index c20da989f62..89c5dbca32d 100644 --- a/crates/client/src/client/schema/balance.rs +++ b/crates/client/src/client/schema/balance.rs @@ -4,7 +4,7 @@ use crate::client::{ Address, AssetId, PageInfo, - U128, + U64, }, PageDirection, PaginationRequest, @@ -99,7 +99,7 @@ pub struct BalanceEdge { #[cynic(schema_path = "./assets/schema.sdl")] pub struct Balance { pub owner: Address, - pub amount: U128, + pub amount: U64, pub asset_id: AssetId, } diff --git a/crates/client/src/client/types/balance.rs b/crates/client/src/client/types/balance.rs index 3220d9c036c..5afc79a470f 100644 --- a/crates/client/src/client/types/balance.rs +++ b/crates/client/src/client/types/balance.rs @@ -20,7 +20,10 @@ impl From for Balance { fn from(value: schema::balance::Balance) -> Self { Balance { owner: value.owner.into(), - amount: value.amount.into(), + amount: { + let amount: u64 = value.amount.into(); + amount as u128 + }, asset_id: value.asset_id.into(), } } diff --git a/crates/fuel-core/src/schema/balance.rs b/crates/fuel-core/src/schema/balance.rs index b6b95228e83..23392216e73 100644 --- a/crates/fuel-core/src/schema/balance.rs +++ b/crates/fuel-core/src/schema/balance.rs @@ -25,6 +25,8 @@ use async_graphql::{ use fuel_core_types::services::graphql_api; use futures::StreamExt; +use super::scalars::U64; + pub struct Balance(graphql_api::AddressBalance); #[Object] @@ -33,7 +35,12 @@ impl Balance { self.0.owner.into() } - async fn amount(&self) -> U128 { + async fn amount(&self) -> U64 { + let amount: u64 = self.0.amount.try_into().unwrap_or(u64::MAX); + amount.into() + } + + async fn amount_u128(&self) -> U128 { self.0.amount.into() } diff --git a/tests/tests/balances.rs b/tests/tests/balances.rs index 20eb662914c..b2804141295 100644 --- a/tests/tests/balances.rs +++ b/tests/tests/balances.rs @@ -169,7 +169,7 @@ async fn balance_messages_only() { let client = FuelClient::from(srv.bound_address); // run test - const NON_RETRYABLE_AMOUNT: u128 = 60 + 90; + const NON_RETRYABLE_AMOUNT: u64 = 60 + 90; let balance = client.balance(&owner, Some(&asset_id)).await.unwrap(); assert_eq!(balance, NON_RETRYABLE_AMOUNT); } diff --git a/tests/tests/chain.rs b/tests/tests/chain.rs index 43470f7c0d3..c5c62b8f600 100644 --- a/tests/tests/chain.rs +++ b/tests/tests/chain.rs @@ -170,11 +170,11 @@ async fn network_operates_with_non_zero_base_asset_id() { .expect("transaction should insert"); // Then - let expected_fee = 1_u128; + let expected_fee = 1; assert!(matches!(result, TransactionStatus::Success { .. })); let balance = client .balance(&owner, Some(&new_base_asset_id)) .await .expect("Should fetch the balance"); - assert_eq!(balance, amount as u128 - expected_fee); + assert_eq!(balance, amount - expected_fee); } diff --git a/tests/tests/fee_collection_contract.rs b/tests/tests/fee_collection_contract.rs index e449e6fa17b..54426b5c293 100644 --- a/tests/tests/fee_collection_contract.rs +++ b/tests/tests/fee_collection_contract.rs @@ -227,7 +227,7 @@ async fn happy_path() { // Make sure that the full balance was been withdrawn assert_eq!( ctx.client.balance(&ctx.address, None).await.unwrap(), - contract_balance_before_collect as u128 + contract_balance_before_collect ); }