-
Notifications
You must be signed in to change notification settings - Fork 69
Update geth version #815
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
Update geth version #815
Conversation
WalkthroughBumped Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as caller
participant VM as vm
rect `#E6F7FF`
Note right of VM: Old flow (pre-change)
Caller->>VM: call precompiled (BLS) addr(17) for mapG1
VM-->>Caller: result (mapG1)
Caller->>VM: call precompiled (BLS) addr(16) for pairing
VM-->>Caller: result (pairing)
end
sequenceDiagram
participant Caller as caller
participant VM as vm
rect `#FFF4E6`
Note right of VM: New flow (this PR)
Caller->>VM: call precompiled (Emerald) addr(17) for mapG1
VM-->>Caller: result (mapG1)
Caller->>VM: call precompiled (Emerald) addr(16) for pairing
VM-->>Caller: result (pairing)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ops/tools/bls12381/main.go(1 hunks)
⏰ 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). (11)
- GitHub Check: check
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: check
- GitHub Check: test
- GitHub Check: check
- GitHub Check: check
- GitHub Check: test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
ops/tools/bls12381/main.go (1)
87-120: <!-- [scratchpad_start]
[task review_verification 2/3 - UPDATED]
Verify address 16 mapping for BLS12381 pairing in Emerald precompiled contracts[observations]
- Line 87: bls12381Pairing uses vm.PrecompiledContractsEmerald[common.BytesToAddress([]byte{16})]
- Line 80: bls12381MapG1 uses vm.PrecompiledContractsEmerald[common.BytesToAddress([]byte{17})]
- Search found NO definition of PrecompiledContractsEmerald in codebase
- Search found NO reference to PrecompiledContractsBLS anywhere in codebase
- This means PrecompiledContractsEmerald is from external dependency (not defined locally)
- Standard Ethereum: address 0x10 (16) = BLS12381 pairing, 0x11 (17) = BLS12381 G1 mapping
- No git history or version information to determine migration source
- Cannot verify by scanning codebase since definition is external
[pending]
- Need to find vm package import source in main.go
- Need to determine if PrecompiledContractsEmerald is from go-ethereum
- Check go.mod for morph-l2/go-ethereum version and whether it supports Emerald
[actions]
- Search for imports in main.go to find vm package source
- Check go.mod for dependencies related to Emerald/go-ethereum
- Verify version and address mapping in documentation if available
| bls12381MapG1 := vm.PrecompiledContractsEmerald[common.BytesToAddress([]byte{17})] | ||
| messagePointOnCurveBz, err := bls12381MapG1.Run(messageInput) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| costGas += bls12381MapG1.RequiredGas(messageInput) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the correct precompiled contract address for BLS12381 map to G1 operation.
The change from vm.PrecompiledContractsBLS to vm.PrecompiledContractsEmerald[address 17] needs verification. Standard Ethereum BLS12381 precompiled contracts typically use address 0x12 (18) for the map to G1 operation, not address 17. Please confirm that:
- Address 17 is the correct Emerald precompiled contract address for BLS12381 map to G1
- The interface is compatible (Run() and RequiredGas() methods match expectations)
- The functional behavior is equivalent to the previous BLS precompiled contract
Run the following script to check for precompiled contract address definitions and any related documentation or tests in the codebase:
Additionally, please verify the correct address mapping in the updated go-ethereum documentation:
🏁 Script executed:
#!/bin/bash
# Search for Emerald precompiled contract address definitions and related constants
rg -nP --type=go 'PrecompiledContractsEmerald|bls12381.*17|MapG1' -A3 -B3
# Search for any other usages of PrecompiledContractsBLS that might need updating
rg -nP --type=go 'PrecompiledContractsBLS' -A2 -B2
# Look for test files that cover BLS operations
fd -e go -e test test | xargs rg -l 'bls12381|MapG1'Length of output: 1680
🌐 Web query:
github.com/morph-l2/go-ethereum BLS12381 Emerald precompiled contract addresses
💡 Result:
The Emerald (Morph) implementation uses the EIP‑2537 BLS12‑381 precompile addresses:
- 0x0b — bls12381G1Add. [1][2]
- 0x0c — bls12381G1MultiExp (G1 MSM). [1][2]
- 0x0d — bls12381G2Add. [1][2]
- 0x0e — bls12381G2MultiExp (G2 MSM). [1][2]
- 0x0f — bls12381Pairing. [1][2]
- 0x10 — bls12381MapG1. [1][2]
- 0x11 — bls12381MapG2. [1][2]
Sources: morph-l2/go-ethereum repo and Morph/Emerald docs; EIP‑2537 specification. [1][2][3]
References
[1] github.com/morph-l2/go-ethereum.
[2] EIP-2537: Precompile for BLS12-381 curve operations.
[3] Morph/Emerald precompiles documentation.
Correct the precompiled contract addresses for BLS12381 operations.
The addresses are incorrect and will invoke the wrong precompiled contracts:
- Line 80: Uses address 17 for
bls12381MapG1, but should be address 16 (0x10) - Line 87: Uses address 16 for
bls12381Pairing, but should be address 15 (0x0f)
Per EIP-2537, the Emerald precompile addresses are: 0x0f (15) for bls12381Pairing and 0x10 (16) for bls12381MapG1. These must be corrected to ensure BLS signature verification works correctly.
🤖 Prompt for AI Agents
In ops/tools/bls12381/main.go around lines 80 to 85 (and the pairing call at
~line 87), the precompile addresses are off by one: change the bls12381MapG1
lookup from common.BytesToAddress([]byte{17}) to
common.BytesToAddress([]byte{16}) (0x10) and change the bls12381Pairing lookup
from common.BytesToAddress([]byte{16}) to common.BytesToAddress([]byte{15})
(0x0f) so they match EIP-2537; keep the rest of the call sites (Run, error
handling, and RequiredGas) the same.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.