Reject queries that are recursive during the resolution (#2330)

If during resolution of the query we hit the same type, reject further
execution. The code to do this check uses `Visitor` logic from the
`async-graphql` library, but this logic is private, so I duplicated it
in our branch, Later it would be nice to create PR to them in make it
public. Or add a way to pass our own query validators into the
`async-graphql`(which will be more performant).

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests

### Before requesting review
- [x] I have reviewed the code myself

---------

Co-authored-by: AurelienFT <aurelien.foucault@epitech.eu>
Co-authored-by: Mårten Blankfors <marten@blankfors.se>
Co-authored-by: AurelienFT <32803821+AurelienFT@users.noreply.github.com>
Co-authored-by: AurelienFT <aurefook@gmail.com>

CHANGELOG.md

@@ -8,12 +8,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Added
 - [2324](https://github.com/FuelLabs/fuel-core/pull/2324): Added metrics for sync, async processor and for all GraphQL queries.
+- [2320](https://github.com/FuelLabs/fuel-core/pull/2320): Added new CLI flag `graphql-max-resolver-recursive-depth` to limit recursion within resolver. The default value it "1".
+
 
 ## Fixed
 - [2320](https://github.com/FuelLabs/fuel-core/issues/2320): Prevent `/health` and `/v1/health` from being throttled by the concurrency limiter.
 - [2322](https://github.com/FuelLabs/fuel-core/issues/2322): Set the salt of genesis contracts to zero on execution.
 - [2324](https://github.com/FuelLabs/fuel-core/pull/2324): Ignore peer if we already are syncing transactions from it.
 
+#### Breaking
+
+- [2320](https://github.com/FuelLabs/fuel-core/pull/2330): Reject queries that are recursive during the resolution of the query.
+
 ### Changed
 
 #### Breaking

bin/fuel-core/src/cli/run.rs

@@ -491,6 +491,8 @@ impl Command {
                 max_queries_depth: graphql.graphql_max_depth,
                 max_queries_complexity: graphql.graphql_max_complexity,
                 max_queries_recursive_depth: graphql.graphql_max_recursive_depth,
+                max_queries_resolver_recursive_depth: graphql
+                    .max_queries_resolver_recursive_depth,
                 max_queries_directives: graphql.max_queries_directives,
                 max_concurrent_queries: graphql.graphql_max_concurrent_queries,
                 request_body_bytes_limit: graphql.graphql_request_body_bytes_limit,

bin/fuel-core/src/cli/run/graphql.rs

@@ -24,6 +24,14 @@ pub struct GraphQLArgs {
     #[clap(long = "graphql-max-recursive-depth", default_value = "16", env)]
     pub graphql_max_recursive_depth: usize,
 
+    /// The max resolver recursive depth of GraphQL queries.
+    #[clap(
+        long = "graphql-max-resolver-recursive-depth",
+        default_value = "1",
+        env
+    )]
+    pub max_queries_resolver_recursive_depth: usize,
+
     /// The max number of directives in the query.
     #[clap(long = "graphql-max-directives", default_value = "10", env)]
     pub max_queries_directives: usize,

crates/client/src/client.rs

@@ -1,3 +1,5 @@
+#[cfg(feature = "subscriptions")]
+use crate::client::types::StatusWithTransaction;
 use crate::client::{
     schema::{
         block::BlockByHeightArgs,
@@ -549,6 +551,33 @@ impl FuelClient {
         Ok(status)
     }
 
+    /// Similar to [`Self::submit_and_await_commit`], but the status also contains transaction.
+    #[cfg(feature = "subscriptions")]
+    pub async fn submit_and_await_commit_with_tx(
+        &self,
+        tx: &Transaction,
+    ) -> io::Result<StatusWithTransaction> {
+        use cynic::SubscriptionBuilder;
+        let tx = tx.clone().to_bytes();
+        let s = schema::tx::SubmitAndAwaitSubscriptionWithTransaction::build(TxArg {
+            tx: HexString(Bytes(tx)),
+        });
+
+        let mut stream = self.subscribe(s).await?.map(
+            |r: io::Result<schema::tx::SubmitAndAwaitSubscriptionWithTransaction>| {
+                let status: StatusWithTransaction = r?.submit_and_await.try_into()?;
+                Result::<_, io::Error>::Ok(status)
+            },
+        );
+
+        let status = stream.next().await.ok_or(io::Error::new(
+            io::ErrorKind::Other,
+            "Failed to get status from the submission",
+        ))??;
+
+        Ok(status)
+    }
+
     /// Submits the transaction to the `TxPool` and returns a stream of events.
     /// Compared to the `submit_and_await_commit`, the stream also contains
     /// `SubmittedStatus` as an intermediate state.

crates/client/src/client/schema/tx.rs

@@ -193,6 +193,21 @@ pub enum TransactionStatus {
     Unknown,
 }
 
+#[allow(clippy::enum_variant_names)]
+#[derive(cynic::InlineFragments, Clone, Debug)]
+#[cynic(
+    schema_path = "./assets/schema.sdl",
+    graphql_type = "TransactionStatus"
+)]
+pub enum StatusWithTransaction {
+    SubmittedStatus(SubmittedStatus),
+    SuccessStatus(SuccessStatusWithTransaction),
+    SqueezedOutStatus(SqueezedOutStatus),
+    FailureStatus(FailureStatusWithTransaction),
+    #[cynic(fallback)]
+    Unknown,
+}
+
 #[derive(cynic::QueryFragment, Clone, Debug)]
 #[cynic(schema_path = "./assets/schema.sdl")]
 pub struct SubmittedStatus {
@@ -202,7 +217,17 @@ pub struct SubmittedStatus {
 #[derive(cynic::QueryFragment, Clone, Debug)]
 #[cynic(schema_path = "./assets/schema.sdl")]
 pub struct SuccessStatus {
-    #[cfg(feature = "test-helpers")]
+    pub block_height: U32,
+    pub time: Tai64Timestamp,
+    pub program_state: Option<ProgramState>,
+    pub receipts: Vec<Receipt>,
+    pub total_gas: U64,
+    pub total_fee: U64,
+}
+
+#[derive(cynic::QueryFragment, Clone, Debug)]
+#[cynic(schema_path = "./assets/schema.sdl", graphql_type = "SuccessStatus")]
+pub struct SuccessStatusWithTransaction {
     pub transaction: OpaqueTransaction,
     pub block_height: U32,
     pub time: Tai64Timestamp,
@@ -215,7 +240,18 @@ pub struct SuccessStatus {
 #[derive(cynic::QueryFragment, Clone, Debug)]
 #[cynic(schema_path = "./assets/schema.sdl")]
 pub struct FailureStatus {
-    #[cfg(feature = "test-helpers")]
+    pub block_height: U32,
+    pub time: Tai64Timestamp,
+    pub reason: String,
+    pub program_state: Option<ProgramState>,
+    pub receipts: Vec<Receipt>,
+    pub total_gas: U64,
+    pub total_fee: U64,
+}
+
+#[derive(cynic::QueryFragment, Clone, Debug)]
+#[cynic(schema_path = "./assets/schema.sdl", graphql_type = "FailureStatus")]
+pub struct FailureStatusWithTransaction {
     pub transaction: OpaqueTransaction,
     pub block_height: U32,
     pub time: Tai64Timestamp,
@@ -432,6 +468,17 @@ pub struct SubmitAndAwaitSubscription {
     pub submit_and_await: TransactionStatus,
 }
 
+#[derive(cynic::QueryFragment, Clone, Debug)]
+#[cynic(
+    schema_path = "./assets/schema.sdl",
+    graphql_type = "Subscription",
+    variables = "TxArg"
+)]
+pub struct SubmitAndAwaitSubscriptionWithTransaction {
+    #[arguments(tx: $tx)]
+    pub submit_and_await: StatusWithTransaction,
+}
+
 #[derive(cynic::QueryFragment, Clone, Debug)]
 #[cynic(
     schema_path = "./assets/schema.sdl",

crates/client/src/client/types.rs

@@ -43,6 +43,7 @@ use crate::client::schema::{
     relayed_tx::RelayedTransactionStatus as SchemaRelayedTransactionStatus,
     tx::{
         OpaqueTransactionWithStatus,
+        StatusWithTransaction as SchemaStatusWithTx,
         TransactionStatus as SchemaTxStatus,
     },
     ConversionError,
@@ -94,14 +95,17 @@ pub struct TransactionResponse {
     pub status: TransactionStatus,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
+pub struct StatusWithTransactionResponse {
+    pub status: StatusWithTransaction,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 pub enum TransactionStatus {
     Submitted {
         submitted_at: Tai64,
     },
     Success {
-        #[cfg(feature = "test-helpers")]
-        transaction: Transaction,
         block_height: BlockHeight,
         time: Tai64,
         program_state: Option<ProgramState>,
@@ -113,8 +117,6 @@ pub enum TransactionStatus {
         reason: String,
     },
     Failure {
-        #[cfg(feature = "test-helpers")]
-        transaction: Transaction,
         block_height: BlockHeight,
         time: Tai64,
         reason: String,
@@ -134,8 +136,6 @@ impl TryFrom<SchemaTxStatus> for TransactionStatus {
                 submitted_at: s.time.0,
             },
             SchemaTxStatus::SuccessStatus(s) => TransactionStatus::Success {
-                #[cfg(feature = "test-helpers")]
-                transaction: s.transaction.try_into()?,
                 block_height: s.block_height.into(),
                 time: s.time.0,
                 program_state: s.program_state.map(TryInto::try_into).transpose()?,
@@ -148,8 +148,6 @@ impl TryFrom<SchemaTxStatus> for TransactionStatus {
                 total_fee: s.total_fee.0,
             },
             SchemaTxStatus::FailureStatus(s) => TransactionStatus::Failure {
-                #[cfg(feature = "test-helpers")]
-                transaction: s.transaction.try_into()?,
                 block_height: s.block_height.into(),
                 time: s.time.0,
                 reason: s.reason,
@@ -172,6 +170,80 @@ impl TryFrom<SchemaTxStatus> for TransactionStatus {
     }
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
+pub enum StatusWithTransaction {
+    Submitted {
+        submitted_at: Tai64,
+    },
+    Success {
+        transaction: Transaction,
+        block_height: BlockHeight,
+        time: Tai64,
+        program_state: Option<ProgramState>,
+        receipts: Vec<Receipt>,
+        total_gas: u64,
+        total_fee: u64,
+    },
+    SqueezedOut {
+        reason: String,
+    },
+    Failure {
+        transaction: Transaction,
+        block_height: BlockHeight,
+        time: Tai64,
+        reason: String,
+        program_state: Option<ProgramState>,
+        receipts: Vec<Receipt>,
+        total_gas: u64,
+        total_fee: u64,
+    },
+}
+
+impl TryFrom<SchemaStatusWithTx> for StatusWithTransaction {
+    type Error = ConversionError;
+
+    fn try_from(status: SchemaStatusWithTx) -> Result<Self, Self::Error> {
+        Ok(match status {
+            SchemaStatusWithTx::SubmittedStatus(s) => StatusWithTransaction::Submitted {
+                submitted_at: s.time.0,
+            },
+            SchemaStatusWithTx::SuccessStatus(s) => StatusWithTransaction::Success {
+                transaction: s.transaction.try_into()?,
+                block_height: s.block_height.into(),
+                time: s.time.0,
+                program_state: s.program_state.map(TryInto::try_into).transpose()?,
+                receipts: s
+                    .receipts
+                    .into_iter()
+                    .map(TryInto::try_into)
+                    .collect::<Result<Vec<_>, _>>()?,
+                total_gas: s.total_gas.0,
+                total_fee: s.total_fee.0,
+            },
+            SchemaStatusWithTx::FailureStatus(s) => StatusWithTransaction::Failure {
+                transaction: s.transaction.try_into()?,
+                block_height: s.block_height.into(),
+                time: s.time.0,
+                reason: s.reason,
+                program_state: s.program_state.map(TryInto::try_into).transpose()?,
+                receipts: s
+                    .receipts
+                    .into_iter()
+                    .map(TryInto::try_into)
+                    .collect::<Result<Vec<_>, _>>()?,
+                total_gas: s.total_gas.0,
+                total_fee: s.total_fee.0,
+            },
+            SchemaStatusWithTx::SqueezedOutStatus(s) => {
+                StatusWithTransaction::SqueezedOut { reason: s.reason }
+            }
+            SchemaStatusWithTx::Unknown => {
+                return Err(Self::Error::UnknownVariant("SchemaTxStatus"))
+            }
+        })
+    }
+}
+
 impl TryFrom<OpaqueTransactionWithStatus> for TransactionResponse {
     type Error = ConversionError;
 

crates/fuel-core/Cargo.toml

@@ -16,6 +16,7 @@ async-graphql = { version = "7.0.11", features = [
   "playground",
   "tracing",
 ], default-features = false }
+async-graphql-value = "7.0.11"
 async-trait = { workspace = true }
 axum = { workspace = true }
 clap = { workspace = true, features = ["derive"] }

crates/fuel-core/src/graphql_api.rs

@@ -13,6 +13,7 @@ pub mod database;
 pub(crate) mod metrics_extension;
 pub mod ports;
 pub mod storage;
+pub(crate) mod validation_extension;
 pub(crate) mod view_extension;
 pub mod worker_service;
 
@@ -22,6 +23,7 @@ pub struct ServiceConfig {
     pub max_queries_depth: usize,
     pub max_queries_complexity: usize,
     pub max_queries_recursive_depth: usize,
+    pub max_queries_resolver_recursive_depth: usize,
     pub max_queries_directives: usize,
     pub max_concurrent_queries: usize,
     pub request_body_bytes_limit: usize,

crates/fuel-core/src/graphql_api/api_service.rs

@@ -11,6 +11,7 @@ use crate::{
             P2pPort,
             TxPoolPort,
         },
+        validation_extension::ValidationExtension,
         view_extension::ViewExtension,
         Config,
     },
@@ -225,6 +226,8 @@ where
     let request_timeout = config.config.api_request_timeout;
     let concurrency_limit = config.config.max_concurrent_queries;
     let body_limit = config.config.request_body_bytes_limit;
+    let max_queries_resolver_recursive_depth =
+        config.config.max_queries_resolver_recursive_depth;
 
     let schema = schema
         .limit_complexity(config.config.max_queries_complexity)
@@ -243,6 +246,9 @@ where
         .data(gas_price_provider)
         .data(consensus_parameters_provider)
         .data(memory_pool)
+        .extension(ValidationExtension::new(
+            max_queries_resolver_recursive_depth,
+        ))
         .extension(async_graphql::extensions::Tracing)
         .extension(ViewExtension::new())
         .finish();

crates/fuel-core/src/graphql_api/metrics_extension.rs

@@ -89,6 +89,11 @@ impl Extension for MetricsExtInner {
             _ => None,
         };
 
+        // If it is not a query, skip time metering.
+        if field_name.is_none() {
+            return next.run(ctx, info).await
+        }
+
         let start_time = Instant::now();
         let res = next.run(ctx, info).await;
         let elapsed = start_time.elapsed();