Skip to content

P256Verify precompile should charges EVM gas for signature verification - #3768

Merged
librelois merged 1 commit into
masterfrom
p256
May 27, 2026
Merged

librelois merged 1 commit into
masterfrom
p256

Conversation

@librelois

@librelois librelois commented May 27, 2026

Copy link
Copy Markdown
Contributor

⚠️ Breaking changes ⚠️

  • P256Verify at 0x0000000000000000000000000000000000000100 now charges EVM gas for signature verification. Transactions or contracts that call this precompile with tight gas limits may need to raise their gas limit.

Goal of the changes

Fix P256Verify fee undercharging by making the precompile charge the EVM gasometer for its native signature verification work, while keeping the existing external ref-time accounting.

What reviewers need to know

  • precompiles/p256verify/src/lib.rs now takes a separate gas provider and calls handle.record_cost(...) before record_external_cost(...).
  • runtime/{moonbeam,moonriver,moonbase}/src/precompiles.rs derives the P256 gas charge from the runtime benchmarked weight via GasWeightMapping::weight_to_gas(P256VerifyWeight::get()).
  • The existing ref-time accounting remains in place, so the transaction ref-time limiter still accounts for the native verification work.
  • The P256 precompile unit test now asserts that the mock EVM gasometer is charged.
  • The precompile benchmark helper was updated to pass a mock gas provider.

Testing

  • cargo test -p pallet-evm-precompile-p256verify
  • cargo check -p moonbeam-runtime
  • cargo check -p moonriver-runtime -p moonbase-runtime

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

@librelois librelois added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit breaking Needs to be mentioned in breaking changes labels May 27, 2026
@coderabbitai

coderabbitai Bot commented May 27, 2026

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: 4662bcd3-b62d-42c0-b8d2-cf9dda941721

📥 Commits

Reviewing files that changed from the base of the PR and between 5418772 and 45fe339.

📒 Files selected for processing (5)
  • pallets/precompile-benchmarks/src/benchmarks.rs
  • precompiles/p256verify/src/lib.rs
  • runtime/moonbase/src/precompiles.rs
  • runtime/moonbeam/src/precompiles.rs
  • runtime/moonriver/src/precompiles.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • pallets/precompile-benchmarks/src/benchmarks.rs
  • runtime/moonriver/src/precompiles.rs
  • runtime/moonbase/src/precompiles.rs
  • runtime/moonbeam/src/precompiles.rs
  • precompiles/p256verify/src/lib.rs

📝 Walkthrough

Walkthrough

The P256Verify precompile is extended to accept a second generic G: Get for explicit gas; cost accounting, unit tests, benchmark invocation, and three runtime precompile registrations are updated to provide and use the new gas parameter.

Changes

P256Verify Dual-Parameter Precompile

Layer / File(s) Summary
P256Verify struct and Precompile implementation
precompiles/p256verify/src/lib.rs
P256Verify<W> becomes P256Verify<W, G> where G: Get<u64> supplies an explicit gas value. The Precompile impl header is updated and cost recording now includes G::get().
Unit test scaffolding and verification
precompiles/p256verify/src/lib.rs
Tests add DummyGas, change prepare_handle to return MockHandle, execute P256Verify::<DummyWeight, DummyGas>, and assert handle.gas_used == DummyGas::get().
Benchmark configuration
pallets/precompile-benchmarks/src/benchmarks.rs
Adds P256VerifyGas<T>: Get<u64> returning 4000 and updates benchmark to call P256Verify::<P256VerifyWeight<T>, P256VerifyGas<T>>::execute(&mut handle).
Runtime precompile integration
runtime/moonbase/src/precompiles.rs, runtime/moonbeam/src/precompiles.rs, runtime/moonriver/src/precompiles.rs
Each runtime imports pallet_evm::GasWeightMapping, defines P256VerifyGas as GasWeightMapping::weight_to_gas(P256VerifyWeight::get()), and wires P256Verify<P256VerifyWeight, P256VerifyGas> at address 256.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

D9-needsaudit👮, A10-evmtracing

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding EVM gas charging to the P256Verify precompile for signature verification.
Description check ✅ Passed The description is directly related to the changeset, providing clear context about breaking changes, goals, implementation details, and testing instructions.
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 p256

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.

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 27, 2026
@librelois
librelois merged commit ff82918 into master May 27, 2026
34 of 35 checks passed
@librelois
librelois deleted the p256 branch May 27, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant