Skip to content

Fix underestimated proof-size accounting for precompile storage reads - #3753

Merged
librelois merged 2 commits into
masterfrom
elois/backport-hotfix
May 19, 2026
Merged

Fix underestimated proof-size accounting for precompile storage reads#3753
librelois merged 2 commits into
masterfrom
elois/backport-hotfix

Conversation

@librelois

@librelois librelois commented May 19, 2026 •
edited by blacksmith-sh Bot
Loading

Copy link
Copy Markdown
Contributor

Goal

Fix underestimated proof-size accounting for precompile storage reads that return conviction-voting class locks and parachain-staking delegator state.

What reviewers need to know

  • precompiles/conviction-voting now charges classLocksFor using the max encoded length of the full ClassLocksFor bounded vector, including each lock balance, instead of only accounting for class IDs.
  • precompiles/parachain-staking now computes the delegator-state read proof size from SCALE upper bounds for the key and max-sized Delegator value, replacing the previous fixed formula.
  • pallets/parachain-staking derives MaxEncodedLen for Bond and DelegatorStatus so the staking precompile can derive the upper bound from the actual encoded types.
  • This changes proof-size/gas accounting for affected view calls, but does not change storage formats, dispatch behavior, selectors, or returned values.

note: Silent label because the change was already deployed to production in release runtimes 4204 and 4303, so we should not mention it in the next regular changelog.

View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

@librelois librelois added B0-silent Changes should not be mentioned in any release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit not-breaking Does not need to be mentioned in breaking changes labels May 19, 2026
@coderabbitai

coderabbitai Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 88a4db58-826b-4dc3-a61f-a243abca710c

📥 Commits

Reviewing files that changed from the base of the PR and between ac98c73 and 759a0a5.

📒 Files selected for processing (5)
  • pallets/parachain-staking/src/types.rs
  • precompiles/conviction-voting/src/lib.rs
  • precompiles/conviction-voting/src/tests.rs
  • precompiles/parachain-staking/src/lib.rs
  • precompiles/parachain-staking/src/tests.rs
📝 Walkthrough

Walkthrough

This PR adds MaxEncodedLen bounds to pallet types and precompiles, introduces proof-size helper functions for storage reads in both parachain staking and conviction voting modules, and refactors view methods to use centralized calculations instead of inline expressions.

Changes

Parachain Staking Proof-Size Refactoring

Layer / File(s) Summary
Parachain Staking Type Encoding
pallets/parachain-staking/src/types.rs		 Added MaxEncodedLen trait derivation with SCALE bounds to Bond<AccountId, Balance> and DelegatorAdded<B> types, expanding the parity_scale_codec imports.
Precompile Generic Bounds and Proof-Size Helper
precompiles/parachain-staking/src/lib.rs		 Updated ParachainStakingPrecompile generic constraints to require Runtime::AccountId: MaxEncodedLen, added Compact and Encode imports, and implemented delegator_state_storage_read_proof_size() helper that computes upper bounds using SCALE encoding sizes.
View Methods Proof-Size Refactoring
precompiles/parachain-staking/src/lib.rs		 Replaced hardcoded inline proof-size calculations in delegation-count, delegation-amount, is-delegator, and total-staked view methods with calls to the centralized helper function.
Parachain Staking Proof-Size Tests
precompiles/parachain-staking/src/tests.rs		 Restructured test imports to expose the precompile type and added validation test confirming delegator_state_storage_read_proof_size() matches the expected SCALE encoding upper-bound formula.

Conviction Voting Proof-Size Refactoring

Layer / File(s) Summary
Conviction Voting Imports and Type Alias
precompiles/conviction-voting/src/lib.rs		 Reorganized imports to include BoundedVec and MaxEncodedLen, and introduced ClassLocksForOf<Runtime> type alias representing the bounded vector of class locks with ClassCountOf-driven compile-time bounds.
Generic Bounds and Proof-Size Simplification
precompiles/conviction-voting/src/lib.rs		 Updated ConvictionVotingPrecompile impl to require MaxEncodedLen for BalanceOf<Runtime> and ClassOf<Runtime>, and simplified class_locks_for view method to use ClassLocksForOf::<Runtime>::max_encoded_len() instead of manual ClassCountOf/Tally expansion.
Conviction Voting Proof-Size Tests
precompiles/conviction-voting/src/tests.rs		 Added MaxEncodedLen to test imports and implemented test validating ClassLocksForOf proof-size formula against accounts-for-balances size computation.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: fixing underestimated proof-size accounting for precompile storage reads.
Description check ✅ Passed The description clearly explains the goal, what was changed across multiple files, and the impact on proof-size/gas accounting, directly relating to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch elois/backport-hotfix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

@@                    Coverage Diff                    @@
##           master   elois/backport-hotfix      +/-   ##
=========================================================
+ Coverage   77.12%                  77.13%   +0.01%     
  Files         389                     389              
+ Lines       77272                   77285      +13     
=========================================================
+ Hits        59593                   59611      +18     
- Misses      17679                   17674       -5
Files Changed Coverage
/precompiles/conviction-voting/src/lib.rs 91.67% (+0.05%) 🔼
/precompiles/conviction-voting/src/tests.rs 99.12% (+0.01%) 🔼
/precompiles/parachain-staking/src/lib.rs 93.74% (+0.76%) 🔼

Coverage generated Tue May 19 10:39:47 UTC 2026

@librelois librelois added D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited and removed D3-trivial PR contains trivial changes in a runtime directory that do not require an audit labels May 19, 2026
@librelois
librelois merged commit c7e39ec into master May 19, 2026
54 of 59 checks passed
@librelois
librelois deleted the elois/backport-hotfix branch May 19, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees

No one assigned

Labels

B0-silent Changes should not be mentioned in any release notes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited not-breaking Does not need to be mentioned in breaking changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@librelois @derconno