From 8b396f740b96809347e3b0163055a999d0ba3fc5 Mon Sep 17 00:00:00 2001 From: John Cub Date: Wed, 23 Mar 2022 23:01:57 +0300 Subject: [PATCH 1/6] feat: add `excluded_ids` argument to `Query.coinsToSpend` --- fuel-client/assets/schema.sdl | 2 +- fuel-client/src/client.rs | 16 +- fuel-client/src/client/schema/coin.rs | 27 +++- fuel-client/src/client/schema/primitives.rs | 7 + fuel-core/src/coin_query.rs | 170 ++++++++++++++++---- fuel-core/src/schema/coin.rs | 7 +- fuel-tests/tests/coin.rs | 25 ++- 7 files changed, 217 insertions(+), 37 deletions(-) diff --git a/fuel-client/assets/schema.sdl b/fuel-client/assets/schema.sdl index 0fadbacf12d..e1c9df5c4c1 100644 --- a/fuel-client/assets/schema.sdl +++ b/fuel-client/assets/schema.sdl @@ -194,7 +194,7 @@ type Query { health: Boolean! coin(utxoId: UtxoId!): Coin coins(filter: CoinFilterInput!, first: Int, after: String, last: Int, before: String): CoinConnection! - coinsToSpend(owner: Address!, spendQuery: [SpendQueryElementInput!]!, maxInputs: Int): [Coin!]! + coinsToSpend(owner: Address!, spendQuery: [SpendQueryElementInput!]!, maxInputs: Int, excludedIds: [UtxoId!]): [Coin!]! contract(id: ContractId!): Contract } type Receipt { diff --git a/fuel-client/src/client.rs b/fuel-client/src/client.rs index 646e6337988..43150ea4267 100644 --- a/fuel-client/src/client.rs +++ b/fuel-client/src/client.rs @@ -271,6 +271,7 @@ impl FuelClient { owner: &str, spend_query: Vec<(&str, u64)>, max_inputs: Option, + excluded_ids: Option>, ) -> io::Result> { let owner: schema::Address = owner.parse()?; let spend_query: Vec = spend_query @@ -282,8 +283,19 @@ impl FuelClient { }) }) .try_collect()?; - let query = - schema::coin::CoinsToSpendQuery::build(&(owner, spend_query, max_inputs).into()); + let excluded_ids: Option> = excluded_ids + .map(|ids| { + ids.iter() + .map( + #[allow(clippy::needless_question_mark)] + |id| -> Result<_, ConversionError> { Ok(id.parse()?) }, + ) + .try_collect() + }) + .transpose()?; + let query = schema::coin::CoinsToSpendQuery::build( + &(owner, spend_query, max_inputs, excluded_ids).into(), + ); let coins = self.query(query).await?.coins_to_spend; Ok(coins) diff --git a/fuel-client/src/client/schema/coin.rs b/fuel-client/src/client/schema/coin.rs index a8dd99c5070..1f657dc10dc 100644 --- a/fuel-client/src/client/schema/coin.rs +++ b/fuel-client/src/client/schema/coin.rs @@ -124,14 +124,31 @@ pub struct CoinsToSpendArgs { spend_query: Vec, /// The max number of utxos that can be used max_inputs: Option, -} - -impl From<(Address, Vec, Option)> for CoinsToSpendArgs { - fn from(r: (Address, Vec, Option)) -> Self { + /// A list of UtxoIds to exlude from the selection + excluded_ids: Option>, +} + +impl + From<( + Address, + Vec, + Option, + Option>, + )> for CoinsToSpendArgs +{ + fn from( + r: ( + Address, + Vec, + Option, + Option>, + ), + ) -> Self { CoinsToSpendArgs { owner: r.0, spend_query: r.1, max_inputs: r.2, + excluded_ids: r.3, } } } @@ -143,7 +160,7 @@ impl From<(Address, Vec, Option)> for CoinsToSpendA argument_struct = "CoinsToSpendArgs" )] pub struct CoinsToSpendQuery { - #[arguments(owner = &args.owner, spend_query = &args.spend_query, max_inputs = &args.max_inputs)] + #[arguments(owner = &args.owner, spend_query = &args.spend_query, max_inputs = &args.max_inputs, excluded_ids = &args.excluded_ids)] pub coins_to_spend: Vec, } diff --git a/fuel-client/src/client/schema/primitives.rs b/fuel-client/src/client/schema/primitives.rs index 65df7a1710c..1bb07f6b575 100644 --- a/fuel-client/src/client/schema/primitives.rs +++ b/fuel-client/src/client/schema/primitives.rs @@ -1,6 +1,7 @@ use super::schema; use crate::client::schema::ConversionError; use crate::client::schema::ConversionError::HexStringPrefixError; +use core::fmt; use cynic::impl_scalar; use serde::de::Error; use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -110,6 +111,12 @@ impl From for fuel_tx::UtxoId { } } +impl LowerHex for UtxoId { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + LowerHex::fmt(&self.0 .0, f) + } +} + #[derive(cynic::Scalar, Debug, Clone)] pub struct HexString(pub Bytes); diff --git a/fuel-core/src/coin_query.rs b/fuel-core/src/coin_query.rs index 1f03f708f33..c94df95ea8d 100644 --- a/fuel-core/src/coin_query.rs +++ b/fuel-core/src/coin_query.rs @@ -39,6 +39,7 @@ pub fn largest_first( db: &Database, spend_query: &SpendQuery, max_inputs: u8, + excluded_ids: Option<&Vec>, ) -> Result, CoinQueryError> { // Merge elements with the same (owner, asset_id) let spend_query: Vec = spend_query @@ -59,10 +60,19 @@ pub fn largest_first( for (owner, asset_id, amount) in spend_query { let coins_of_asset_id: Vec<(UtxoId, Coin)> = { - let coin_ids: Vec = db + let mut coin_ids: Vec = db .owned_coins_by_asset_id(owner, asset_id, None, None) .try_collect()?; + // Filter excluded coins + coin_ids.retain(|&id| { + if let Some(excluded_ids) = excluded_ids { + !excluded_ids.contains(&id) + } else { + true + } + }); + let mut coins: Vec<(UtxoId, Coin)> = coin_ids .into_iter() .map(|id| { @@ -110,6 +120,7 @@ pub fn random_improve( db: &Database, spend_query: &SpendQuery, max_inputs: u8, + excluded_ids: Option<&Vec>, ) -> Result, CoinQueryError> { // Merge elements with the same (owner, asset_id) let spend_query: Vec = spend_query @@ -131,10 +142,19 @@ pub fn random_improve( let mut coins_by_asset_id: Vec> = spend_query .iter() .map(|(owner, asset_id, _)| -> Result<_, CoinQueryError> { - let coin_ids: Vec = db + let mut coin_ids: Vec = db .owned_coins_by_asset_id(*owner, *asset_id, None, None) .try_collect()?; + // Filter excluded coins + coin_ids.retain(|&id| { + if let Some(excluded_ids) = excluded_ids { + !excluded_ids.contains(&id) + } else { + true + } + }); + let coins: Vec<(UtxoId, Coin)> = coin_ids .into_iter() .map(|id| { @@ -164,7 +184,7 @@ pub fn random_improve( // Fallback to largest_first if we can't fit more coins if coins.len() >= max_inputs as usize { - return largest_first(db, &spend_query, max_inputs); + return largest_first(db, &spend_query, max_inputs, excluded_ids); } // Error if we don't have more coins @@ -247,6 +267,8 @@ pub fn random_improve( #[cfg(test)] mod tests { + use std::sync::atomic::{AtomicU8, Ordering}; + use assert_matches::assert_matches; use fuel_asm::Word; use fuel_tx::{Address, Bytes32}; @@ -255,29 +277,26 @@ mod tests { use super::*; - fn gen_test_db(owner: Address, asset_ids: &[AssetId]) -> Database { + static COIN_INDEX: AtomicU8 = AtomicU8::new(0); + fn make_coin(owner: Address, amount: Word, asset_id: AssetId) -> (UtxoId, Coin) { + let index = COIN_INDEX.fetch_add(1, Ordering::SeqCst); + let utxo_id = UtxoId::new(Bytes32::from([0u8; 32]), index); + let coin = Coin { + owner, + amount, + asset_id, + maturity: Default::default(), + status: CoinStatus::Unspent, + block_created: Default::default(), + }; + (utxo_id, coin) + } + + fn gen_test_db(coins: &[(UtxoId, Coin)]) -> Database { let mut db = Database::default(); - let coins: Vec<(UtxoId, Coin)> = asset_ids - .iter() - .flat_map(|asset_id| { - (0..5usize).map(move |i| Coin { - owner, - amount: (i + 1) as Word, - asset_id: *asset_id, - maturity: Default::default(), - status: CoinStatus::Unspent, - block_created: Default::default(), - }) - }) - .enumerate() - .map(|(i, coin)| { - let utxo_id = UtxoId::new(Bytes32::from([0u8; 32]), i as u8); - (utxo_id, coin) - }) - .collect(); for (id, coin) in coins { - Storage::::insert(&mut db, &id, &coin).unwrap(); + Storage::::insert(&mut db, id, coin).unwrap(); } db @@ -288,11 +307,19 @@ mod tests { // Setup let owner = Address::default(); let asset_ids = [AssetId::new([1u8; 32]), AssetId::new([2u8; 32])]; - let db = gen_test_db(owner, &asset_ids); + let coins: Vec<(UtxoId, Coin)> = (0..5usize) + .flat_map(|i| { + [ + make_coin(owner, (i + 1) as Word, asset_ids[0]), + make_coin(owner, (i + 1) as Word, asset_ids[1]), + ] + }) + .collect(); + let db = gen_test_db(&coins); let query = |spend_query: &[SpendQueryElement], max_inputs: u8| -> Result, CoinQueryError> { - let coins = largest_first(&db, spend_query, max_inputs); + let coins = largest_first(&db, spend_query, max_inputs, None); // Transform result for convenience coins.map(|coins| { @@ -362,11 +389,19 @@ mod tests { // Setup let owner = Address::default(); let asset_ids = [AssetId::new([1u8; 32]), AssetId::new([2u8; 32])]; - let db = gen_test_db(owner, &asset_ids); + let coins: Vec<(UtxoId, Coin)> = (0..5usize) + .flat_map(|i| { + [ + make_coin(owner, (i + 1) as Word, asset_ids[0]), + make_coin(owner, (i + 1) as Word, asset_ids[1]), + ] + }) + .collect(); + let db = gen_test_db(&coins); let query = |spend_query: &[SpendQueryElement], max_inputs: u8| -> Result, CoinQueryError> { - let coins = random_improve(&db, spend_query, max_inputs); + let coins = random_improve(&db, spend_query, max_inputs, None); // Transform result for convenience coins.map(|coins| { @@ -454,4 +489,85 @@ mod tests { let coins = query(&[(owner, asset_ids[0], 6)], 1); assert_matches!(coins, Err(CoinQueryError::NotEnoughInputs)); } + + #[test] + fn exclusion() { + // Setup + let owner = Address::default(); + let asset_ids = [AssetId::new([1u8; 32]), AssetId::new([2u8; 32])]; + let coins: Vec<(UtxoId, Coin)> = (0..5usize) + .flat_map(|i| { + [ + make_coin(owner, (i + 1) as Word, asset_ids[0]), + make_coin(owner, (i + 1) as Word, asset_ids[1]), + ] + }) + .collect(); + let db = gen_test_db(&coins); + let query = |spend_query: &[SpendQueryElement], + max_inputs: u8, + excluded_ids: Option<&Vec>| + -> Result, CoinQueryError> { + let coins = random_improve(&db, spend_query, max_inputs, excluded_ids); + + // Transform result for convenience + coins.map(|coins| { + coins + .into_iter() + .map(|coin| (coin.1.asset_id, coin.1.amount)) + .sorted_by_key(|(asset_id, amount)| { + ( + asset_ids.iter().position(|c| c == asset_id).unwrap(), + Reverse(*amount), + ) + }) + .collect() + }) + }; + + // Exclude largest coin IDs + let excluded_ids = coins + .into_iter() + .filter(|(_, coin)| coin.amount == 5) + .map(|(utxo_id, _)| utxo_id) + .collect(); + + // Query some amounts, including higher than the owner's balance + for amount in 0..20 { + let coins = query( + &[(owner, asset_ids[0], amount)], + u8::MAX, + Some(&excluded_ids), + ); + + // Transform result for convenience + let coins = coins.map(|coins| { + coins + .into_iter() + .map(|(asset_id, amount)| { + // Check the asset ID before we drop it + assert_eq!(asset_id, asset_ids[0]); + + amount + }) + .collect::>() + }); + + match amount { + // This should return nothing + 0 => assert_matches!(coins, Ok(coins) if coins.is_empty()), + // This range should... + 1..=4 => { + // ...satisfy the amount + assert_matches!(coins, Ok(coins) if coins.iter().sum::() >= amount) + // ...and add more for dust management + // TODO: Implement the test + } + // This range should return all coins + 5..=10 => assert_matches!(coins, Ok(coins) if coins == vec![ 4, 3, 2, 1]), + // Asking for more than the owner's balance should error + _ => assert_matches!(coins, Err(CoinQueryError::NotEnoughCoins)), + }; + } + } } diff --git a/fuel-core/src/schema/coin.rs b/fuel-core/src/schema/coin.rs index 6d810c2a08c..58564a2e557 100644 --- a/fuel-core/src/schema/coin.rs +++ b/fuel-core/src/schema/coin.rs @@ -184,6 +184,9 @@ impl CoinQuery { SpendQueryElementInput, >, #[graphql(desc = "The max number of utxos that can be used")] max_inputs: Option, + #[graphql(desc = "The max number of utxos that can be used")] excluded_ids: Option< + Vec, + >, ) -> async_graphql::Result> { let owner: fuel_tx::Address = owner.0; let spend_query: Vec = spend_query @@ -191,10 +194,12 @@ impl CoinQuery { .map(|e| (owner, e.asset_id.0, e.amount.0)) .collect(); let max_inputs: u8 = max_inputs.unwrap_or(MAX_INPUTS); + let excluded_ids: Option> = + excluded_ids.map(|ids| ids.into_iter().map(|id| id.0).collect()); let db = ctx.data_unchecked::(); - let coins = random_improve(db, &spend_query, max_inputs)? + let coins = random_improve(db, &spend_query, max_inputs, excluded_ids.as_ref())? .into_iter() .map(|(id, coin)| Coin(id, coin)) .collect(); diff --git a/fuel-tests/tests/coin.rs b/fuel-tests/tests/coin.rs index 571ddb5b2d1..f2db2ec992a 100644 --- a/fuel-tests/tests/coin.rs +++ b/fuel-tests/tests/coin.rs @@ -241,7 +241,7 @@ async fn coins_to_spend() { // empty spend_query let coins = client - .coins_to_spend(format!("{:#x}", owner).as_str(), vec![], None) + .coins_to_spend(format!("{:#x}", owner).as_str(), vec![], None, None) .await .unwrap(); assert!(coins.is_empty()); @@ -255,6 +255,7 @@ async fn coins_to_spend() { (format!("{:#x}", asset_id_b).as_str(), 1), ], None, + None, ) .await .unwrap(); @@ -269,11 +270,31 @@ async fn coins_to_spend() { (format!("{:#x}", asset_id_b).as_str(), 300), ], None, + None, ) .await .unwrap(); assert_eq!(coins.len(), 6); + // spend_query for 1 a and 1 b, but with all coins excluded + let all_coin_ids = coins + .iter() + .map(|c| format!("{:#x}", c.utxo_id)) + .collect::>(); + let all_coin_ids = all_coin_ids.iter().map(String::as_str).collect(); + let coins = client + .coins_to_spend( + format!("{:#x}", owner).as_str(), + vec![ + (format!("{:#x}", asset_id_a).as_str(), 1), + (format!("{:#x}", asset_id_b).as_str(), 1), + ], + None, + Some(all_coin_ids), + ) + .await; + assert!(coins.is_err()); + // not enough coins let coins = client .coins_to_spend( @@ -283,6 +304,7 @@ async fn coins_to_spend() { (format!("{:#x}", asset_id_b).as_str(), 301), ], None, + None, ) .await; assert!(coins.is_err()); @@ -296,6 +318,7 @@ async fn coins_to_spend() { (format!("{:#x}", asset_id_b).as_str(), 300), ], 5.into(), + None, ) .await; assert!(coins.is_err()); From d5c1e7fecadede44364d0c4f47499ad458df06e9 Mon Sep 17 00:00:00 2001 From: John Cub Date: Wed, 6 Apr 2022 16:10:24 +0300 Subject: [PATCH 2/6] Update fuel-client/src/client.rs Co-authored-by: Brandon Kite --- fuel-client/src/client.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/fuel-client/src/client.rs b/fuel-client/src/client.rs index 43150ea4267..e384a2e773f 100644 --- a/fuel-client/src/client.rs +++ b/fuel-client/src/client.rs @@ -284,14 +284,7 @@ impl FuelClient { }) .try_collect()?; let excluded_ids: Option> = excluded_ids - .map(|ids| { - ids.iter() - .map( - #[allow(clippy::needless_question_mark)] - |id| -> Result<_, ConversionError> { Ok(id.parse()?) }, - ) - .try_collect() - }) + .map(|ids| ids.into_iter().map(UtxoId::from_str).try_collect()) .transpose()?; let query = schema::coin::CoinsToSpendQuery::build( &(owner, spend_query, max_inputs, excluded_ids).into(), From b0e065e0938bc27193418b9998f68b3ed60db34e Mon Sep 17 00:00:00 2001 From: John Cub Date: Wed, 6 Apr 2022 16:22:29 +0300 Subject: [PATCH 3/6] Update fuel-core/src/coin_query.rs Co-authored-by: Brandon Kite --- fuel-core/src/coin_query.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/fuel-core/src/coin_query.rs b/fuel-core/src/coin_query.rs index c94df95ea8d..b5e47e1cb59 100644 --- a/fuel-core/src/coin_query.rs +++ b/fuel-core/src/coin_query.rs @@ -66,11 +66,9 @@ pub fn largest_first( // Filter excluded coins coin_ids.retain(|&id| { - if let Some(excluded_ids) = excluded_ids { - !excluded_ids.contains(&id) - } else { - true - } + excluded_ids + .map(|excluded_ids| !excluded_ids.contains(&id)) + .unwrap_or(true) }); let mut coins: Vec<(UtxoId, Coin)> = coin_ids From 2be6acc198674f03727f14e6a07be173938338f5 Mon Sep 17 00:00:00 2001 From: John Cub Date: Wed, 6 Apr 2022 16:22:43 +0300 Subject: [PATCH 4/6] Update fuel-core/src/coin_query.rs Co-authored-by: Brandon Kite --- fuel-core/src/coin_query.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/fuel-core/src/coin_query.rs b/fuel-core/src/coin_query.rs index b5e47e1cb59..d6c95cc10e6 100644 --- a/fuel-core/src/coin_query.rs +++ b/fuel-core/src/coin_query.rs @@ -146,11 +146,9 @@ pub fn random_improve( // Filter excluded coins coin_ids.retain(|&id| { - if let Some(excluded_ids) = excluded_ids { - !excluded_ids.contains(&id) - } else { - true - } + excluded_ids + .map(|excluded_ids| !excluded_ids.contains(&id)) + .unwrap_or(true) }); let coins: Vec<(UtxoId, Coin)> = coin_ids From 29784ea75d5b7f4f7a87b37ffe163a80e56dcb49 Mon Sep 17 00:00:00 2001 From: John Cub Date: Wed, 6 Apr 2022 16:31:00 +0300 Subject: [PATCH 5/6] tuple --- fuel-client/src/client/schema/coin.rs | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/fuel-client/src/client/schema/coin.rs b/fuel-client/src/client/schema/coin.rs index 1f657dc10dc..088543ced36 100644 --- a/fuel-client/src/client/schema/coin.rs +++ b/fuel-client/src/client/schema/coin.rs @@ -128,22 +128,15 @@ pub struct CoinsToSpendArgs { excluded_ids: Option>, } -impl - From<( - Address, - Vec, - Option, - Option>, - )> for CoinsToSpendArgs -{ - fn from( - r: ( - Address, - Vec, - Option, - Option>, - ), - ) -> Self { +pub(crate) type CoinsToSpendArgsTuple = ( + Address, + Vec, + Option, + Option>, +); + +impl From for CoinsToSpendArgs { + fn from(r: CoinsToSpendArgsTuple) -> Self { CoinsToSpendArgs { owner: r.0, spend_query: r.1, From c93a80f486d6766b4b8c541d8acc000ca9a34e12 Mon Sep 17 00:00:00 2001 From: John Cub Date: Wed, 6 Apr 2022 16:46:13 +0300 Subject: [PATCH 6/6] fix --- fuel-client/src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fuel-client/src/client.rs b/fuel-client/src/client.rs index e384a2e773f..7acaf3220b1 100644 --- a/fuel-client/src/client.rs +++ b/fuel-client/src/client.rs @@ -284,7 +284,7 @@ impl FuelClient { }) .try_collect()?; let excluded_ids: Option> = excluded_ids - .map(|ids| ids.into_iter().map(UtxoId::from_str).try_collect()) + .map(|ids| ids.into_iter().map(schema::UtxoId::from_str).try_collect()) .transpose()?; let query = schema::coin::CoinsToSpendQuery::build( &(owner, spend_query, max_inputs, excluded_ids).into(),