Skip to content

Fix precompile address#256

Merged
SegueII merged 1 commit intomainfrom
precompile
Dec 3, 2025
Merged

Fix precompile address#256
SegueII merged 1 commit intomainfrom
precompile

Conversation

@SegueII
Copy link
Copy Markdown
Contributor

@SegueII SegueII commented Dec 3, 2025

Summary by CodeRabbit

  • Chores
    • Standardized internal precompiled contract address notation.
    • Restructured contract precompile test suite for consistency with updated address mappings.

✏️ Tip: You can customize this high-level summary in your review settings.

@SegueII SegueII requested a review from a team as a code owner December 3, 2025 03:26
@SegueII SegueII requested review from panos-xyz and removed request for a team December 3, 2025 03:26
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 3, 2025

Walkthrough

Two contract-related files were modified: address mappings for precompiled contracts were reformatted from plain integer notation to explicit hexadecimal byte representation in contracts.go, and corresponding test suite for BLS12381 precompiles was restructured with updated address keys and test function identifiers in contracts_test.go.

Changes

Cohort / File(s) Summary
Address mapping reformatting
core/vm/contracts.go
Precompiled contract address mappings reformatted from common.BytesToAddress([]byte{n}) to common.BytesToAddress([]byte{0x0n}) notation (e.g., 1 → 0x01). Purely formatting change; no functional or logic modifications.
BLS12381 test suite restructuring
core/vm/contracts_test.go
Updated precompile address mappings for BLS12381 contracts from single-byte slots (10–16) to multi-byte keys (0x0f, 0x0a–0x0f, 0x10). Renamed test functions (TestPrecompiledBLS12381G1Add, G1Mul, G1MultiExp, etc.) with updated address identifiers (e.g., "f0a", "f0b"). Updated failure and benchmark tests to align with new naming and address scheme.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Attention areas:
    • Verify that the hexadecimal address mapping reformatting in contracts.go produces equivalent bytewise addresses to the original integer notation across all precompiles
    • Confirm that all BLS12381 test address key changes in contracts_test.go correctly correspond to the updated mappings and that no test coverage is lost
    • Cross-reference updated address identifiers in test invocations to ensure consistency between the allPrecompiles map and test data wiring

Poem

🐰 A hop through precompile lanes so fine,
From plain old ints to hex divine,
The BLS tests now sing their song,
With renamed paths to right the wrong,
Addresses mapped, both fresh and true—
The VM's contracts wear new shoes!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fix precompile address' is vague and does not clearly convey what was changed or why it needed fixing. Consider a more descriptive title such as 'Reformat precompile address mappings to hex notation' or 'Fix BLS12381 precompile address mapping' to better clarify the specific change.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch precompile

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7618ae1 and 1e1637e.

📒 Files selected for processing (2)
  • core/vm/contracts.go (1 hunks)
  • core/vm/contracts_test.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
core/vm/contracts.go (2)
common/types.go (1)
  • BytesToAddress (206-210)
accounts/abi/abi_test.go (1)
  • Address (73-73)
core/vm/contracts_test.go (1)
common/types.go (1)
  • BytesToAddress (206-210)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (4)
core/vm/contracts.go (2)

60-64: LGTM! Consistent hex notation improves readability.

The change from decimal to explicit hex notation (e.g., []byte{1}[]byte{0x01}) is a cosmetic improvement that makes the address values more explicit without changing their underlying values.


161-169: BLS12381 precompile addresses look correct.

The BLS12381 precompiles at addresses 0x0b-0x11 align with EIP-2537 specifications, and the p256Verify at 0x100 is consistent with its EIP assignment.

core/vm/contracts_test.go (2)

61-68: Test map addresses updated consistently.

The allPrecompiles map keys have been updated to use the new two-byte address format for BLS12381 precompiles.


313-340: Test function addresses updated consistently.

The test and benchmark function address strings ("f0a", "f0b", etc.) correctly correspond to the updated allPrecompiles map keys.


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.

@SegueII SegueII merged commit 49fa27b into main Dec 3, 2025
8 checks passed
@SegueII SegueII deleted the precompile branch December 3, 2025 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants