feat(feynman): cherry-pick EIP-7823 and EIP-7883 from upstream#1211
feat(feynman): cherry-pick EIP-7823 and EIP-7883 from upstream#1211
Conversation
https://eips.ethereum.org/EIPS/eip-7883 --------- Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de> Co-authored-by: Felix Lange <fjl@twurst.com>
…hereum#32015) Implements the updated gas cost changes introduced in ethereum/EIPs@5cdd751
WalkthroughThe changes introduce support for EIP-7883 and EIP-7823 in the modular exponentiation (bigModExp) precompile by adding new boolean flags to its struct and updating gas calculation and input validation logic accordingly. Test coverage and benchmarks are extended for the new EIP-7883 variant, including new test vectors in a dedicated JSON file. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant PrecompileMap
participant BigModExp
participant GasCalculator
TestRunner->>PrecompileMap: Lookup address (e.g., 0xf6)
PrecompileMap->>BigModExp: Instantiate with eip2565/eip7883 flags
TestRunner->>BigModExp: Run(input)
BigModExp->>GasCalculator: RequiredGas(input, flags)
GasCalculator-->>BigModExp: Calculated gas
BigModExp->>BigModExp: Validate input lengths (per EIP flags)
BigModExp->>BigModExp: Perform modular exponentiation
BigModExp-->>TestRunner: Return result or error
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/vm/contracts.go(12 hunks)core/vm/contracts_test.go(2 hunks)core/vm/testdata/precompiles/modexp_eip7883.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test
- GitHub Check: check
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (go)
🔇 Additional comments (11)
core/vm/contracts_test.go (2)
53-55: LGTM! Comprehensive test infrastructure update.The changes properly extend the test infrastructure by:
- Explicitly setting
eip7883: falsefor existing precompile variants to maintain clarity- Adding a new variant at address
0xf6with botheip2565andeip7883enabled- Following the established pattern for precompile configuration
248-250: Test coverage appropriately extended for EIP-7883.The new test and benchmark functions follow established naming conventions and correctly reference the new test data file (
modexp_eip7883) and precompile address (f6).core/vm/testdata/precompiles/modexp_eip7883.json (1)
1-107: Comprehensive test data for EIP-7883 modular exponentiation.The test vectors provide excellent coverage:
- Variety: Includes squaring, cubing, and power operations with exponent 0x10001
- Scale: Covers input sizes from 0x40 to 0x400 bytes (64 to 1024 bytes)
- Gas progression: Appropriate gas cost increases with input complexity (500 → 43690 → 174762)
- Standard vectors: Uses well-established "nagydani" test series for compatibility
The JSON structure is consistent and includes all necessary fields for comprehensive testing.
core/vm/contracts.go (8)
23-23: Import addition necessary for enhanced error formatting.The
fmtimport is correctly added to support the new formatted error message in the EIP-7823 validation logic.
68-68: Consistent precompile configuration across releases.The explicit initialization of
eip7823: false, eip7883: falsefor existing releases maintains backward compatibility while clearly documenting the flag states for each version.Also applies to: 81-81, 95-95, 109-109, 123-123, 137-137
152-152: Feynman release correctly enables all EIP flags.The Feynman precompile configuration appropriately enables
eip2565: true, eip7823: true, eip7883: trueto support the new EIP implementations.
353-355: Struct extension follows established pattern.The addition of
eip7823andeip7883boolean flags to thebigModExpstruct follows the existing pattern established byeip2565.
443-449: Gas calculation logic reorganization.The refactored gas calculation now uses
max(baseLen, modLen)instead of separate variables, which simplifies the logic. The introduction ofmaxLenOver32flag for subsequent conditional logic is well-structured.
507-513: Input validation logic correctly preserves backward compatibility.The 32-byte length limit is only enforced when neither
eip7823noreip7883are enabled, maintaining backward compatibility while allowing larger inputs for newer EIP versions.
524-526: EIP-7823 size limit enforcement.The 1024-byte limit for EIP-7823 with a clear error message using
fmt.Errorfprovides appropriate bounds checking. The error message is descriptive and helpful for debugging.
435-439: Review the bit shifting logic for EIP-7883.The gas calculation introduces different bit shifting based on EIP-7883:
- EIP-7883: 4-bit left shift (16x multiplier)
- Non-EIP-7883: 3-bit left shift (8x multiplier)
This represents a significant change in gas calculation that should be thoroughly validated against the EIP specification.
Please verify that the 4-bit vs 3-bit shift difference aligns with the EIP-7883 specification requirements.
#!/bin/bash # Search for any documentation or comments explaining the bit shift logic rg -A 5 -B 5 "Lsh.*4|shift.*4|EIP-7883.*shift" --type go
|
Spec is not final yet, so this is not included in Feynman. |
3 participants
1. Purpose or design rationale of this PR
Enable https://eips.ethereum.org/EIPS/eip-7823 and https://eips.ethereum.org/EIPS/eip-7883 in Feynman. Disable our previous length check at the same time.
Relevant previous changes:
modexpinput length #580Upstream PRs:
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit