Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unknown transaction variants gracefully (consensus params, tx types, block headers) #1477

Open
2 tasks
Voxelot opened this issue Aug 7, 2024 · 1 comment · May be fixed by #1499
Open
2 tasks

Handle unknown transaction variants gracefully (consensus params, tx types, block headers) #1477

Voxelot opened this issue Aug 7, 2024 · 1 comment · May be fixed by #1499
Assignees

Comments

@Voxelot
Copy link
Member

Voxelot commented Aug 7, 2024

There are numerous areas of the transaction format that are extendable / upgradeable. Without proper handling, any new types added onto the spec could cause total failure for SDK users on older versions.

Nonexhaustive list of additive changes that could break existing SDK versions:

  • New transactional variants (i.e. ScriptV2, CreateV2)
  • New input or output types (i.e CoinInputV2)
  • New opcodes w/ brand new gas price in the consensus params

Without handling for these new and unknown types in the SDKs, a purely additive network change could instantly break apps like:

  • Explorers
  • Third party indexers (i.e. envio)
  • Wallets (i.e. account history)

On the Rust side, this likely requires something along the lines of a default unknown enum variant such as:

pub enum Transactions {
    Script(...)
    Create(...)
    Mint(...)
    Upload(...)
    Upgrade(...)
    Blob(...)
    #[serde(untagged)]
    Unknown // untagged variants are used to capture variants missing from the current enum definition during deserialization
}

TODO:

  • Rust SDK doesn't throw unexpected errors when new types are added to the node API
  • TS SDK doesn't throw unexpected errors when new types are added to the node API
@arboleya
Copy link
Member

@Voxelot Is the idea here to ignore unsupported items and print a warning?

  1. Someone asks for a list of 10x TXs
  2. This list contain 1x unsupported TX (CreateV2)
  3. SDK print warning about the unsupported item
  4. SDK moves on and returns 9x TXs

hal3e added a commit to FuelLabs/fuel-core that referenced this issue Nov 19, 2024
…2154)

## Linked Issues/PRs
FuelLabs/fuels-rs#1477

## Description
Refactored code to handle `Unknown` transaction types,
`ConsensusParameters` and `Blocks`.

For transactions I have craeted a new `TransactionType` that we can
`try_into` the `fuel_tx::Transaction`.
For `ConsesnsuParameters` and `Block` I have used `cynic's` `fallback`
attrbute and added a `Unknown` variant for all `Version` enums.

I had to bump `cynic` to `3.1.0` and `reqwest` to `0.12.0` to use the
`fallback` attribute on enums.

BREAKING CHANGES:
`graphql` endpoints for transactions use the new `TransactionType` type
instead of `fuel_tx::Transaction`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants