Skip to content
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

node: proposer for limiting blocks #156

Merged
merged 1 commit into from
Dec 31, 2023
Merged

node: proposer for limiting blocks #156

merged 1 commit into from
Dec 31, 2023

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Dec 27, 2023

This creates, but does not activate, a solution to #147.

The implementation differs from the proposed model in #147 because UpgradeGoAhead signals are accounted for directly, so there is no need for an intermittent heartbeat.

We cannot activate this yet because the fees logic needs to account for skipped blocks, which it currently is incapable of.

Copy link
Contributor Author

rphmeier commented Dec 27, 2023

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@rphmeier rphmeier marked this pull request as ready for review December 27, 2023 21:01
use sp_runtime::generic::Digest;

use cumulus_client_consensus_proposer::{Error as ProposerError, ProposerInterface};
use cumulus_pallet_parachain_system::relay_state_snapshot::RelayChainStateProof;
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like we might need to think to extract this. (Well, I guess by we I mean cumulus devs)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extract in what sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, sorry, I meant that RelayChainStateProof seems like a generally useful primitive and as such probably would be better placed in a primitives crate. That crate would be dependent upon by places such as this one and the cumulus_pallet_parachain_system.

} else {
Err(ProposerError::proposing(anyhow!(
"no need to create a block"
)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hopefully, this isn't going to be printed all the time, or is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is. Needs an upstream fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

pepyakin commented Dec 31, 2023

Merge activity

@pepyakin pepyakin merged commit 56de3b3 into main Dec 31, 2023
6 checks passed
@pepyakin pepyakin deleted the rh-limiting-proposer branch December 31, 2023 12:58
@seunlanlege
Copy link

Consider publishing this on crates.io or upstreaming to polkadot-sdk? This is a must-have for all runtimes post-coretime

@pepyakin
Copy link
Contributor

pepyakin commented Jan 2, 2024

Consider publishing this on crates.io or upstreaming to polkadot-sdk? This is a must-have for all runtimes post-coretime

Tracked as part of #163.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants