Skip to content
This repository has been archived by the owner on Aug 2, 2024. It is now read-only.

Refactor non-default sealing #1189

Merged
merged 10 commits into from
Oct 18, 2023
Merged

Refactor non-default sealing #1189

merged 10 commits into from
Oct 18, 2023

Conversation

kalaninja
Copy link
Contributor

@kalaninja kalaninja commented Oct 15, 2023

  • fix(service): confusing message when node starts (output the actual sealing method being used)
  • refactor: how the sealing mode is passed into runtime
  • feat: finalization for instant sealing

New sealing mode instant-finality is added to the cli (kudos to @tdelabro ) for command parsing:

enum Sealing {
  Manual,
  Instant,
  InstantFinality,
}

Internally sealing mode is represented with the following domain model:

enum SealingMode {
    #[default]
    Default,
    Manual,
    Instant {
        finalize: bool,
    },
}

let (command_sink, commands_stream) = mpsc::channel(1000); is adjusted to create for SealingMode::Manual only.

Closes #1144

@d-roak
Copy link
Collaborator

d-roak commented Oct 16, 2023

Hey @kalaninja, thanks ser !
Can u just had an entry to the changelog pls

@kalaninja
Copy link
Contributor Author

kalaninja commented Oct 16, 2023

@tdelabro @d-roak

NOTE: By default substrate doesn't finalize blocks with instant sealing. It is hardcoded
I can implement a workaround if needed, but I don't know what behavior is needed (if needed):

  • always finalize
  • add cli switch, e.g. --sealing=instant --finalize=true

crates/node/src/service.rs Outdated Show resolved Hide resolved
@kalaninja
Copy link
Contributor Author

As discussed with @d-roak I've added support for block finalization when instant sealing is used.

@kalaninja kalaninja requested a review from tdelabro October 17, 2023 08:44
crates/node/src/commands/run.rs Outdated Show resolved Hide resolved
crates/node/src/commands/run.rs Outdated Show resolved Hide resolved
@kalaninja kalaninja requested a review from tdelabro October 17, 2023 14:38
crates/node/src/service.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@tdelabro tdelabro left a comment

Choose a reason for hiding this comment

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

Good for me as soon as you address oak last comment

@tdelabro tdelabro merged commit 490caca into keep-starknet-strange:main Oct 18, 2023
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: allow dev nodes to use instant seal
3 participants