Hold till after release -- Max proof size from relay, get Impl for max weight, int tests env hos… #1134
Conversation
thea-leake
force-pushed
the
pull_max_proof_size_from_relay
branch
from
429dc38
to
446feb6
Compare
|
@thea-leake did you agree with Miguel about leaving this out firstly? Should we close this PR then? |
|
@mustermeiszer yeah I agreed with Miguel to leave the PR out for the release. I think we still want it pulled in, we just wanted to make sure it wasn't merged before the release PR was merged. |
thea-leake
force-pushed
the
pull_max_proof_size_from_relay
branch
2 times, most recently
from
ac3f323
to
b8a4bb8
Compare
…ties env errors when get impls moved out of specific runtime. moved weight get impls to runtime common
…et around weight generation outside of runtime module using relay chain value not having access to externalities provided env.
thea-leake
force-pushed
the
pull_max_proof_size_from_relay
branch
from
b8a4bb8
to
2452894
Compare
|
This has been refactored to have the weight param generation with proof size based off the relay chain in one place for all the runtimes. Currently using a Macro as seperating these out into a seperate module usable by the different runtimes Curently have Runtime generic as we would need this to switch back from macro, though we could technically |
|
@mikiquantum would we want this in review now, or should it wait until the other benchmarking work is done? |
|
Thanks @thea-leake , I would keep it as draft until we cut the release in the next few weeks. |
|
@mikiquantum sounds good, thanks :) |
thea-leake
changed the title
|
Do we know what's the state of this? |
|
We can bring this is IMO. |
| Runtime: cumulus_pallet_parachain_system::Config, | ||
| { | ||
| fn get() -> frame_support::weights::Weight { | ||
| if cfg!(test) { |
There was a problem hiding this comment.
As a note:
I guess the need for the macro usage was caused by the fact that only the compiled crate is compiling with test. So if you compile i.e. development-runtime with test then runtime-common (where this lives) will not compile with test.
I think we can avoid to use the macro if we check against std instead.
Another option is to get this value through a function where the cfg(test) value is passed by param.
| @@ -837,3 +841,41 @@ pub fn pass_n(env: &mut TestEnv, n: u64) -> Result<(), ()> { | |||
|
|
|||
| Ok(()) | |||
| } | |||
|
|
|||
| fn default_parachains_host_configuration() -> HostConfiguration<BlockNumber> { | |||
There was a problem hiding this comment.
Wondering whether we cannot get this from the relay chain spec which we use. This configuration is assumed to change over time.
Set max block weight proof size from relay chain val, and gen runtime weights with this val.
Description
Sets the proof size for the max block weight based off the value returned from the relay chain, and if that's not available, then use the value set in cumulus (pulled from polkadot).
Changes and Descriptions
Added gen_weight_parameters macro that generates weight params (Get impl) for Runtimes with the max proof size pulled from the relay if in an externalities provided
environment, and the max proof size for relay chains as defined by polkadot used if not.
Provides:
Updates runtimes to use new params generated with relay proof size.
Currently using a Macro as separating these out into a separate module usable by the different runtimes
causes issues with the
__construct_runtime_integrity_test::runtime_integrity_teststest for runtime/developmentwith the cfg!(test) macro returning false when this is in separate module, and then attempted data fetch relay chain with no externalities provided... which panics. Separate module does work for tests with provided externalities env & execute_with though.
Type of change
How Has This Been Tested?
Integration tests/unit tests
Checklist:
mainbranch