Skip to content

Enable statement store in polkadot-omni-node and polkadot-parachain #8076

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.

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

gui1117
Copy link
Contributor

@gui1117 gui1117 commented Mar 28, 2025

We want to enable statement store for people chain.

This PR introduces and enables the statement store and add a new argument disable_statement_store in polkadot-omni-node-lib.
This takes effect in polkadot-omni-node and polkadot-parachain and any node depending on polkadot-omni-node-lib.

The reasoning is following other service such as offchain_worker the default behavior is to support it, otherwise I expect not many collator to explictly opt-in (but I can be wrong assuming this).
Do system chain collator use omni-node or polkadot-parachain? we could otherwise only enable it for polkadot-parachain. Or we could also create a new binary for people chain and enable it only for this new binary, and remove people chain from polkadot-parachain.

If the runtime doesn't support the API then it any submission will fail with Error calling into the runtime.

It would be ideal to target the next cut-off for this issue/PR given that it can take time for node operator to update their node.

To have a configurable CLI and default behavior for statement-store, I did in this PR: #8421

@gui1117 gui1117 force-pushed the gui-statement-store-in-omni-node branch from 64f012b to 92057c9 Compare April 25, 2025 11:24
@gui1117 gui1117 force-pushed the gui-statement-store-in-omni-node branch from 4acb5fe to cad2edc Compare April 28, 2025 07:28
@gui1117
Copy link
Contributor Author

gui1117 commented Apr 29, 2025

/cmd prdoc --audience node_dev --audience node_operator

@gui1117 gui1117 added the T0-node This PR/Issue is related to the topic “node”. label Apr 29, 2025
@gui1117 gui1117 marked this pull request as ready for review April 29, 2025 02:12
@gui1117 gui1117 changed the title [Draft] Add statement store in omni-node Enable statement store in polkadot-omni-node and polkadot-parachain Apr 29, 2025
@gui1117
Copy link
Contributor Author

gui1117 commented Apr 29, 2025

This is ready to be looked at

Copy link
Contributor

@georgepisaltu georgepisaltu left a comment

Choose a reason for hiding this comment

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

Looking at the code, I think we could get away with always enabling the statement store (i.e. not having it be an Option in the node spec) since it would just be inaccessible if the runtime API doesn't allow it, but I'm happy to move forward with this.

/// and OCW.
/// It uses the runtime api to get the allowance associated to an account.
#[arg(long)]
pub disable_statement_store: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

As per the offline discussion, should we disable the statement store by default ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we disable by default I fear nobody will enable it. But maybe it will be enough with the few that will enable it.

I feel going into a dedicated binary for people chain seems the better then no?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that having a dedicated binary for the people chain would create more complications. We would need to release one more binary, change all the tests that involve the rococo-people chain to use the dedicated binary, change the deployment process only for the people chain. I don't know.

Or maybe we should just enable the statement-store by default as it is done here. I'm not sure which solution is the best. I'm curious what others think as well.

cc: @skunert @iulianbarbu @seemantaggarwal

@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/14849365823
Failed job name: test-linux-stable

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.

3 participants