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

Replace fuel_core_client::client::types::ConsensusParameters with fuel_tx::ConsensusParameters #1312

Closed
sdankel opened this issue Aug 21, 2023 · 2 comments
Assignees

Comments

@sdankel
Copy link
Member

sdankel commented Aug 21, 2023

Currently there are three definitions of this type:

  • fuel_core_client::client::types::ConsensusParameters
  • fuel_core_types::fuel_tx::ConsensusParameters
  • schema::chain::ConsensusParameters

Some of the types are missing from and default impls, which makes it inconvenient to work with for unit testing in forc.

There is a way to go from client -> fuel-tx here
And a way from schema -> client here
But missing fuel-tx -> client

Brandon K:

Hmm that type in fuel-core-client shouldn't be needed now, this should just be the fuel-tx version: https://github.com/FuelLabs/fuel-core/blob/master/crates/client/src/client/types/chain_info.rs#L14

We should be able to just replace fuel_core_client::client::types::ConsensusParameters with fuel_core_types::fuel_tx::ConsensusParameters

Similarly, for NodeInfo

@bvrooman
Copy link
Contributor

bvrooman commented Aug 22, 2023

I am able to remove the client type ConesnsusParameters, and directly swap in the fuel_tx::ConsensusParameters type in instances where it was used.

However, I'm not finding an existing fuel_tx type for NodeInfo. Looking at the code again, it looks like this type is an API level concept, existing only at the GraphQL layer and client abstraction layer. It is returned by an API call, rather than used internally by the client. That said, it doesn't I can give it the same treatment as ConsensusParameters.

Maybe if you can provide some more concrete context as to what you need to achieve, I can look in that direction. Or if I'm just misunderstanding anything, please let me know!

bvrooman pushed a commit that referenced this issue Aug 22, 2023
Related: 
- #1312

Removes the `types::ConsensusParameters` type designed to duplicate the
existing `fuel_tx::ConsensusParameters` type. Instead, `ChainInfo` can
use the `fuel_tx` type directly.

---------

Co-authored-by: Green Baneling <[email protected]>
@xgreenx
Copy link
Collaborator

xgreenx commented Jun 14, 2024

Fixed by #1314

@xgreenx xgreenx closed this as completed Jun 14, 2024
crypto523 pushed a commit to crypto523/fuel-core that referenced this issue Oct 7, 2024
Related: 
- FuelLabs/fuel-core#1312

Removes the `types::ConsensusParameters` type designed to duplicate the
existing `fuel_tx::ConsensusParameters` type. Instead, `ChainInfo` can
use the `fuel_tx` type directly.

---------

Co-authored-by: Green Baneling <[email protected]>
sui319 added a commit to sui319/fuel-core that referenced this issue Feb 17, 2025
Related: 
- FuelLabs/fuel-core#1312

Removes the `types::ConsensusParameters` type designed to duplicate the
existing `fuel_tx::ConsensusParameters` type. Instead, `ChainInfo` can
use the `fuel_tx` type directly.

---------

Co-authored-by: Green Baneling <[email protected]>
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

No branches or pull requests

3 participants