Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Graphite Automations"Auto-assign PRs to author" took an action on this PR • (03/14/25)1 assignee was added to this PR based on Juan Ignacio Rios's automation. |
a021c92 to
e07c1a9
Compare
e07c1a9 to
cbc803c
Compare
0ca5002 to
891799a
Compare
cbc803c to
4317213
Compare
891799a to
d116e92
Compare
4317213 to
8c9017b
Compare
d116e92 to
2828880
Compare
8c9017b to
d1e02ed
Compare
lrazovic
left a comment
There was a problem hiding this comment.
Just 2 nits, but the approach seems straightforward!
|
|
||
| #[frame_support::pallet] | ||
| pub mod pallet { | ||
| use super::BlockNumberFor; |
There was a problem hiding this comment.
Isn't this also part of the use super::*; two lines below?
There was a problem hiding this comment.
ah, that's because the type is ambiguous here. there is BlockNumberFor in the frame_system::pallet_prelude::* as well
d1e02ed to
699d4da
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR updates several pallets and related tests to use a unified notion of block number via the new BlockNumberProvider type, enabling more reliable async backing.
- Replaces direct calls to frame_system’s block_number with BlockNumberProvider::current_block_number.
- Adds BlockNumberProvider implementations and configurations in TestRuntime, mocks, and integration tests.
- Updates Cargo.toml dependencies to include necessary cumulus pallets to support asynchronous backing.
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pallets/funding/src/mock.rs | Updated system config and added ParachainSystem configuration. |
| pallets/funding/src/lib.rs | Replaced frame_system block_number calls with BlockNumberProvider calls. |
| pallets/funding/src/instantiator/* | Updated type bounds and block number calls to incorporate BlockNumberProvider. |
| pallets/funding/src/functions/* | Uniformly replaced block_number retrieval methods. |
| pallets/democracy/* | Switched to using BlockNumberProvider in multiple block number checks. |
| integration-tests/* | Adjusted integration tests to reference the relay chain’s block number provider. |
Comments suppressed due to low confidence (2)
pallets/funding/src/instantiator/chain_interactions.rs:177
- [nitpick] This workaround uses frame_system::Pallet's set_block_number instead of a corresponding method from BlockNumberProvider. Consider unifying block number setting under BlockNumberProvider if a proper method becomes available.
self.execute(|| { frame_system::Pallet::<T>::set_block_number(block - One::one()); });
integration-tests/src/tests/e2e.rs:586
- [nitpick] The change from PolimecSystem to PolkadotSystem for retrieving the block number may lead to inconsistencies. Verify that using PolkadotSystem here is intentional and consistent with the overall use of BlockNumberProvider in production code.
let now = PolkadotSystem::block_number();
699d4da to
f4e4f84
Compare
e205664 to
fc38ace
Compare
f4e4f84 to
e8ccde4
Compare
fc38ace to
51c4399
Compare
6ab4ec1 to
e087f8f
Compare
e087f8f to
b207098
Compare

What?
Introduces
BlockNumberProvidertype for palletsWhy?
So that we can use a more reliable, consistent notion of block number, since with async backing system block number becomes nondeterministic
How?
Relay chain's block number is used where possible, i.e funding, linear-release, and we rely on system block number in other cases
Testing?
Changes were made to
Instantiatorso that it works for both integration and unit tests, since now integration tests Runtime uses relay chain's block number instead of chain'sScreenshots (optional)
Anything Else?
#469