feat: Support block and header versions gql

[bvrooman]

Related issues:

• Closes Support versioning of Block and BlockHeader in the GraphQL API #1796

This PR adds a version field to the Block and BlockHeader entities returned by the GraphQL API.

Additionally, the Block and BlockHeader client types are updated to include the version as well.

Checklist

• Breaking changes are clearly marked as such in the PR description and changelog
• New behavior is reflected in tests
• The specification matches the implemented behavior (link update PR if changes are needed)

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

[Add or remove entries as needed]

• Rust SDK
• Sway compiler
• Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
• Someone else?

[bvrooman]

I moved this out of chain.rs and into the parent level schema.rs for reuse in other submodules.

[bvrooman]

This PR updates the relevant snapshot tests. However, it does not appear that we have tests for all resources against the GQL server-level API or client API (integration or mock GQL), meaning I did not add automated tests that verify the version returned by the GQL server . We have some high level tests for specific actions and resources like TXs but it could be worth having more tests for GQL responses (as long as this responsibility lies with fuel-core and not an indexer).

[MitchTurner]

Looking good. Can you add a test to tests/tests/blocks.rs that checks the version?

edit: I just saw your last comment. Yeah. I think tests/tests/blocks.rs is the right place for that coverage

[bvrooman]

As I was working on the client side testing, I discovered something that may require us to pivot slightly on our current approach:

When using the pattern SomeTypeVersion, defined something like this:

The compiler actually doesn't like this:

error: This type is already used in another variant
  --> crates/fuel-core/src/schema/block.rs:93:8
   |
93 |     V2(Version),
   |        ^^^^^^^

We can't use this pattern to define multiple versions as discriminated variants (it's as if the variant types themselves are the discriminant here).

Next, I tried using something like:

#[derive(SimpleObject)]
pub struct BlockVersion(pub Version);

But this still isn't allowed because tuple types aren't supported, and all values must be named in async_graphql.

So instead, I will just use the Version object directly. We lose the advantages strong typing, but using Version directly allows us to actually create a future-facing pattern.

[bvrooman]

Looking good. Can you add a test to tests/tests/blocks.rs that checks the version?

edit: I just saw your last comment. Yeah. I think tests/tests/blocks.rs is the right place for that coverage

This is a bit tricky: The version isn't actually a field on the client Block type; rather, the version is used to instruct the client on how to construct the client Block type. This means that we cannot simply test for the version number. Real unit tests would require having other versions of the Block, which would allow us to test things like:

• different constructions of Block (i.e. version 1, version 2, etc.)
• unsupported version (i.e. server returns a version not supported by the client)

Because we don't have these, I created some contrived versions and tests here: https://github.com/FuelLabs/fuel-core/pull/1852/files#diff-b43da8aec184b70c3e3ac74b3f156eb2142bf651b724ccfb3d7dc9e6d7d70b01R120. This is just for reference, but should illustrate the kinds of tests we can have in the future, and why we can't really test for that now.

[xgreenx]

We have a separate enum with all supported versions for ConsensusParameters(and other parameters). This strategy forces us to cover all possible values during compilation time, and I think it is still good to continue using it.

[bvrooman]

Updated to use enum at the async-graphql server level and at the cynic client level. This gives us exhaustive variant checks.

[xgreenx]

Looks good to me=)

[xgreenx]

I think we can remove this one=)

[MitchTurner]

This is a bit tricky: The version isn't actually a field on the client Block type; rather, the version is used to instruct the client on how to construct the client Block type. This means that we cannot simply test for the version number. Real unit tests would require having other versions of the Block, which would allow us to test things like:

Okay. That makes sense. I feel like someone mentioned creating something like

    #[cfg(feature = "test_helpers")]
    VTest(VTestBlock)

variant that we could use to show upgradability.

Maybe I misheard. But I'm kinda impartial to something like that. On one hand it would be a decent tracer bullet to prove that we have upgradability, but it also doesn't really capture any desired behavior. Obviously we could get rid of it when we add a real version, but we could also just add the test once we add thet variant.

🤷

[bvrooman]

This is a bit tricky: The version isn't actually a field on the client Block type; rather, the version is used to instruct the client on how to construct the client Block type. This means that we cannot simply test for the version number. Real unit tests would require having other versions of the Block, which would allow us to test things like:

Okay. That makes sense. I feel like someone mentioned creating something like

    #[cfg(feature = "test_helpers")]
    VTest(VTestBlock)

variant that we could use to show upgradability.

Maybe I misheard. But I'm kinda impartial to something like that. On one hand it would be a decent tracer bullet to prove that we have upgradability, but it also doesn't really capture any desired behavior. Obviously we could get rid of it when we add a real version, but we could also just add the test once we add thet variant.

🤷

I like the idea. I hacked together some "test" blocks in another branch to ensure the functionality, which is how I discovered that our previous approach (using Union) didn't actually work. So there's definitely value there.

Adding a test block variant will require adding the supporting test code. This means adding test code to all the match statements, places requiring destructuring of the block into parts (transactions, etc), and other areas touched. The cost of this is added maintenance and code clutter. And ultimately, this may be fine. I can understand reasons to do it and not to do it.

I'd like to try it locally with a cleaner version of my above mentioned branch, including using the proper compiler flags. I will see how it sits and if it still makes sense in practice.

[bvrooman]

After spending some time on adding the Test variant, I think there are practical reasons that make this infeasible.

For one, this kind of test requires supporting the Test variant at both the server level and client level. Whenever an object is added to the server API, it must be included in the SDL. We don't have the ability to have separate SDLs based on what is and isn't hidden behind the test compiler flag (i.e., #[cfg(test)]). We can simply choose to always include the Test variant in the SDL, but now we are leaking test code into production. We also cannot use #[cfg(test)] at the client level at this point, because the SDL doesn't distinguish it as test code, and the compiler complains that the variant is not covered. We could omit using the #[cfg(test)] flag, but again, this would require us to leak test code into production.

For two, supporting this variant introduces test code that requires maintenance. There are a few places that require us to retrieve details about the Block, such as the header and transactions. To support this, we need to fill out the Test block with almost the same data and transformations as a regular V1 block. The maintenance cost may not be justified.

See this commit for details.

In the end, I recommend against codifying this test.

[MitchTurner]

LGTM