Add GraphQL extension to provide current STF and CP versions #2715

rafal-ch · 2025-02-13T16:54:08Z

Linked Issues/PRs

Description

This PR adds new GraphQL extensions to provide:

current STF version
current consensus parameters version

Example response:

{
  "data": {
    "nodeInfo": {
      "nodeVersion": "0.41.6"
    }
  },
  "extensions": {
    "current_consensus_parameters_version": 0,
    "current_fuel_block_height": 0,
    "current_stf_version": 21
  }
}

Additional housekeeping changes:

all graphql extensions have been moved into a subdirectory

TODO

Add tests to check that correct versions are returned when STF or CP are bumped
Rework how we retrieve the current STF version

Checklist

Breaking changes are clearly marked as such in the PR description and changelog
New behavior is reflected in tests

Before requesting review

I have reviewed the code myself
I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

Rust SDK

…sus_parameters_with_version()`

rafal-ch · 2025-02-13T16:59:17Z

crates/fuel-core/src/graphql_api/extensions/current_stf_version.rs

+        if let Ok(view) = db.view() {
+            if let Ok(latest_block) = view.latest_block() {
+                let current_stf_version =
+                    latest_block.header().state_transition_bytecode_version();
+                response.extensions.insert(
+                    CURRENT_STF_VERSION.to_string(),
+                    Value::Number(current_stf_version.into()),
+                );
+            }
+        }


We might prefer reading the current STF version directly from on chain DB table StateTransitionBytecodeVersions similarly to how it's done in BlockProducerDatabase::latest_state_transition_bytecode_version(), but I couldn't figure out how to access this in GraphQL.

We need to have a place where we cache this kind of information. Maybe, we could extend consensus parameters provider to also provide this kind of information, because fetch it from the database it a lot of waste work.

If we do that, it might be useful to also add the block height to the information retrieved, as in this case all the extension metadata will come from the same place.

Btw, If I am not mistaken all the information we want to fetch comes from the block header right now, so maybe a HeaderProvider trait with a function that returns the whole block header, and then stf_version and consensus_parameters_version and block_height can be provided methods?

I think I'll prefer to extend (and rename) the "Consensus Parameters Provider". However, I find caching the entire block header not optimal and my preference would be to cache exactly what we need. So, in addition to the current data, it'll store:

current STF version

current block height

wdyt?

Currently, the STF version is cached in the "Consensus Parameters Provider", so we do not read from the DB every time, but handling of the current block height remains unchanged from the original implementation.

…onal_graphql_extensions

…d via GraphQL extension

…ension

rafal-ch · 2025-02-14T11:54:15Z

We're currently attaching some substantial number of bytes to every response. Even if user requests just the node version, he will get a bunch of additional data, which, AFAIK, cannot be opted-out from.

{
  "data": {
    "nodeInfo": {
      "nodeVersion": "0.41.6"
    }
  },
  "extensions": {
    "current_consensus_parameters_version": 0,
    "current_fuel_block_height": 0,
    "current_stf_version": 21
  }
}

Not sure if this should be a concern, but I thought it's worth pointing out.

rymnc · 2025-02-14T12:52:40Z

We're currently attaching some substantial number of bytes to every response. Even if user requests just the node version, he will get a bunch of additional data, which, AFAIK, cannot be opted-out from.
{
  "data": {
    "nodeInfo": {
      "nodeVersion": "0.41.6"
    }
  },
  "extensions": {
    "current_consensus_parameters_version": 0,
    "current_fuel_block_height": 0,
    "current_stf_version": 21
  }
}
Not sure if this should be a concern, but I thought it's worth pointing out.

we miss the whole point of graphql here, if we provide data no one asked for :(

rafal-ch · 2025-02-14T13:02:02Z

we miss the whole point of graphql here, if we provide data no one asked for :(

Exactly, that's why I was thinking that we should just provide this data via some info endpoint, but IIUC, the idea here is to provide this data proactively instead of requiring users to poll regularly.

Maybe there's a way to opt out from extensions which I'm just missing.

xgreenx · 2025-02-14T16:41:12Z

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

        .extension(RequiredFuelBlockHeightExtension::new())
+        .extension(CurrentStfVersionExtension::new())
+        .extension(CurrentConsensusParametersVersionExtension::new())


I think spawning a separate extension for the field it is too overkill=D It decreases performance without any reason

I think we can have one extension fully dedicated to return information to the end user. Like STFV, CPV, current block height(we can remove setting of the current height from the RequiredFuelBlockHeightExtension).

In this case we will have one extension that we can use to add any metadata

The following changes have been made:

Both STF and CP extensions have been merged into "chain state info" extension

The "chain state info" extension is now also providing data about the current block height

xgreenx · 2025-02-14T16:43:07Z

crates/fuel-core/src/graphql_api/extensions/current_stf_version.rs

+        if let Ok(view) = db.view() {
+            if let Ok(latest_block) = view.latest_block() {
+                let current_stf_version =
+                    latest_block.header().state_transition_bytecode_version();
+                response.extensions.insert(
+                    CURRENT_STF_VERSION.to_string(),
+                    Value::Number(current_stf_version.into()),
+                );
+            }
+        }


We need to have a place where we cache this kind of information. Maybe, we could extend consensus parameters provider to also provide this kind of information, because fetch it from the database it a lot of waste work.

xgreenx · 2025-02-14T16:46:03Z

tests/test-helpers/src/lib.rs

+    vec![op::ret(1)].into_iter().collect::<Vec<u8>>()
+}
+
+pub fn get_graphql_extension_field_value(result_json: &str, name: &str) -> u64 {


fuel-core-client should fetch this information in the same way how it is done for the required block height. We have ExtensionsResponse where you can add several new fields.

Implementation details: In the case of the CPV and STFV we don't need any policy, we just can fetch the latest one and provide a getter

Updated in ef9c445

xgreenx · 2025-02-14T16:56:57Z

we miss the whole point of graphql here, if we provide data no one asked for :(

The extensions field is part of the GraphQL specification, and we can put there any data what we want. The user controls only the data field.

…al_graphql_extensions

crates/fuel-core/src/graphql_api/extensions/current_consensus_parameters_version.rs

rafal-ch · 2025-02-18T09:32:05Z

We need to have a place where we cache this kind of information. Maybe, we could extend consensus parameters provider to also provide this kind of information, because fetch it from the database it a lot of waste work.

Done in: b861a51

…ions

xgreenx

Looks good to mee, small things to change and we are good to go=)

xgreenx · 2025-02-19T19:15:19Z

crates/client/src/client.rs

@@ -333,6 +388,8 @@ impl FuelClient {
            .await
            .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;

+        self.update_chain_state_info(&response);


We need to do it in the Self::decode_response function because we also want to handle the case of subscriptions

Updated in 3aa2d2d

xgreenx · 2025-02-19T19:16:39Z

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

+        // `RequiredFuelBlockHeightExtension` uses the view set by the ViewExtension.
+        // Do not reorder this line before adding the `ViewExtension`.


Suggested change

// `RequiredFuelBlockHeightExtension` uses the view set by the ViewExtension.

// Do not reorder this line before adding the `ViewExtension`.

Done in 3f32362

xgreenx · 2025-02-19T20:15:13Z

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

+        let current_stf_version = block.header().state_transition_bytecode_version();
+        self.stf_parameters_provider
+            .cache_stf_version(current_stf_version);


We can have this login inside of the ConsensusParametersProvider. We don't need to teach off-chain worker to provide this information=)

You're right, that was a bit of a code smell since the beginning.
Updated in 932a1c5

xgreenx · 2025-02-19T20:18:28Z

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

@@ -117,7 +117,7 @@ impl MessageCoin {
    async fn asset_id(&self, ctx: &Context<'_>) -> AssetId {
        let params = ctx
            .data_unchecked::<ConsensusProvider>()
-            .latest_consensus_params();
+            .current_consensus_params();


May I also ask you to replace Mutex in the ConsensusProvider with RwLock please?=)

Since we only read in most of the cases and write happens once per month

Done in d8ea573

xgreenx · 2025-02-19T20:19:32Z

crates/fuel-core/src/service/adapters/consensus_parameters_provider.rs

    pub(crate) consensus_parameters:
        SharedMutex<HashMap<ConsensusParametersVersion, Arc<ConsensusParameters>>>,
+    pub(crate) latest_stf_version: SharedMutex<StateTransitionBytecodeVersion>,


May I also ask you to replace Mutex with RwLock for consensus_parameters and latest_stf_version please?=)

Since we only read in most of the cases and write happens once per month

Done in d8ea573

…al_graphql_extensions

…in subscriptions

…al_graphql_extensions

xgreenx · 2025-02-22T00:57:36Z

crates/client/src/client.rs

    fn decode_response<R, E>(
+        &self,
        response: FuelGraphQlResponse<R, E>,
        inner_required_height: Option<Arc<Mutex<Option<BlockHeight>>>>,


If you are able to use &self, then you can remove inner_required_height(move logic inside update_chain_state_info) and update everything in one function=)

xgreenx · 2025-02-22T01:12:35Z

crates/fuel-core/src/service/adapters/consensus_parameters_provider.rs

+                if !update_cached_value(
+                    &self.shared_state.latest_consensus_parameters_version,
+                    new_consensus_parameters_version,
+                    || self.shared_state.cache_consensus_parameters(new_consensus_parameters_version).map(|_| ())
+                ) {


You have a deadlock here. self.shared_state.cache_consensus_parameters is doing write lock inside and update_cached_value is doing upgradable lock

rafal-ch added 9 commits February 13, 2025 11:50

Introduce separate module for GraphQL extensions

cc4e8c2

Revert changes to test structure

3b48d47

Add scaffolding for the current_stf_version extension

9bd97b7

Add possible way to go with the SFT update notification

ac861c4

Add current_consensus_parameters_version GraphQL extension

c022ba7

Elaborate on the possible ways to go for STF

56d4c57

Use latest_consensus_parameters_version() instead of `latest_consen…

51237b3

…sus_parameters_with_version()`

Add current_stf_version GraphQL extension

335b243

Update comment

946e315

rafal-ch commented Feb 13, 2025

View reviewed changes

rafal-ch added 5 commits February 13, 2025 18:04

Update changelog

2616cd7

Merge remote-tracking branch 'upstream/master' into rafal/2596_additi…

da57953

…onal_graphql_extensions

Add test checking that proper consensus parameters version is returne…

2772ba7

…d via GraphQL extension

Add test checking that proper stf version is returned via GraphQL ext…

3359d61

…ension

Clean-up

d51222f

rafal-ch marked this pull request as ready for review February 14, 2025 11:52

rafal-ch requested review from xgreenx, Dentosal, MitchTurner, netrome, acerone85, rymnc and AurelienFT as code owners February 14, 2025 11:52

xgreenx reviewed Feb 14, 2025

View reviewed changes

rafal-ch added 2 commits February 17, 2025 11:38

Merge remote-tracking branch 'origin/master' into rafal/2596_addition…

5c24c74

…al_graphql_extensions

Merge STF and CP extensions into one

bd610e7

acerone85 reviewed Feb 17, 2025

View reviewed changes

crates/fuel-core/src/graphql_api/extensions/current_consensus_parameters_version.rs Outdated Show resolved Hide resolved

Provide current block height via 'chain state info' extension

3f3e971

rafal-ch force-pushed the rafal/2596_additional_graphql_extensions branch from 420319f to 3f3e971 Compare February 17, 2025 12:26

Typos

c3c0148

rafal-ch changed the title ~~Add GraphQL extensions to provide current STF and CP versions~~ Add GraphQL extension to provide current STF and CP versions Feb 17, 2025

rafal-ch added 2 commits February 17, 2025 19:27

Access latest STF and CP version via FuelClient

ef9c445

Cache STF version in consensus parameters provider

b861a51

rafal-ch added 3 commits February 18, 2025 10:58

Move all GraphQL extensions into a single place

0b27037

Worker service now uses STF provider, not ConsensusParameters provider

946fc41

Unify current vs latest naming in the code around grapqhql extens…

accfae0

…ions

xgreenx reviewed Feb 19, 2025

View reviewed changes

rafal-ch added 13 commits February 20, 2025 13:02

Merge remote-tracking branch 'origin/master' into rafal/2596_addition…

4cd21b4

…al_graphql_extensions

Update changelog according to the new changelog management system

bbecdb8

Merge remote-tracking branch 'origin/master' into rafal/2596_addition…

e0f21bf

…al_graphql_extensions

Move const into the test code

6d1ec52

Remove unnecessary comment

3f32362

ConsensusParametersProvider uses RwLock instead of Mutex

d8ea573

Match all variants explicitly

69b0da5

Chain state info is updated in decode_response to also be included …

3aa2d2d

…in subscriptions

Fix formatting

e03baac

Do not add ConsensusParametersProvider to worker service

932a1c5

Clean-up

ed1dd26

Clean-up

bfc5f58

Merge remote-tracking branch 'origin/master' into rafal/2596_addition…

03f9645

…al_graphql_extensions

xgreenx reviewed Feb 22, 2025

View reviewed changes

Applied comments from the PR

b44cac9

xgreenx approved these changes Feb 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GraphQL extension to provide current STF and CP versions #2715

Add GraphQL extension to provide current STF and CP versions #2715

rafal-ch commented Feb 13, 2025 •

edited

Loading

rafal-ch Feb 13, 2025

xgreenx Feb 14, 2025

acerone85 Feb 17, 2025 •

edited

Loading

rafal-ch Feb 17, 2025

rafal-ch Feb 19, 2025

rafal-ch commented Feb 14, 2025

rymnc commented Feb 14, 2025

rafal-ch commented Feb 14, 2025 •

edited

Loading

xgreenx Feb 14, 2025

xgreenx Feb 14, 2025

rafal-ch Feb 17, 2025

xgreenx Feb 14, 2025

xgreenx Feb 14, 2025

rafal-ch Feb 17, 2025

xgreenx commented Feb 14, 2025

rafal-ch commented Feb 18, 2025

xgreenx left a comment

xgreenx Feb 19, 2025

rafal-ch Feb 21, 2025

xgreenx Feb 19, 2025

rafal-ch Feb 21, 2025

xgreenx Feb 19, 2025

rafal-ch Feb 21, 2025

xgreenx Feb 19, 2025

rafal-ch Feb 21, 2025

xgreenx Feb 19, 2025

rafal-ch Feb 21, 2025

xgreenx Feb 22, 2025

xgreenx Feb 22, 2025

		// `RequiredFuelBlockHeightExtension` uses the view set by the ViewExtension.
		// Do not reorder this line before adding the `ViewExtension`.

Add GraphQL extension to provide current STF and CP versions #2715

Are you sure you want to change the base?

Add GraphQL extension to provide current STF and CP versions #2715

Conversation

rafal-ch commented Feb 13, 2025 • edited Loading

Linked Issues/PRs

Description

Additional housekeeping changes:

TODO

Checklist

Before requesting review

After merging, notify other teams

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acerone85 Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafal-ch commented Feb 14, 2025

rymnc commented Feb 14, 2025

rafal-ch commented Feb 14, 2025 • edited Loading

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

xgreenx commented Feb 14, 2025

rafal-ch commented Feb 18, 2025

xgreenx 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

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

rafal-ch commented Feb 13, 2025 •

edited

Loading

acerone85 Feb 17, 2025 •

edited

Loading

rafal-ch commented Feb 14, 2025 •

edited

Loading