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

The max_extrinsic weight limit for domain extrinsic seems to be too large #2387

Closed
NingLin-P opened this issue Jan 4, 2024 · 3 comments · Fixed by #2801
Closed

The max_extrinsic weight limit for domain extrinsic seems to be too large #2387

NingLin-P opened this issue Jan 4, 2024 · 3 comments · Fixed by #2801
Assignees
Labels
execution Subspace execution mainnet-beta

Comments

@NingLin-P
Copy link
Member

NingLin-P commented Jan 4, 2024

The max_extrinsic weight limit seems to become too large since we set the max domain block weight to u64::MAX due to max_extrinsic is derived from max_total:
https://github.com/paritytech/polkadot-sdk/blob/6f9b1f61ec759723bb8e86ae6db60c94841cf4c7/substrate/frame/system/src/limits.rs#L415-L426

This may be an issue, especially for the evm transaction whose weight is computed from the gas_limit and it is provided by the user, if gas_limit is set to a large value the evm transaction will enter the operator's tx pool while it will fail to include in the bundle as it exceeds the bundle weight limit, thus it may stay at the tx pool forever.

cc @vedhavyas

@NingLin-P NingLin-P added the execution Subspace execution label Jan 4, 2024
@vedhavyas vedhavyas self-assigned this Jan 4, 2024
@vedhavyas
Copy link
Member

True, I initially wanted to limit max_extrinsic weight to a sane value but somehow missed to update that in the previous PR. This was not straight forward since the BlockWeights max_block_weight was used to derive this. I think we need to manually create the Block weight unfortunately.

@NingLin-P
Copy link
Member Author

This is getting a bit more tricky since we introduced bundle limit in #2568, because the max_extrinsic weight needs to be smaller than the max bundle weight limit while the bundle limit is calculated from the domain config which is a param of the instantiate_domain call and is not a constant value. So different domain instances may need different max_extrinsic values, as a result, we may need to store the bundle limit in the domain state either via genesis state (like domain id) or inherent extrinsic (like XDM channel allow list).

Also, in gemini-3h, the bundle limit of domain 0 is now DomainBundleLimit { max_bundle_size: 357469, max_bundle_weight: Weight { ref_time: 136363636363, proof_size: 1257732550480196701 } }, meaning the max_extrinsic is ~136ms compare to previous 1500ms with domain block weight limit. cc @dariolina

@dariolina
Copy link
Member

dariolina commented Mar 27, 2024

There's also #2365. Can we have bundle limit in domain config instead of block limit? Especially since domain block limit is not enforced.
So in domain config we would have target_bundles_per_slot (1 or less, or more) and max_bundle_weight instead of all these. Then the average domain block weight would be target_bundles_per_slot * max_bundle_weight/SLOT_PROBABILITY and each domain should have hardware requirements to be able to achieve that.

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

Successfully merging a pull request may close this issue.

4 participants