Skip to content

Commit

Permalink
Revert balances amount to U64 and introduce new amountU128 getter (
Browse files Browse the repository at this point in the history
…#2472)

## Description
This PR partially reverts the changes introduced in the [balances
indexation PR](#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
  • Loading branch information
rafal-ch authored Dec 5, 2024
1 parent 791b79c commit 95dfeb3
Show file tree
Hide file tree
Showing 10 changed files with 24 additions and 13 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<BlockHeight>` 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.
Expand All @@ -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<Vec<u8>>`.
Expand Down
2 changes: 1 addition & 1 deletion bin/e2e-test-client/src/test_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl Wallet {
}

/// returns the balance associated with a wallet
pub async fn balance(&self, asset_id: Option<AssetId>) -> anyhow::Result<u128> {
pub async fn balance(&self, asset_id: Option<AssetId>) -> anyhow::Result<u64> {
self.client
.balance(&self.address, Some(&asset_id.unwrap_or_default()))
.await
Expand Down
3 changes: 2 additions & 1 deletion crates/client/assets/schema.sdl
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ scalar AssetId

type Balance {
owner: Address!
amount: U128!
amount: U64!
amountU128: U128!
assetId: AssetId!
}

Expand Down
4 changes: 2 additions & 2 deletions crates/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1069,15 +1069,15 @@ impl FuelClient {
&self,
owner: &Address,
asset_id: Option<&AssetId>,
) -> io::Result<u128> {
) -> io::Result<u64> {
let owner: schema::Address = (*owner).into();
let asset_id: schema::AssetId = match asset_id {
Some(asset_id) => (*asset_id).into(),
None => schema::AssetId::default(),
};
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
Expand Down
4 changes: 2 additions & 2 deletions crates/client/src/client/schema/balance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::client::{
Address,
AssetId,
PageInfo,
U128,
U64,
},
PageDirection,
PaginationRequest,
Expand Down Expand Up @@ -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,
}

Expand Down
5 changes: 4 additions & 1 deletion crates/client/src/client/types/balance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ impl From<schema::balance::Balance> 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(),
}
}
Expand Down
9 changes: 8 additions & 1 deletion crates/fuel-core/src/schema/balance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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()
}

Expand Down
2 changes: 1 addition & 1 deletion tests/tests/balances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 1 addition & 1 deletion tests/tests/fee_collection_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}

Expand Down

0 comments on commit 95dfeb3

Please sign in to comment.