Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion cumulus/polkadot-omni-node/lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

//! CLI options of the omni-node. See [`Command`].

/// Default block time for dev mode when using `--dev` flag.
const DEFAULT_DEV_BLOCK_TIME_MS: u64 = 3000;

use crate::{
chain_spec::DiskChainSpecLoader,
common::{
Expand Down Expand Up @@ -163,9 +166,19 @@ pub struct Cli<Config: CliConfig> {
///
/// The `--dev` flag sets the `dev_block_time` to a default value of 3000ms unless explicitly
/// provided.
#[arg(long)]
#[arg(long, conflicts_with = "instant_seal")]
pub dev_block_time: Option<u64>,

/// Start a dev node with instant seal.
///
/// This is a dev option that enables instant sealing, meaning blocks are produced
/// immediately when transactions are received, rather than at fixed intervals.
/// Using this option won't result in starting or connecting to a parachain network.
/// The resulting node will work on its own, running the wasm blob and producing blocks
/// instantly upon receiving transactions.
#[arg(long, conflicts_with = "dev_block_time")]
pub instant_seal: bool,

/// DEPRECATED: This feature has been stabilized, pLease use `--authoring slot-based` instead.
///
/// Use slot-based collator which can handle elastic scaling.
Expand Down Expand Up @@ -209,6 +222,16 @@ pub struct Cli<Config: CliConfig> {
pub(crate) _phantom: PhantomData<Config>,
}

/// Development sealing mode.
#[derive(Debug, Clone, Copy)]
pub enum DevSealMode {
/// Produces blocks immediately upon receiving transactions.
InstantSeal,
/// Produces blocks at fixed time intervals.
/// The u64 parameter represents the block time in milliseconds.
ManualSeal(u64),
}

/// Collator implementation to use.
#[derive(PartialEq, Debug, ValueEnum, Clone, Copy)]
pub enum AuthoringPolicy {
Expand Down Expand Up @@ -242,6 +265,19 @@ impl<Config: CliConfig> Cli<Config> {
enable_statement_store: self.enable_statement_store,
}
}

/// Returns the dev seal mode if the node is in dev mode.
pub fn dev_mode(&self) -> Option<DevSealMode> {
if self.instant_seal {
Some(DevSealMode::InstantSeal)
} else if let Some(dev_block_time) = self.dev_block_time {
Some(DevSealMode::ManualSeal(dev_block_time))
} else if self.run.base.is_dev() {
Some(DevSealMode::ManualSeal(DEFAULT_DEV_BLOCK_TIME_MS))
} else {
None
}
}
}

impl<Config: CliConfig> SubstrateCli for Cli<Config> {
Expand Down
17 changes: 3 additions & 14 deletions cumulus/polkadot-omni-node/lib/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,10 @@ use clap::{CommandFactory, FromArgMatches};
use cumulus_client_service::storage_proof_size::HostFunctions as ReclaimHostFunctions;
use frame_benchmarking_cli::{BenchmarkCmd, SUBSTRATE_REFERENCE_HARDWARE};
use log::info;
use sc_cli::{CliConfiguration, Result, SubstrateCli};
use sc_cli::{Result, SubstrateCli};
#[cfg(feature = "runtime-benchmarks")]
use sp_runtime::traits::HashingFor;

const DEFAULT_DEV_BLOCK_TIME_MS: u64 = 3000;

/// Structure that can be used in order to provide customizers for different functionalities of the
/// node binary that is being built using this library.
pub struct RunConfig {
Expand Down Expand Up @@ -303,17 +301,8 @@ where
let node_spec =
new_node_spec(&config, &cmd_config.runtime_resolver, &cli.node_extra_args())?;

if cli.run.base.is_dev()? {
let dev_block_time = cli.dev_block_time.unwrap_or(DEFAULT_DEV_BLOCK_TIME_MS);
return node_spec
.start_manual_seal_node(config, dev_block_time)
.map_err(Into::into);
}

if let Some(dev_block_time) = cli.dev_block_time {
return node_spec
.start_manual_seal_node(config, dev_block_time)
.map_err(Into::into);
if let Some(dev_mode) = cli.dev_mode() {
return node_spec.start_dev_node(config, dev_mode).map_err(Into::into);
}

// If Statemint (Statemine, Westmint, Rockmine) DB exists and we're using the
Expand Down
19 changes: 10 additions & 9 deletions cumulus/polkadot-omni-node/lib/src/common/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

use crate::{
chain_spec::Extensions,
cli::DevSealMode,
common::{
command::NodeCommandRunner,
rpc::BuildRpcExtensions,
Expand Down Expand Up @@ -300,11 +301,11 @@ pub(crate) trait NodeSpec: BaseNodeSpec {

const SYBIL_RESISTANCE: CollatorSybilResistance;

fn start_manual_seal_node(
fn start_dev_node(
_config: Configuration,
_block_time: u64,
_mode: DevSealMode,
) -> sc_service::error::Result<TaskManager> {
Err(sc_service::Error::Other("Manual seal not supported for this node type".into()))
Err(sc_service::Error::Other("Dev not supported for this node type".into()))
}

/// Start a node with the given parachain spec.
Expand Down Expand Up @@ -557,11 +558,11 @@ pub(crate) trait NodeSpec: BaseNodeSpec {
}

pub(crate) trait DynNodeSpec: NodeCommandRunner {
/// Start node with manual-seal consensus.
fn start_manual_seal_node(
/// Start node with manual or instant seal consensus.
fn start_dev_node(
self: Box<Self>,
config: Configuration,
block_time: u64,
mode: DevSealMode,
) -> sc_service::error::Result<TaskManager>;

/// Start the node.
Expand All @@ -579,12 +580,12 @@ impl<T> DynNodeSpec for T
where
T: NodeSpec + NodeCommandRunner,
{
fn start_manual_seal_node(
fn start_dev_node(
self: Box<Self>,
config: Configuration,
block_time: u64,
mode: DevSealMode,
) -> sc_service::error::Result<TaskManager> {
<Self as NodeSpec>::start_manual_seal_node(config, block_time)
<Self as NodeSpec>::start_dev_node(config, mode)
}

fn start_node(
Expand Down
114 changes: 67 additions & 47 deletions cumulus/polkadot-omni-node/lib/src/nodes/aura.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// limitations under the License.

use crate::{
cli::AuthoringPolicy,
cli::{AuthoringPolicy, DevSealMode},
common::{
aura::{AuraIdT, AuraRuntimeApi},
rpc::{BuildParachainRpcExtensions, BuildRpcExtensions},
Expand Down Expand Up @@ -211,9 +211,9 @@ where
type StartConsensus = StartConsensus;
const SYBIL_RESISTANCE: CollatorSybilResistance = CollatorSybilResistance::Resistant;

fn start_manual_seal_node(
fn start_dev_node(
mut config: Configuration,
block_time: u64,
mode: DevSealMode,
) -> sc_service::error::Result<TaskManager> {
let PartialComponents {
client,
Expand Down Expand Up @@ -277,24 +277,6 @@ where
None,
);

let (manual_seal_sink, manual_seal_stream) = futures::channel::mpsc::channel(1024);
let mut manual_seal_sink_clone = manual_seal_sink.clone();
task_manager
.spawn_essential_handle()
.spawn("block_authoring", None, async move {
loop {
futures_timer::Delay::new(std::time::Duration::from_millis(block_time)).await;
manual_seal_sink_clone
.try_send(sc_consensus_manual_seal::EngineCommand::SealNewBlock {
create_empty: true,
finalize: true,
parent_hash: None,
sender: None,
})
.unwrap();
}
});

// Note: Changing slot durations are currently not supported
let slot_duration = sc_consensus_aura::slot_duration(&*client)
.expect("slot_duration is always present; qed.");
Expand All @@ -305,29 +287,67 @@ where

let para_id =
Self::parachain_id(&client, &config).ok_or("Failed to retrieve the parachain id")?;
let create_inherent_data_providers = Self::create_manual_seal_inherent_data_providers(
client.clone(),
para_id,
slot_duration,
);

let params = sc_consensus_manual_seal::ManualSealParams {
block_import: client.clone(),
env: proposer,
client: client.clone(),
pool: transaction_pool.clone(),
select_chain: LongestChain::new(backend.clone()),
commands_stream: Box::pin(manual_seal_stream),
consensus_data_provider: Some(Box::new(aura_digest_provider)),
create_inherent_data_providers,
};

let authorship_future = sc_consensus_manual_seal::run_manual_seal(params);
task_manager.spawn_essential_handle().spawn_blocking(
"manual-seal",
None,
authorship_future,
);
let create_inherent_data_providers =
Self::create_dev_node_inherent_data_providers(client.clone(), para_id, slot_duration);

match mode {
DevSealMode::InstantSeal => {
let params = sc_consensus_manual_seal::InstantSealParams {
block_import: client.clone(),
env: proposer,
client: client.clone(),
pool: transaction_pool.clone(),
select_chain: LongestChain::new(backend.clone()),
consensus_data_provider: Some(Box::new(aura_digest_provider)),
create_inherent_data_providers,
};

let authorship_future = sc_consensus_manual_seal::run_instant_seal(params);
task_manager.spawn_essential_handle().spawn_blocking(
"instant-seal",
None,
authorship_future,
);
},
DevSealMode::ManualSeal(block_time) => {
let (manual_seal_sink, manual_seal_stream) = futures::channel::mpsc::channel(1024);
let mut manual_seal_sink_clone = manual_seal_sink.clone();
task_manager
.spawn_essential_handle()
.spawn("block_authoring", None, async move {
loop {
futures_timer::Delay::new(std::time::Duration::from_millis(block_time))
.await;
manual_seal_sink_clone
.try_send(sc_consensus_manual_seal::EngineCommand::SealNewBlock {
create_empty: true,
finalize: true,
parent_hash: None,
sender: None,
})
.unwrap();
}
});

let params = sc_consensus_manual_seal::ManualSealParams {
block_import: client.clone(),
env: proposer,
client: client.clone(),
pool: transaction_pool.clone(),
select_chain: LongestChain::new(backend.clone()),
commands_stream: Box::pin(manual_seal_stream),
consensus_data_provider: Some(Box::new(aura_digest_provider)),
create_inherent_data_providers,
};

let authorship_future = sc_consensus_manual_seal::run_manual_seal(params);
task_manager.spawn_essential_handle().spawn_blocking(
"manual-seal",
None,
authorship_future,
);
},
}

let rpc_extensions_builder = {
let client = client.clone();
Expand Down Expand Up @@ -373,11 +393,11 @@ where
RuntimeApi::RuntimeApi: AuraRuntimeApi<Block, AuraId>,
AuraId: AuraIdT + Sync,
{
/// Creates the inherent data providers for manual seal consensus.
/// Creates the inherent data providers for manual and instant seal consensus.
///
/// This function sets up the timestamp and parachain validation data providers
/// required for manual seal block production in a parachain environment.
fn create_manual_seal_inherent_data_providers(
/// required for dev seal block production in a parachain environment.
fn create_dev_node_inherent_data_providers(
client: Arc<ParachainClient<Block, RuntimeApi>>,
para_id: ParaId,
slot_duration: sp_consensus_aura::SlotDuration,
Expand Down
12 changes: 12 additions & 0 deletions prdoc/pr_10008.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
title: Add instant seal to omni-node
doc:
- audience: Runtime Dev
description: |-
Adds a new `--instant-seal` CLI flag to enable instant seal mode in omni-node. When this flag
is passed, blocks are produced immediately upon receiving transactions, rather than at fixed
intervals. This flag cannot be used together with `--dev-block-time`.

fixes https://github.com/paritytech/polkadot-sdk/issues/9996
crates:
- name: polkadot-omni-node-lib
bump: patch
2 changes: 1 addition & 1 deletion substrate/client/cli/src/commands/run_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ impl CliConfiguration for RunCmd {
}

fn rpc_addr(&self, default_listen_port: u16) -> Result<Option<Vec<RpcEndpoint>>> {
self.rpc_params.rpc_addr(self.is_dev()?, self.validator, default_listen_port)
self.rpc_params.rpc_addr(self.is_dev(), self.validator, default_listen_port)
}

fn rpc_methods(&self) -> Result<sc_service::config::RpcMethods> {
Expand Down
8 changes: 4 additions & 4 deletions substrate/client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
/// Returns `true` if the node is for development or not
///
/// By default this is retrieved from `SharedParams`.
fn is_dev(&self) -> Result<bool> {
Ok(self.shared_params().is_dev())
fn is_dev(&self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this unrequired change.

Copy link
Contributor Author

@pgherveou pgherveou Oct 15, 2025

Choose a reason for hiding this comment

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

I use is_dev() in this commit, this didnt make any sense, why does it have to return a result?

Copy link
Contributor

@iulianbarbu iulianbarbu Oct 21, 2025

Choose a reason for hiding this comment

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

We can probably unwrap_or(false) there instead to avoid doing changes here. I don't have a good example for why to return Result<bool> - the only thing that comes to mind is that some nodes might work in a development setup where nodes query memory or IO to find whether they should run in development (however convoluted this might seem), and a third state like Err could represent an internal error happened during the query, which is not equivalent with true or false (which represent the query result). Also, changing it could break users (in an unnecessary way IMO) and would be simpler (from a case perspective) to pack it with a bigger breaking change that's needed (if any).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am happy to rollback 7f73cfb if you can give me an explanation of why something that is always returning Ok(bool) should not instead just return bool

Copy link
Contributor

@iulianbarbu iulianbarbu Oct 21, 2025

Choose a reason for hiding this comment

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

It is true that "always returning Ok(bool)" is equivalent to just returning bool, but the method can be overridden to return also Err - given it is a trait fn. However, I also find it very unlikely that node developers would need the Err to be returned by is_dev - whatever they need to do based on that error can still happen inside is_dev override (mostly panicing). Returning the error though will give the flexibility to handle it outside is_dev too, which might be needed in case some outer scope variables that can not be passed to is_dev are needed (but the logic can also have this context on self, and made available in is_dev, but that enforces some constraints on implementations). I would personally not touch this for this PR, since it raises unrelated concerns, and maybe attack it via a different PR, but if we do it here, the prdoc should be updated accordingly.

self.shared_params().is_dev()
}

/// Gets the role
Expand Down Expand Up @@ -455,7 +455,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
/// By default this is retrieved from `NodeKeyParams` if it is available. Otherwise its
/// `NodeKeyConfig::default()`.
fn node_key(&self, net_config_dir: &PathBuf) -> Result<NodeKeyConfig> {
let is_dev = self.is_dev()?;
let is_dev = self.is_dev();
let role = self.role(is_dev)?;
self.node_key_params()
.map(|x| x.node_key(net_config_dir, role, is_dev))
Expand Down Expand Up @@ -489,7 +489,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
cli: &C,
tokio_handle: tokio::runtime::Handle,
) -> Result<Configuration> {
let is_dev = self.is_dev()?;
let is_dev = self.is_dev();
let chain_id = self.chain_id(is_dev)?;
let chain_spec = cli.load_spec(&chain_id)?;
let base_path = base_path_or_default(self.base_path()?, &C::executable_name());
Expand Down
Loading