Skip to content

trace_block: Support overwriting execute_block#9871

Merged
bkchr merged 10 commits intomasterfrom
bkchr-trace-block
Oct 10, 2025
Merged

trace_block: Support overwriting execute_block#9871
bkchr merged 10 commits intomasterfrom
bkchr-trace-block

Conversation

@bkchr
Copy link
Member

@bkchr bkchr commented Sep 29, 2025

This is required for example for parachains that require special extensions to be registered (e.g. ProofSizeExt) to succeed the block execution.

This pull request changes the signature of spawn_tasks which now requires a tracing_execute_block parameter. If your chain is a solochain, just set the parameter to None or overwrite it if you need any special handling. For parachain builders, this value can be set to cumulus_service::ParachainTracingExecuteBlock.

This is required for example for parachains that require special extensions to be registered (e.g. `ProofSizeExt`) to succeed the block execution.

This pull request changes the signature of `spawn_tasks` which now requires a `tracing_execute_block` parameter. If your chain is a solochain, just set the
parameter to `None` or overwrite it if you need any special handling. For parachain builders, this value can be set to `cumulus_service::ParachainTracingExecuteBlock`.
@bkchr bkchr requested a review from koute as a code owner September 29, 2025 20:52
@bkchr bkchr added the T0-node This PR/Issue is related to the topic “node”. label Sep 29, 2025
@bkchr bkchr requested a review from a team September 29, 2025 20:52
@bkchr
Copy link
Member Author

bkchr commented Sep 30, 2025

/cmd fmt

client: Arc<Client>,
executor: SubscriptionTaskExecutor,
_phantom: PhantomData<(BE, Block)>,
block_execute: Option<Arc<dyn TracingExecuteBlock<Block>>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please name it the same everywhere?

Suggested change
block_execute: Option<Arc<dyn TracingExecuteBlock<Block>>>,
execute_block: Option<Arc<dyn TracingExecuteBlock<Block>>>,

///
/// The execution should be done sync on the same thread, because the caller will register
/// special tracing collectors.
fn execute_block(&self, orig_hash: Block::Hash, block: Block) -> sp_blockchain::Result<()>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need the orig_hash as parameter here? As far as I see none of the implementations use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

The implementation in my magical branch will use it :) Also it is any way important to be able to load data based on the correct block hash.

@bkchr bkchr enabled auto-merge October 9, 2025 08:51
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/18370876116
Failed job name: build-templates-node

@bkchr bkchr changed the title trace_block: Support overwriting the execute_block trace_block: Support overwriting execute_block Oct 9, 2025
@bkchr
Copy link
Member Author

bkchr commented Oct 9, 2025

/cmd prdoc --bump major --audience node_dev

@bkchr bkchr added this pull request to the merge queue Oct 10, 2025
Merged via the queue into master with commit c61e227 Oct 10, 2025
245 of 247 checks passed
@bkchr bkchr deleted the bkchr-trace-block branch October 10, 2025 11:11
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
This is required for example for parachains that require special
extensions to be registered (e.g. `ProofSizeExt`) to succeed the block
execution.

This pull request changes the signature of `spawn_tasks` which now
requires a `tracing_execute_block` parameter. If your chain is a
solochain, just set the parameter to `None` or overwrite it if you need
any special handling. For parachain builders, this value can be set to
`cumulus_service::ParachainTracingExecuteBlock`.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
EgrPrty pushed a commit that referenced this pull request Nov 6, 2025
This is a backport of
#9871. Its not a 1:1
backport to avoid many of the breaking changes. This PR makes an attempt
to automatically detect parachains based on metadata and the presence of
the weight reclaim transaction extension.

It is not perfect, but should work well for all the production chains.
In the non-backported versions proof recording is properly enabled, so
this whole mechanism is temporary.

---------

Co-authored-by: Egor_P <egor@parity.io>
EgrPrty pushed a commit that referenced this pull request Nov 10, 2025
This is a backport of #9871. Its not a 1:1 backport to avoid many of the
breaking changes. This PR makes an attempt to automatically detect
parachains based on metadata and the presence of the weight reclaim
transaction extension.

It is not perfect, but should work well for all the production chains.
In the non-backported versions proof recording is properly enabled, so
this whole mechanism is temporary.

After this is approved, will backport to more versions.

---------

Co-authored-by: Egor_P <egor@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T0-node This PR/Issue is related to the topic “node”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants