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.

Sign up for GitHub

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

Jump to bottom

asset-hub-rococo: Extrinsic exceeds total pov size #4559

Closed
skunert opened this issue May 23, 2024 · 1 comment · Fixed by #4571
Closed

asset-hub-rococo: Extrinsic exceeds total pov size #4559

skunert opened this issue May 23, 2024 · 1 comment · Fixed by #4571
Labels
I2-bug The node fails to follow expected behavior.

Comments

@skunert
Copy link
Contributor

skunert commented May 23, 2024 •
edited
Loading

Asset hub rococo stopped producing blocks today a few hours after the upgrade to 1.12.0.
logs

In the logs we see that the reported PoV size is too large:

DEBUG tokio-runtime-worker runtime::system: [Parachain] Extrinsic exceeds total pov size: 5156.3271484375kb, limit: 5120kb

I introduced the check that is failing just recently #4326.

Usually this would just lead to an extrinsic not getting included in a block. But here block production is complaining about set_validation_data not being in the block. This means that the inherent does not get included. Since they get included in the block first, the working thesis is that we are using too much weight during on_initialize already.

Next investigation steps:

  • Patch runtime with debug logs indicating which pallet is using too much weight
  • Try to reproduce error locally by using --wasm-overrides
@skunert skunert added the I2-bug The node fails to follow expected behavior. label May 23, 2024
@bkchr
Copy link
Member

bkchr commented May 24, 2024

Okay we found the problematic line of code:

polkadot-sdk/substrate/frame/session/src/lib.rs

Line 556 in 5dc522d

T::BlockWeights::get().max_block

With the recent changes to also take the transaction size into account when ensuring that we don't run over the block size limit, this broke the asset hub parachain. IMO the best solution would be that the CheckWeight always allows inherents, even if they probably run over the block size limit.

@skunert skunert mentioned this issue May 24, 2024
Merged
@bkchr bkchr mentioned this issue May 24, 2024
Closed
1 task
@skunert skunert mentioned this issue May 27, 2024
Closed
github-merge-queue bot pushed a commit that referenced this issue May 27, 2024
@skunert
check-weight: Disable total pov size check for mandatory extrinsics (#…
70dd67a 
…4571)

So in some pallets we like
[here](https://github.com/paritytech/polkadot-sdk/blob/5dc522d02fe0b53be1517f8b8979176e489a388b/substrate/frame/session/src/lib.rs#L556)
we use `max_block` as return value for `on_initialize` (ideally we would
not).

This means the block is already full when we try to apply the inherents,
which lead to the error seen in #4559 because we are unable to include
the required inherents. This was not erroring before #4326 because we
were running into this branch:

https://github.com/paritytech/polkadot-sdk/blob/e4b89cc50c8d17868d6c8b122f2e156d678c7525/substrate/frame/system/src/extensions/check_weight.rs#L222-L224

The inherents are of `DispatchClass::Mandatory` and therefore have a
`reserved` value of `None` in all runtimes I have inspected. So they
will always pass the normal check.

So in this PR I adjust the `check_combined_proof_size` to return an
early `Ok(())` for mandatory extrinsics.

If we agree on this PR I will backport it to the 1.12.0 branch.

closes #4559

---------

Co-authored-by: command-bot <>
@skunert skunert reopened this May 27, 2024
@skunert skunert closed this as completed May 31, 2024
@skunert skunert mentioned this issue Jun 3, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Jun 4, 2024
@skunert
Add codeSubstitutes to rococo parachains (#4669)
624a1d5 
Since the fix for #4559
was to add a codeSubstitute to the rococo-parachains, we should update
the chain-specs in the repo.

The json diffs are not super easy to review, so if you want to manually
check:
- Run for example `jd -o people-rococo.diff
people-rococo-from-this-branch.json people-rococo-master.json`
- Check in the resulting diff file that only `codeSubstitutes` field is
changed
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this issue Jun 5, 2024
@skunert @hitchhooker
check-weight: Disable total pov size check for mandatory extrinsics (p…
7a630f9 
…aritytech#4571)

So in some pallets we like
[here](https://github.com/paritytech/polkadot-sdk/blob/5dc522d02fe0b53be1517f8b8979176e489a388b/substrate/frame/session/src/lib.rs#L556)
we use `max_block` as return value for `on_initialize` (ideally we would
not).

This means the block is already full when we try to apply the inherents,
which lead to the error seen in paritytech#4559 because we are unable to include
the required inherents. This was not erroring before paritytech#4326 because we
were running into this branch:

https://github.com/paritytech/polkadot-sdk/blob/e4b89cc50c8d17868d6c8b122f2e156d678c7525/substrate/frame/system/src/extensions/check_weight.rs#L222-L224

The inherents are of `DispatchClass::Mandatory` and therefore have a
`reserved` value of `None` in all runtimes I have inspected. So they
will always pass the normal check.

So in this PR I adjust the `check_combined_proof_size` to return an
early `Ok(())` for mandatory extrinsics.

If we agree on this PR I will backport it to the 1.12.0 branch.

closes paritytech#4559

---------

Co-authored-by: command-bot <>
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this issue Jun 5, 2024
@skunert @hitchhooker
Add codeSubstitutes to rococo parachains (paritytech#4669)
2a39450 
Since the fix for paritytech#4559
was to add a codeSubstitute to the rococo-parachains, we should update
the chain-specs in the repo.

The json diffs are not super easy to review, so if you want to manually
check:
- Run for example `jd -o people-rococo.diff
people-rococo-from-this-branch.json people-rococo-master.json`
- Check in the resulting diff file that only `codeSubstitutes` field is
changed
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this issue Aug 2, 2024
@skunert @TarekkMA
check-weight: Disable total pov size check for mandatory extrinsics (p…
2e88ce5 
…aritytech#4571)

So in some pallets we like
[here](https://github.com/paritytech/polkadot-sdk/blob/5dc522d02fe0b53be1517f8b8979176e489a388b/substrate/frame/session/src/lib.rs#L556)
we use `max_block` as return value for `on_initialize` (ideally we would
not).

This means the block is already full when we try to apply the inherents,
which lead to the error seen in paritytech#4559 because we are unable to include
the required inherents. This was not erroring before paritytech#4326 because we
were running into this branch:

https://github.com/paritytech/polkadot-sdk/blob/e4b89cc50c8d17868d6c8b122f2e156d678c7525/substrate/frame/system/src/extensions/check_weight.rs#L222-L224

The inherents are of `DispatchClass::Mandatory` and therefore have a
`reserved` value of `None` in all runtimes I have inspected. So they
will always pass the normal check.

So in this PR I adjust the `check_combined_proof_size` to return an
early `Ok(())` for mandatory extrinsics.

If we agree on this PR I will backport it to the 1.12.0 branch.

closes paritytech#4559

---------

Co-authored-by: command-bot <>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this issue Aug 2, 2024
@skunert @TarekkMA
Add codeSubstitutes to rococo parachains (paritytech#4669)
d8ec8b3 
Since the fix for paritytech#4559
was to add a codeSubstitute to the rococo-parachains, we should update
the chain-specs in the repo.

The json diffs are not super easy to review, so if you want to manually
check:
- Run for example `jd -o people-rococo.diff
people-rococo-from-this-branch.json people-rococo-master.json`
- Check in the resulting diff file that only `codeSubstitutes` field is
changed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
I2-bug The node fails to follow expected behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

check-weight: Disable total pov size check for mandatory extrinsics paritytech/polkadot-sdk
2 participants
@skunert @bkchr