Prevent api spam with GQL complexity limits #1676

Voxelot · 2024-02-17T02:44:12Z

Closes #1651
Closes #623

Updated the code to use "complexity" for all queries with context.
Moved all related fields/arguments into GraphQL config/arguments.
Added new CLI arguments:

graphql-max-depth
graphql-max-complexity
graphql-max-recursive-depth

Changed api-request-timeout to be 30s.
Breaking:

Changed the Block request to only fetch transaction IDs.
Changed the SuccessStatus and FailureStatus requests to only fetch block height.

Moved all related fields/arguments into GraphQL config/arguments. Added new CLI arguments: - graphql-max-depth - graphql-max-complexity - graphql-max-recursive-depth Changed `api-request-timeout` to be `30s`. Breaking: - Changed the `Block` request to only fetch transaction IDs. - Changed the `SuccessStatus` and `FailureStatus` requests to only fetch block height.

xgreenx · 2024-06-14T14:27:49Z

crates/fuel-core/src/graphql_api.rs

+pub const QUERY_COSTS: Costs = Costs {
+    balance_query: 4000,
+    coins_to_spend: 10000,
+    get_peers: 2000,
+    estimate_predicates: 3000,
+    dry_run: 3000,
+    submit: 5000,
+    submit_and_await: 10000,
+    status_change: 10000,
+    raw_payload: 10,
+    storage_read: 10,
+    storage_iterator: 100,
+    bytecode_read: 2000,
+};


Default gas costs for main operations/queries.

crates/client/assets/schema.sdl

crates/client/src/client/schema/block.rs

crates/fuel-core/src/schema/block.rs

Dentosal · 2024-06-14T15:40:00Z

Approved as-is, but maybe it would be nice to have some tests that actually verify all of the endpoints respect the timeouts and such.

Voxelot · 2024-06-14T16:43:06Z

@xgreenx these breaking changes seem pretty significant. I think there may be a way to allow users to self restrict the complexity of their queries without breaking the api.

at this stage, huge changes like this could break integrations with indexers or others. If we're going to drastically change the kinds of allowed queries, this should be held off to V2.

Voxelot · 2024-06-14T16:45:40Z

...pshots/fuel_core_client__client__schema__block__tests__block_by_height_query_gql_output.snap

-    transactions {
-      id
-    }
+    transactionIds


why are we making a separate field for these ids?

In this case we don't resolve the transactions field. Resolving of the field fetch al transactions from the storage.

crates/fuel-core/src/schema/balance.rs

xgreenx · 2024-06-14T17:27:35Z

@xgreenx these breaking changes seem pretty significant. I think there may be a way to allow users to self restrict the complexity of their queries without breaking the api.

at this stage, huge changes like this could break integrations with indexers or others. If we're going to drastically change the kinds of allowed queries, this should be held off to V2.

Indexers can just increase the allowed complexity. You can see an example in the test, where I increased it to 1_000_000. To be able to request a lot of blocks.

…without panics

# Conflicts: # bin/fuel-core/chainspec/local-testnet/state_transition_bytecode.wasm

MitchTurner

Looking good. Just want to understand something first.

MitchTurner · 2024-06-25T21:08:35Z

tests/tests/dos.rs

+}
+
+#[tokio::test]
+async fn complex_queries__50_block__query_to_complex() {


Suggested change

async fn complex_queries__50_block__query_to_complex() {

async fn complex_queries__50_block__query_too_complex() {

MitchTurner · 2024-06-25T21:14:12Z

crates/fuel-core/src/schema/balance.rs

@@ -50,6 +51,7 @@ pub struct BalanceQuery;

 #[Object]
 impl BalanceQuery {
+    #[graphql(complexity = "QUERY_COSTS.balance_query")]


I'm confused where these complexity values are being parsed. I guess I'm not familiar enough with the async_graphql::Object api, but looking through the docs I can't find where you can extend it with custom fields like complexity.

It's built into the proc-macro system: https://async-graphql.github.io/async-graphql/en/depth_and_complexity.html

Voxelot · 2024-06-26T21:14:58Z

crates/fuel-core/src/graphql_api.rs

+    get_peers: 10001,
+    // estimate_predicates: 3000,
+    estimate_predicates: 10001,
+    dry_run: 3000,


Isn't a dry-run potentially more expensive than estimating predicates?

It is with more transactions, but we want to allow the use of the feature with several dry runs.

Voxelot · 2024-06-26T21:16:29Z

crates/fuel-core/src/graphql_api.rs

+
+pub const QUERY_COSTS: Costs = Costs {
+    // balance_query: 4000,
+    balance_query: 10001,


[nit] maybe we should have a constant for the default max complexity, and then use that here (ie. MAX_COMPLEXITY_DEFAULT / 2 + 1)

Voxelot · 2024-06-26T21:17:34Z

crates/fuel-core/src/graphql_api.rs

+    submit: 10001,
+    submit_and_await: 10001,
+    status_change: 10001,
+    raw_payload: 10,


a raw payload could also include the same complexity as a bytecode query if a block is full of create txs right?

Yes, but we can't limit it=(

With the bytecode for the contract, we want to be sure that there is no more than one query. In the case of a transaction, we need to return the e whole transaction.

# Conflicts: # bin/fuel-core/chainspec/local-testnet/state_transition_bytecode.wasm # crates/fuel-core/src/schema/balance.rs # crates/fuel-core/src/schema/chain.rs # crates/fuel-core/src/schema/coins.rs # crates/fuel-core/src/schema/contract.rs # crates/fuel-core/src/schema/gas_price.rs # crates/fuel-core/src/schema/message.rs # crates/fuel-core/src/schema/relayed_tx.rs

@Dentosal

## Version v0.31.0 ### Added - [#2014](#2014): Added a separate thread for the block importer. - [#2013](#2013): Added a separate thread to process P2P database lookups. - [#2004](#2004): Added new CLI argument `continue-services-on-error` to control internal flow of services. - [#2004](#2004): Added handling of incorrect shutdown of the off-chain GraphQL worker by using state rewind feature. - [#2007](#2007): Improved metrics: - Added database metrics per column. - Added statistic about commit time of each database. - Refactored how metrics are registered: Now, we use only one register shared between all metrics. This global register is used to encode all metrics. - [#1996](#1996): Added support for rollback command when state rewind feature is enabled. The command allows the rollback of the state of the blockchain several blocks behind until the end of the historical window. The default historical window it 7 days. - [#1996](#1996): Added support for the state rewind feature. The feature allows the execution of the blocks in the past and the same execution results to be received. Together with forkless upgrades, execution of any block from the past is possible if historical data exist for the target block height. - [#1994](#1994): Added the actual implementation for the `AtomicView::latest_view`. - [#1972](#1972): Implement `AlgorithmUpdater` for `GasPriceService` - [#1948](#1948): Add new `AlgorithmV1` and `AlgorithmUpdaterV1` for the gas price. Include tools for analysis - [#1676](#1676): Added new CLI arguments: - `graphql-max-depth` - `graphql-max-complexity` - `graphql-max-recursive-depth` ### Changed - [#2015](#2015): Small fixes for the database: - Fixed the name for historical columns - Metrics was working incorrectly for historical columns. - Added recommended setting for the RocksDB - The source of recommendation is official documentation https://github.com/facebook/rocksdb/wiki/Setup-Options-and-Basic-Tuning#other-general-options. - Removed repairing since it could corrupt the database if fails - Several users reported about the corrupted state of the database after having a "Too many descriptors" error where in logs, repairing of the database also failed with this error creating a `lost` folder. - [#2010](#2010): Updated the block importer to allow more blocks to be in the queue. It improves synchronization speed and mitigate the impact of other services on synchronization speed. - [#2006](#2006): Process block importer events first under P2P pressure. - [#2002](#2002): Adapted the block producer to react to checked transactions that were using another version of consensus parameters during validation in the TxPool. After an upgrade of the consensus parameters of the network, TxPool could store invalid `Checked` transactions. This change fixes that by tracking the version that was used to validate the transactions. - [#1999](#1999): Minimize the number of panics in the codebase. - [#1990](#1990): Use latest view for mutate GraphQL queries after modification of the node. - [#1992](#1992): Parse multiple relayer contracts, `RELAYER-V2-LISTENING-CONTRACTS` env variable using a `,` delimiter. - [#1980](#1980): Add `Transaction` to relayer 's event filter #### Breaking - [#2012](#2012): Bumped the `fuel-vm` to `0.55.0` release. More about the change [here](https://github.com/FuelLabs/fuel-vm/releases/tag/v0.55.0). - [#2001](#2001): Prevent GraphQL query body to be huge and cause OOM. The default body size is `1MB`. The limit can be changed by the `graphql-request-body-bytes-limit` CLI argument. - [#1991](#1991): Prepare the database to use different types than `Database` for atomic view. - [#1989](#1989): Extract `HistoricalView` trait from the `AtomicView`. - [#1676](#1676): New `fuel-core-client` is incompatible with the old `fuel-core` because of two requested new fields. - [#1676](#1676): Changed default value for `api-request-timeout` to be `30s`. - [#1676](#1676): Now, GraphQL API has complexity and depth limitations on the queries. The default complexity limit is `20000`. It is ~50 blocks per request with transaction IDs and ~2-5 full blocks. ### Fixed - [#2000](#2000): Use correct query name in metrics for aliased queries. ## What's Changed * Generate and publish code coverage reports in the CI by @Dentosal in #1947 * Gas Price Algorithm by @MitchTurner in #1948 * Use companies fork of the `publish-crates` action by @xgreenx in #1986 * Weekly `cargo update` by @github-actions in #1985 * Implement gas price updater for service by @MitchTurner in #1972 * Extract `HistoricalView` trait from the `AtomicView` by @xgreenx in #1989 * Use fresh `ReadView` for mutate queries by @xgreenx in #1990 * Prevent api spam with GQL complexity limits by @Voxelot in #1676 * Enable parsing multiple relayer listening contract addresses from environment variables by @Jurshsmith in #1992 * Prepare the database to use different types than `Database` for atomic view by @xgreenx in #1991 * Added the actual implementation for the `AtomicView::latest_view` by @xgreenx in #1994 * Weekly `cargo update` by @github-actions in #1998 * Minimize the number of panics in the codebase by @xgreenx in #1999 * feat: include Transaction events in topic0 filter for download_logs by @DefiCake in #1980 * Use correct query name for metrics by @xgreenx in #2000 * Prevent GraphQL query body to be huge and cause OOM by @xgreenx in #2001 * Adapted the block producer to react on the outdated transactions from the TxPool by @xgreenx in #2002 * Process block importer events first under P2P pressure by @xgreenx in #2006 * Implementation of the state rewind feature for the RocksDB by @xgreenx in #1996 * Upgraded `fuel-vm` to `0.55.0` by @xgreenx in #2012 * Improved metrics for the database by @xgreenx in #2007 * Updated block importer to allow more blocks to be queue by @xgreenx in #2010 * Added handling of incorrect shutdown of the off-chain GraphQL worker by @xgreenx in #2004 * Moved P2P database lookups into a separate thread by @xgreenx in #2013 * Use dedicated thread for the block importer by @xgreenx in #2014 * Small fixes for the database by @xgreenx in #2015 ## New Contributors * @Jurshsmith made their first contribution in #1992 * @DefiCake made their first contribution in #1980 **Full Changelog**: v0.30.0...v0.31.0

incorporate query complexity limits to restrict api abuse

999c980

Voxelot self-assigned this Feb 17, 2024

xgreenx added 2 commits June 14, 2024 10:50

Merge branch 'refs/heads/master' into Voxelot/gql-complexity-limits

03e8b2a

xgreenx requested review from xgreenx and a team June 14, 2024 14:24

xgreenx marked this pull request as ready for review June 14, 2024 14:24

xgreenx previously approved these changes Jun 14, 2024

View reviewed changes

Make spellcheck happy

f9c4724

xgreenx dismissed their stale review via f9c4724 June 14, 2024 14:25

xgreenx previously approved these changes Jun 14, 2024

View reviewed changes

xgreenx requested a review from a team June 14, 2024 14:29

xgreenx changed the title ~~WIP: prevent api spam with GQL complexity limits~~ Prevent api spam with GQL complexity limits Jun 14, 2024

Make CI happy

f5353e1

xgreenx dismissed their stale review via f5353e1 June 14, 2024 14:44

xgreenx previously approved these changes Jun 14, 2024

View reviewed changes

Dentosal reviewed Jun 14, 2024

View reviewed changes

crates/client/assets/schema.sdl Outdated Show resolved Hide resolved

Dentosal reviewed Jun 14, 2024

View reviewed changes

crates/client/src/client/schema/block.rs Outdated Show resolved Hide resolved

Dentosal reviewed Jun 14, 2024

View reviewed changes

crates/fuel-core/src/schema/block.rs Outdated Show resolved Hide resolved

Apply suggestions

a714a12

xgreenx dismissed their stale review via a714a12 June 14, 2024 15:35

xgreenx requested a review from a team June 14, 2024 15:37

Dentosal previously approved these changes Jun 14, 2024

View reviewed changes

Merge branch 'master' into Voxelot/gql-complexity-limits

0021281

Voxelot commented Jun 14, 2024

View reviewed changes

crates/fuel-core/src/schema/balance.rs Show resolved Hide resolved

Only allow one big query per request for now

4431e26

xgreenx dismissed Dentosal’s stale review via 4431e26 June 14, 2024 18:17

xgreenx and others added 8 commits June 14, 2024 22:35

Merge branch 'master' into Voxelot/gql-complexity-limits

3eace26

Use the latest async-graphql because it supports complexity better …

124def4

…without panics

Merge branch 'refs/heads/master' into Voxelot/gql-complexity-limits

311307f

# Conflicts: # bin/fuel-core/chainspec/local-testnet/state_transition_bytecode.wasm

We want to allow dry running of up to 6 transactions

a8946d2

Added tests to verify new behaviour

f61cf16

Updated schema because of new version async graphqk lirbarry

54bde30

Fixed test beacuase of WASM compilatio ntimeone

0842188

Merge branch 'master' into Voxelot/gql-complexity-limits

e9f95e3

xgreenx previously approved these changes Jun 24, 2024

View reviewed changes

xgreenx mentioned this pull request Jun 24, 2024

feat!: implement pagination for Account methods FuelLabs/fuels-ts#2408

Merged

4 tasks

Merge branch 'master' into Voxelot/gql-complexity-limits

2912a9e

MitchTurner reviewed Jun 25, 2024

View reviewed changes

Voxelot commented Jun 26, 2024

View reviewed changes

Voxelot added 2 commits June 26, 2024 14:33

fix merge issue

06da1c5

Voxelot dismissed xgreenx’s stale review via 06da1c5 June 26, 2024 21:38

xgreenx approved these changes Jun 26, 2024

View reviewed changes

Voxelot enabled auto-merge (squash) June 26, 2024 21:50

Voxelot merged commit 749e732 into master Jun 26, 2024
29 checks passed

Voxelot deleted the Voxelot/gql-complexity-limits branch June 26, 2024 21:57

xgreenx mentioned this pull request Jul 5, 2024

Release v0.31.0 #2016

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent api spam with GQL complexity limits #1676

Prevent api spam with GQL complexity limits #1676

Voxelot commented Feb 17, 2024 •

edited by xgreenx

Loading

xgreenx Jun 14, 2024

Dentosal commented Jun 14, 2024

Voxelot commented Jun 14, 2024 •

edited

Loading

Voxelot Jun 14, 2024

xgreenx Jun 14, 2024

xgreenx commented Jun 14, 2024

MitchTurner left a comment

MitchTurner Jun 25, 2024

MitchTurner Jun 25, 2024

Voxelot Jun 26, 2024

Voxelot Jun 26, 2024

xgreenx Jun 26, 2024

Voxelot Jun 26, 2024

Voxelot Jun 26, 2024 •

edited

Loading

xgreenx Jun 26, 2024

	async fn complex_queries__50_block__query_to_complex() {
	async fn complex_queries__50_block__query_too_complex() {

Prevent api spam with GQL complexity limits #1676

Prevent api spam with GQL complexity limits #1676

Conversation

Voxelot commented Feb 17, 2024 • edited by xgreenx Loading

Choose a reason for hiding this comment

Dentosal commented Jun 14, 2024

Voxelot commented Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xgreenx commented Jun 14, 2024

MitchTurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Voxelot Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Voxelot commented Feb 17, 2024 •

edited by xgreenx

Loading

Voxelot commented Jun 14, 2024 •

edited

Loading

Voxelot Jun 26, 2024 •

edited

Loading