Skip to content

Commit

Permalink
fix: Use Enum for ConsensusParametersVersion and related types (#…
Browse files Browse the repository at this point in the history
…1856)

Related issues:
- Untracked

While testing the feature for sending `Block` versions over GraphQL, we
discovered that the current approach to GraphQL type versioning doesn't
work as expected. Using `async-graphql::Union` is poorly suited for
versioning because:
- union types cannot define multiple variants with the same inner types
(e.g., you cannot define a union with `V1(Version), V2(Version), ...`)
- union types are designed to represent sum types/disjoint union types
(e.g., `HeavyOperation` and `LightOperation` where the type must be one
or the other)

This prevents us from defining a second version. I.e.,
```
#[derive(Union)]
pub enum ConsensusParametersVersion {
    V1(Version),
    V2(Version),
}
```
will fail to compile, preventing us from releasing new versions.

The solution is to use `async-graphql::Enum`. `Enum` writes the enum in
the SDL as a real enum. This allows us to reuse types within variants or
provide empty variants. This is better suited for versioning. This will
allow us to define future versions easily:

```
#[derive(Enum)]
pub enum ConsensusParametersVersion {
    V1,
    V2,
}
```

This change must be used to patch `0.24` and `0.25`. Once these versions
are updated:
- We must update the image in the dev cluster
- SDK teams must update their versions to include the fix
- Sway team must be updated to use new `fuels-rs`

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [ ] New behavior is reflected in tests
- [ ] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [x] 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](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?
  • Loading branch information
Brandon Vrooman authored Apr 24, 2024
1 parent 88a2ce7 commit f7bd84e
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 157 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

Description of the upcoming release here.
### Fixed

- [#1856](https://github.com/FuelLabs/fuel-core/pull/1856): Replaced instances of `Union` with `Enum` for GraphQL definitions of `ConsensusParametersVersion` and related types. This is needed because `Union` does not support multiple `Version`s inside discriminants or empty variants.

### Changed

Expand Down
34 changes: 21 additions & 13 deletions crates/client/assets/schema.sdl
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ type ConsensusParametersPurpose {
checksum: Bytes32!
}

union ConsensusParametersVersion = Version
enum ConsensusParametersVersion {
V1
}

type Contract {
id: ContractId!
Expand Down Expand Up @@ -264,7 +266,9 @@ type ContractParameters {
maxStorageSlots: U64!
}

union ContractParametersVersion = Version
enum ContractParametersVersion {
V1
}

union DependentCost = LightOperation | HeavyOperation

Expand Down Expand Up @@ -323,7 +327,9 @@ type FeeParameters {
gasPerByte: U64!
}

union FeeParametersVersion = Version
enum FeeParametersVersion {
V1
}


type GasCosts {
Expand Down Expand Up @@ -440,7 +446,9 @@ type GasCosts {
newStoragePerByte: U64!
}

union GasCostsVersion = Version
enum GasCostsVersion {
V1
}

type Genesis {
"""
Expand Down Expand Up @@ -798,7 +806,9 @@ type PredicateParameters {
maxMessageDataLength: U64!
}

union PredicateParametersVersion = Version
enum PredicateParametersVersion {
V1
}

type ProgramState {
returnType: ReturnType!
Expand Down Expand Up @@ -952,7 +962,9 @@ type ScriptParameters {
maxScriptDataLength: U64!
}

union ScriptParametersVersion = Version
enum ScriptParametersVersion {
V1
}

scalar Signature

Expand Down Expand Up @@ -1100,7 +1112,9 @@ type TxParameters {
maxBytecodeSubsections: U16!
}

union TxParametersVersion = Version
enum TxParametersVersion {
V1
}

scalar TxPointer

Expand All @@ -1110,8 +1124,6 @@ scalar U32

scalar U64

scalar U8

union UpgradePurpose = ConsensusParametersPurpose | StateTransitionPurpose

scalar UtxoId
Expand All @@ -1122,10 +1134,6 @@ type VariableOutput {
assetId: AssetId!
}

type Version {
value: U8!
}

schema {
query: Query
mutation: Mutation
Expand Down
84 changes: 21 additions & 63 deletions crates/client/src/client/schema/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,8 @@ use crate::client::schema::{
U16,
U32,
U64,
U8,
};

#[derive(cynic::QueryFragment, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub struct Version {
pub value: U8,
}

#[derive(cynic::QueryFragment, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub struct ConsensusParameters {
Expand All @@ -32,12 +25,10 @@ pub struct ConsensusParameters {
pub privileged_address: Address,
}

#[derive(cynic::InlineFragments, Clone, Debug)]
#[derive(cynic::Enum, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub enum ConsensusParametersVersion {
V1(Version),
#[cynic(fallback)]
Unknown,
V1,
}

#[derive(cynic::QueryFragment, Clone, Debug)]
Expand All @@ -52,20 +43,18 @@ pub struct TxParameters {
pub max_bytecode_subsections: U16,
}

#[derive(cynic::InlineFragments, Clone, Debug)]
#[derive(cynic::Enum, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub enum TxParametersVersion {
V1(Version),
#[cynic(fallback)]
Unknown,
V1,
}

impl TryFrom<TxParameters> for fuel_core_types::fuel_tx::TxParameters {
type Error = ConversionError;

fn try_from(params: TxParameters) -> Result<Self, Self::Error> {
match params.version {
TxParametersVersion::V1(_) => Ok(
TxParametersVersion::V1 => Ok(
fuel_core_types::fuel_tx::consensus_parameters::TxParametersV1 {
max_inputs: params.max_inputs.into(),
max_outputs: params.max_outputs.into(),
Expand All @@ -76,9 +65,6 @@ impl TryFrom<TxParameters> for fuel_core_types::fuel_tx::TxParameters {
}
.into(),
),
TxParametersVersion::Unknown => {
Err(ConversionError::UnknownVariant("TxParametersVersion"))
}
}
}
}
Expand All @@ -93,20 +79,18 @@ pub struct PredicateParameters {
pub max_gas_per_predicate: U64,
}

#[derive(cynic::InlineFragments, Clone, Debug)]
#[derive(cynic::Enum, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub enum PredicateParametersVersion {
V1(Version),
#[cynic(fallback)]
Unknown,
V1,
}

impl TryFrom<PredicateParameters> for fuel_core_types::fuel_tx::PredicateParameters {
type Error = ConversionError;

fn try_from(params: PredicateParameters) -> Result<Self, Self::Error> {
match params.version {
PredicateParametersVersion::V1(_) => Ok(
PredicateParametersVersion::V1 => Ok(
fuel_core_types::fuel_tx::consensus_parameters::PredicateParametersV1 {
max_predicate_length: params.max_predicate_length.into(),
max_predicate_data_length: params.max_predicate_data_length.into(),
Expand All @@ -115,9 +99,6 @@ impl TryFrom<PredicateParameters> for fuel_core_types::fuel_tx::PredicateParamet
}
.into(),
),
PredicateParametersVersion::Unknown => Err(ConversionError::UnknownVariant(
"PredicateParametersVersion",
)),
}
}
}
Expand All @@ -130,29 +111,24 @@ pub struct ScriptParameters {
pub max_script_data_length: U64,
}

#[derive(cynic::InlineFragments, Clone, Debug)]
#[derive(cynic::Enum, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub enum ScriptParametersVersion {
V1(Version),
#[cynic(fallback)]
Unknown,
V1,
}

impl TryFrom<ScriptParameters> for fuel_core_types::fuel_tx::ScriptParameters {
type Error = ConversionError;

fn try_from(params: ScriptParameters) -> Result<Self, Self::Error> {
match params.version {
ScriptParametersVersion::V1(_) => Ok(
ScriptParametersVersion::V1 => Ok(
fuel_core_types::fuel_tx::consensus_parameters::ScriptParametersV1 {
max_script_length: params.max_script_length.into(),
max_script_data_length: params.max_script_data_length.into(),
}
.into(),
),
ScriptParametersVersion::Unknown => {
Err(ConversionError::UnknownVariant("ScriptParametersVersion"))
}
}
}
}
Expand All @@ -165,29 +141,24 @@ pub struct ContractParameters {
pub max_storage_slots: U64,
}

#[derive(cynic::InlineFragments, Clone, Debug)]
#[derive(cynic::Enum, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub enum ContractParametersVersion {
V1(Version),
#[cynic(fallback)]
Unknown,
V1,
}

impl TryFrom<ContractParameters> for fuel_core_types::fuel_tx::ContractParameters {
type Error = ConversionError;

fn try_from(params: ContractParameters) -> Result<Self, Self::Error> {
match params.version {
ContractParametersVersion::V1(_) => Ok(
ContractParametersVersion::V1 => Ok(
fuel_core_types::fuel_tx::consensus_parameters::ContractParametersV1 {
contract_max_size: params.contract_max_size.into(),
max_storage_slots: params.max_storage_slots.into(),
}
.into(),
),
ContractParametersVersion::Unknown => {
Err(ConversionError::UnknownVariant("ContractParametersVersion"))
}
}
}
}
Expand All @@ -200,29 +171,24 @@ pub struct FeeParameters {
pub gas_per_byte: U64,
}

#[derive(cynic::InlineFragments, Clone, Debug)]
#[derive(cynic::Enum, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub enum FeeParametersVersion {
V1(Version),
#[cynic(fallback)]
Unknown,
V1,
}

impl TryFrom<FeeParameters> for fuel_core_types::fuel_tx::FeeParameters {
type Error = ConversionError;

fn try_from(params: FeeParameters) -> Result<Self, Self::Error> {
match params.version {
FeeParametersVersion::V1(_) => Ok(
FeeParametersVersion::V1 => Ok(
fuel_core_types::fuel_tx::consensus_parameters::FeeParametersV1 {
gas_price_factor: params.gas_price_factor.into(),
gas_per_byte: params.gas_per_byte.into(),
}
.into(),
),
FeeParametersVersion::Unknown => {
Err(ConversionError::UnknownVariant("FeeParametersVersion"))
}
}
}
}
Expand Down Expand Up @@ -346,20 +312,18 @@ pub struct GasCosts {
pub new_storage_per_byte: U64,
}

#[derive(cynic::InlineFragments, Clone, Debug)]
#[derive(cynic::Enum, Clone, Debug)]
#[cynic(schema_path = "./assets/schema.sdl")]
pub enum GasCostsVersion {
V1(Version),
#[cynic(fallback)]
Unknown,
V1,
}

impl TryFrom<GasCosts> for fuel_core_types::fuel_tx::GasCosts {
type Error = ConversionError;

fn try_from(value: GasCosts) -> Result<Self, Self::Error> {
match value.version {
GasCostsVersion::V1(_) => {
GasCostsVersion::V1 => {
let values = fuel_core_types::fuel_tx::consensus_parameters::gas::GasCostsValuesV1 {
add: value.add.into(),
addi: value.addi.into(),
Expand Down Expand Up @@ -474,9 +438,6 @@ impl TryFrom<GasCosts> for fuel_core_types::fuel_tx::GasCosts {
};
Ok(fuel_core_types::fuel_tx::GasCosts::new(values.into()))
}
GasCostsVersion::Unknown => {
Err(ConversionError::UnknownVariant("GasCostsVersion"))
}
}
}
}
Expand Down Expand Up @@ -509,7 +470,7 @@ impl TryFrom<ConsensusParameters> for fuel_core_types::fuel_tx::ConsensusParamet

fn try_from(params: ConsensusParameters) -> Result<Self, Self::Error> {
match params.version {
ConsensusParametersVersion::V1(_) => Ok(
ConsensusParametersVersion::V1 => Ok(
fuel_core_types::fuel_tx::consensus_parameters::ConsensusParametersV1 {
tx_params: params.tx_params.try_into()?,
predicate_params: params.predicate_params.try_into()?,
Expand All @@ -524,9 +485,6 @@ impl TryFrom<ConsensusParameters> for fuel_core_types::fuel_tx::ConsensusParamet
}
.into(),
),
ConsensusParametersVersion::Unknown => Err(ConversionError::UnknownVariant(
"ConsensusParametersVersion",
)),
}
}
}
Expand Down
1 change: 0 additions & 1 deletion crates/client/src/client/schema/primitives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ macro_rules! number_scalar {
number_scalar!(U64, u64);
number_scalar!(U32, u32);
number_scalar!(U16, u16);
number_scalar!(U8, u8);

impl TryFrom<U64> for PanicInstruction {
type Error = ConversionError;
Expand Down
Loading

0 comments on commit f7bd84e

Please sign in to comment.