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

Integrate block production and the new trigger system #712

Merged
merged 22 commits into from
Oct 24, 2022
Merged

Integrate block production and the new trigger system #712

merged 22 commits into from
Oct 24, 2022

Conversation

Dentosal
Copy link
Member

Closes #677

@Dentosal Dentosal self-assigned this Oct 17, 2022
@Dentosal Dentosal force-pushed the block-production-trigger branch 2 times, most recently from 2139680 to 50d0c8a Compare October 17, 2022 14:29
Voxelot
Voxelot reviewed Oct 19, 2022
View reviewed changes
fuel-core/src/database.rs Outdated Show resolved Hide resolved
Voxelot
Voxelot reviewed Oct 19, 2022
View reviewed changes
fuel-core/src/schema/tx.rs Outdated Show resolved Hide resolved
@Dentosal Dentosal force-pushed the block-production-trigger branch from 50d0c8a to 98c06cb Compare October 20, 2022 12:59
@Voxelot Voxelot marked this pull request as ready for review October 21, 2022 18:37
@Voxelot Voxelot self-assigned this Oct 21, 2022
@Voxelot
Copy link
Member

Voxelot commented Oct 21, 2022

Although I could approve and merge this right now, since both @Dentosal and I worked on this PR we should get some other reviews.

xgreenx
xgreenx previously approved these changes Oct 22, 2022
View reviewed changes
xgreenx
xgreenx reviewed Oct 22, 2022
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.

The change looks good!=)
I'm not sure why we need this line:

        // cleanup unfinalized headers (block height + time + producer)
        block_db_transaction
            .deref_mut()
            .storage::<FuelBlocks>()
            .remove(&Bytes32::zeroed())?;

And maybe we need to rename produce_block into produce_block_and_commit or produce_block_and_execute to make it clear.

Do we want to move the commit of the block into a separate step in the future?

@xgreenx
Removed redundant consensus_params from TxPoolConfig.
edd9f42 
Fixed `block_producer` test for futures testing to use right config in the TxPool.
@Voxelot
Copy link
Member

Voxelot commented Oct 24, 2022

The change looks good!=) I'm not sure why we need this line:

        // cleanup unfinalized headers (block height + time + producer)
        block_db_transaction
            .deref_mut()
            .storage::<FuelBlocks>()
            .remove(&Bytes32::zeroed())?;

And maybe we need to rename produce_block into produce_block_and_commit or produce_block_and_execute to make it clear.

Do we want to move the commit of the block into a separate step in the future?

Yeah this is kind of ugly, but we need a way to reference the current block during execution for some of the VM opcodes to be able to fetch the current timestamp etc. Since the block id isn't known until execution has completed, it is temporarily stored with a block id of 0x00 for the duration of the execution and then unset at the end.

@Voxelot Voxelot merged commit 5c0e26a into master Oct 24, 2022
@Voxelot Voxelot deleted the block-production-trigger branch October 24, 2022 04:12
MujkicA pushed a commit that referenced this pull request Oct 25, 2022
@Dentosal @Voxelot
@xgreenx @MujkicA
Integrate block production and the new trigger system (#712)
a83268d 
* WIP: Use the new trigger instead of manual GQL block production

* Manually merge from Voxelots multi-tx-blocks branch

* Merge multi-tx-blocks branch

* Fix test in CI

* Fix all tests with `submit`

* working through txpool test failures

* fix more tests, move away from setup_tx

* investigate why dependency results are different

* mostly works

* some cleanup

* add missing gas prices to fix last test

* fix clippy

* Fix test

* Fix build

* goodbye submit_txs

* eliminate built-in timeout, let caller use their own timeout mechanism

* warnings cleanup

* Enable test `block_producer`

* Minor changes to improve naming=)

* Removed redundant `consensus_params` from `TxPoolConfig`.
Fixed `block_producer` test for futures testing to use right config in the TxPool.

* Fix "cargo make check --locked"

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: green <xgreenx9999@gmail.com>
MujkicA pushed a commit that referenced this pull request Oct 26, 2022
@Dentosal @Voxelot
@xgreenx @MujkicA
Integrate block production and the new trigger system (#712)
4669b4a 
* WIP: Use the new trigger instead of manual GQL block production

* Manually merge from Voxelots multi-tx-blocks branch

* Merge multi-tx-blocks branch

* Fix test in CI

* Fix all tests with `submit`

* working through txpool test failures

* fix more tests, move away from setup_tx

* investigate why dependency results are different

* mostly works

* some cleanup

* add missing gas prices to fix last test

* fix clippy

* Fix test

* Fix build

* goodbye submit_txs

* eliminate built-in timeout, let caller use their own timeout mechanism

* warnings cleanup

* Enable test `block_producer`

* Minor changes to improve naming=)

* Removed redundant `consensus_params` from `TxPoolConfig`.
Fixed `block_producer` test for futures testing to use right config in the TxPool.

* Fix "cargo make check --locked"

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: green <xgreenx9999@gmail.com>
MujkicA pushed a commit that referenced this pull request Oct 26, 2022
@Dentosal @Voxelot
@xgreenx @MujkicA
Integrate block production and the new trigger system (#712)
73c6e51 
* WIP: Use the new trigger instead of manual GQL block production

* Manually merge from Voxelots multi-tx-blocks branch

* Merge multi-tx-blocks branch

* Fix test in CI

* Fix all tests with `submit`

* working through txpool test failures

* fix more tests, move away from setup_tx

* investigate why dependency results are different

* mostly works

* some cleanup

* add missing gas prices to fix last test

* fix clippy

* Fix test

* Fix build

* goodbye submit_txs

* eliminate built-in timeout, let caller use their own timeout mechanism

* warnings cleanup

* Enable test `block_producer`

* Minor changes to improve naming=)

* Removed redundant `consensus_params` from `TxPoolConfig`.
Fixed `block_producer` test for futures testing to use right config in the TxPool.

* Fix "cargo make check --locked"

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: green <xgreenx9999@gmail.com>
MujkicA pushed a commit that referenced this pull request Oct 26, 2022
@Dentosal @Voxelot
@xgreenx @MujkicA
Integrate block production and the new trigger system (#712)
2b3daa9 
* WIP: Use the new trigger instead of manual GQL block production

* Manually merge from Voxelots multi-tx-blocks branch

* Merge multi-tx-blocks branch

* Fix test in CI

* Fix all tests with `submit`

* working through txpool test failures

* fix more tests, move away from setup_tx

* investigate why dependency results are different

* mostly works

* some cleanup

* add missing gas prices to fix last test

* fix clippy

* Fix test

* Fix build

* goodbye submit_txs

* eliminate built-in timeout, let caller use their own timeout mechanism

* warnings cleanup

* Enable test `block_producer`

* Minor changes to improve naming=)

* Removed redundant `consensus_params` from `TxPoolConfig`.
Fixed `block_producer` test for futures testing to use right config in the TxPool.

* Fix "cargo make check --locked"

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: green <xgreenx9999@gmail.com>
MujkicA pushed a commit that referenced this pull request Oct 26, 2022
@Dentosal @Voxelot
@xgreenx @MujkicA
Integrate block production and the new trigger system (#712)
1134a00 
* WIP: Use the new trigger instead of manual GQL block production

* Manually merge from Voxelots multi-tx-blocks branch

* Merge multi-tx-blocks branch

* Fix test in CI

* Fix all tests with `submit`

* working through txpool test failures

* fix more tests, move away from setup_tx

* investigate why dependency results are different

* mostly works

* some cleanup

* add missing gas prices to fix last test

* fix clippy

* Fix test

* Fix build

* goodbye submit_txs

* eliminate built-in timeout, let caller use their own timeout mechanism

* warnings cleanup

* Enable test `block_producer`

* Minor changes to improve naming=)

* Removed redundant `consensus_params` from `TxPoolConfig`.
Fixed `block_producer` test for futures testing to use right config in the TxPool.

* Fix "cargo make check --locked"

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: green <xgreenx9999@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Voxelot Voxelot Voxelot left review comments

@xgreenx xgreenx xgreenx approved these changes

@ControlCplusControlV ControlCplusControlV Awaiting requested review from ControlCplusControlV

Assignees

@Voxelot Voxelot

@Dentosal Dentosal

Labels
fuel-block-producer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integrate block trigger with block producer, and remove graphql driven production
3 participants
@Dentosal @Voxelot @xgreenx