Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Manual Seal #4143

Merged
merged 5 commits into from
Jan 27, 2020
Merged

Manual Seal #4143

merged 5 commits into from
Jan 27, 2020

Conversation

seunlanlege
Copy link
Contributor

@seunlanlege seunlanlege commented Nov 19, 2019 •
edited
Loading

Manual Seal is a block production engine for single-node environments for testing.

It provides two RPC methods engine_createBlock and engine_finalizeBlock for creating and finalizing blocks respectively.

Alongside manual seal is instant seal, which creates blocks as soon as new transactions are available

@parity-cla-bot
Copy link

It looks like @seunlanlege signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@seunlanlege seunlanlege added A3-in_progress Pull request is in progress. No review needed at this stage. J0-enhancement An additional feature request. M4-core labels Nov 19, 2019
@gavofyork gavofyork removed the J0-enhancement An additional feature request. label Nov 19, 2019
@kianenigma
Copy link
Contributor

Interesting; is this mostly used for testing then? can you elaborate a bit more?

@jam10o-new
Copy link

If this is based on what we have on parity-ethereum, yes @kianenigma - (a --dev chain on parity-ethereum uses the nstantSeal engine, which is intended for local-only debugging)

@tomusdrw
Copy link
Contributor

tomusdrw commented Nov 20, 2019 •
edited
Loading

Rather than having InstantSeal engine I think it would be better to have an engine that is manually controlled. InstantSeal can then build on top of this, but also we can just expose the engine functions over RPC. More control allows more sophisticated testing (especially if we build a simple UI on top) and might be super useful in the future to do some simulations. For instance, you might want to create a block with 3 transactions, then a bunch of empty blocks and then yet another one with the transaction that just got in.

Imagine if you could take an existing chain, locally swap consensus to "manual" and then check what result your transaction would have, how it changes over time (empty blocks being produced), etc. Using just the same tools that you use already (polkadot apps).

@seunlanlege seunlanlege changed the title Instant Seal Manual Seal Nov 21, 2019
tomusdrw
tomusdrw reviewed Nov 21, 2019
View reviewed changes
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good, feels like you are on the right track.

client/consensus/manual-seal/src/lib.rs Show resolved Hide resolved
client/consensus/manual-seal/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/rpc.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/rpc.rs Outdated Show resolved Hide resolved
@seunlanlege seunlanlege force-pushed the seun-instant-seal branch 2 times, most recently from 7427e34 to dc2a43c Compare November 27, 2019 21:43
@seunlanlege seunlanlege force-pushed the seun-instant-seal branch 3 times, most recently from 82fc7dc to 7e25fd6 Compare December 17, 2019 00:54
@seunlanlege seunlanlege added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Dec 17, 2019
@seunlanlege
Copy link
Contributor Author

@tomusdrw 👋 this is ready

tomusdrw
tomusdrw reviewed Dec 18, 2019
View reviewed changes
client/consensus/manual-seal/Cargo.toml Outdated Show resolved Hide resolved
client/consensus/manual-seal/Cargo.toml Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/error.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/error.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/finalize_block.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/seal_new_block.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/seal_new_block.rs Outdated Show resolved Hide resolved
client/transaction-pool/Cargo.toml Outdated Show resolved Hide resolved
client/transaction-pool/Cargo.toml Outdated Show resolved Hide resolved
client/transaction-pool/Cargo.toml Outdated Show resolved Hide resolved
This was referenced Dec 18, 2019
Closed
Closed
@bkchr bkchr mentioned this pull request Jan 6, 2020
Closed
5 tasks
@seunlanlege
Copy link
Contributor Author

SYN @tomusdrw

tomusdrw
tomusdrw reviewed Jan 7, 2020
View reviewed changes
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

lgtm! But still a few minor grumbles.

client/consensus/manual-seal/src/error.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/rpc.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/rpc.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/seal_new_block.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/seal_new_block.rs Outdated Show resolved Hide resolved
@seunlanlege seunlanlege force-pushed the seun-instant-seal branch 2 times, most recently from a066f55 to 7cf1e65 Compare January 8, 2020 11:19
@JoshOrndorff JoshOrndorff mentioned this pull request Jan 8, 2020
Closed
@seunlanlege seunlanlege force-pushed the seun-instant-seal branch 2 times, most recently from 7db9b88 to a85802e Compare January 22, 2020 09:12
tomusdrw
tomusdrw reviewed Jan 24, 2020
View reviewed changes
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good already, few tiny things still though.

client/consensus/manual-seal/Cargo.toml Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/error.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/finalize_block.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/seal_new_block.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/seal_new_block.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/seal_new_block.rs Outdated Show resolved Hide resolved
client/consensus/manual-seal/src/seal_new_block.rs Outdated Show resolved Hide resolved
@seunlanlege
instant/manual seal
27a1a84 
unbounded queues are evil

Apply suggestions from code review

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>

add fork tests, docs, remove todos

moar docs

Update client/consensus/manual-seal/src/rpc.rs

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>

remove unbound generic, parameter, docs, deps, code style changes

Apply suggestions from code review

Co-Authored-By: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

code style chnges

remove unused deps, remove dep renames, check if block is empty before importing, use ? for error propagation

fix tests

log errors for instant seal

use debug

code style changes, updated copyright dates

use txpool::Pool instead of BasicPool, code style changes

fixed tests
@seunlanlege
fix tests
4ff2da9
@seunlanlege
requested changes from review
f18936c
@seunlanlege
check inherents len
4859a0c
@seunlanlege
rebase
150c08d
@seunlanlege
Copy link
Contributor Author

@tomusdrw done, 😅

tomusdrw
tomusdrw approved these changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

lgtm!

@seunlanlege seunlanlege merged commit 03ffa5a into master Jan 27, 2020
@seunlanlege seunlanlege deleted the seun-instant-seal branch January 27, 2020 09:59
@xlc
Copy link
Contributor

xlc commented Feb 14, 2020 •
edited
Loading

Is this been integrated anywhere? I want to find example of how to use this but found none.

@4meta5 4meta5 mentioned this pull request Mar 21, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rphmeier rphmeier rphmeier left review comments

@tomusdrw tomusdrw tomusdrw approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@seunlanlege @parity-cla-bot @kianenigma @jam10o-new @tomusdrw @rphmeier @xlc @gavofyork