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.

Sign up for GitHub

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

Jump to bottom

Split initial state from chain config #1459

Merged
merged 51 commits into from
Dec 20, 2023
Merged

Split initial state from chain config #1459

merged 51 commits into from
Dec 20, 2023

Conversation

MujkicA
Copy link
Contributor

@MujkicA MujkicA commented Oct 27, 2023 •
edited by xgreenx
Loading

Closes #1450

Separates the loading of the initial chain state from loading ChainConfig.

The parameters to the run command have been renamed to chain_params and chain_state. Should we rename the corresponding structs ChainConfig and StateConfig accordingly?
The block height is moved to the ChainConfig to be available instantly because the StateConfig will be streamed in a later PR.

@MujkicA MujkicA added the enhancement New feature or request label Oct 27, 2023
@MujkicA MujkicA self-assigned this Oct 27, 2023
@MujkicA MujkicA changed the base branch from master to feature/regenesis-support October 31, 2023 14:01
@MujkicA MujkicA changed the base branch from feature/regenesis-support to master October 31, 2023 14:16
@MujkicA MujkicA changed the base branch from master to feature/regenesis-support October 31, 2023 14:20
@MujkicA MujkicA marked this pull request as ready for review October 31, 2023 15:22
@MujkicA MujkicA requested review from bvrooman and xgreenx and removed request for bvrooman October 31, 2023 15:22
MitchTurner
MitchTurner approved these changes Nov 15, 2023
View reviewed changes
Copy link
Member

@MitchTurner MitchTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I feel like I'm lacking a little context in the goals of the overall feature and the followup work, but I'm happy with everything here.

xgreenx
xgreenx reviewed Nov 29, 2023
View reviewed changes
Copy link
Collaborator

@xgreenx xgreenx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, looks definitely much better than everything in one structure!=)

Small comments that should be easy to resolve and we can merge

bin/fuel-core/src/cli/run.rs Outdated Show resolved Hide resolved
bin/fuel-core/src/cli/snapshot.rs Outdated Show resolved Hide resolved
crates/chain-config/src/config/chain.rs Outdated Show resolved Hide resolved
deployment/scripts/chainspec/beta_chainstate_spec.json Outdated Show resolved Hide resolved
tests/tests/deployment.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/service/config.rs Outdated Show resolved Hide resolved
@MujkicA MujkicA requested a review from xgreenx November 30, 2023 13:13
@segfault-magnet segfault-magnet mentioned this pull request Dec 1, 2023
Merged
xgreenx
xgreenx approved these changes Dec 6, 2023
View reviewed changes
tests/tests/deployment.rs Outdated Show resolved Hide resolved
tests/tests/deployment.rs Outdated Show resolved Hide resolved
bvrooman
bvrooman reviewed Dec 6, 2023
View reviewed changes
crates/chain-config/src/config/chain.rs Outdated Show resolved Hide resolved
crates/chain-config/src/config/state.rs Outdated Show resolved Hide resolved
crates/fuel-core/src/service/genesis.rs Outdated Show resolved Hide resolved
MujkicA and others added 4 commits December 7, 2023 15:16
@MujkicA
Update crates/chain-config/src/config/state.rs
5b33aa1 
Co-authored-by: Brandon Vrooman <brandon.vrooman@fuel.sh>
@MujkicA
Update crates/chain-config/src/config/chain.rs
0e25780 
Co-authored-by: Brandon Vrooman <brandon.vrooman@fuel.sh>
@MujkicA
add checked add and clippy allow
f90bbb5
@MujkicA
Merge branch 'feature/split-state-from-config' of github.com:FuelLabs…
6764f5d 
…/fuel-core into feature/split-state-from-config
bvrooman
bvrooman approved these changes Dec 7, 2023
View reviewed changes
Copy link
Contributor

@bvrooman bvrooman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together and addressing all the feedback!

xgreenx
xgreenx approved these changes Dec 19, 2023
View reviewed changes
benches/benches/vm_initialization.rs Outdated Show resolved Hide resolved
@MujkicA MujkicA merged commit 1ff2a7b into feature/regenesis-support Dec 20, 2023
31 checks passed
@MujkicA MujkicA deleted the feature/split-state-from-config branch December 20, 2023 15:23
@xgreenx xgreenx mentioned this pull request Jan 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@segfault-magnet segfault-magnet segfault-magnet left review comments

@xgreenx xgreenx xgreenx approved these changes

@MitchTurner MitchTurner MitchTurner approved these changes

@bvrooman bvrooman bvrooman approved these changes

@Voxelot Voxelot Awaiting requested review from Voxelot

Assignees

@MujkicA MujkicA

@segfault-magnet segfault-magnet

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extract the initial_state from the ChainConfig
7 participants
@MujkicA @Voxelot @bvrooman @MitchTurner @xgreenx @segfault-magnet @Salka1988