Skip to content

Conversation

@ercecan
Copy link
Member

@ercecan ercecan commented Jan 7, 2026

Description

Initial pr for switching to eip712 structured message types for security council and lcp
Uses addresses instead of pubkeys to verify signatures, basically recovers pubkeys from signatures calculates the addresses and compares them

@github-actions
Copy link

github-actions bot commented Jan 7, 2026

⚠️ Changelog entry missing

No changes detected in CHANGELOG.md.

Recommendation:

feat: Use address instead of pubkeys for security council

Please add an entry to the CHANGELOG.md or dismiss this if the change doesn't require documentation.

To dismiss: Reply with /skip-changelog in any comment.

@ercecan ercecan self-assigned this Jan 7, 2026
@ercecan ercecan marked this pull request as ready for review January 7, 2026 15:08
@ercecan ercecan requested a review from a team as a code owner January 7, 2026 15:08
Copilot AI review requested due to automatic review settings January 7, 2026 15:08
@ercecan ercecan changed the base branch from nightly to needs-audit January 7, 2026 15:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a significant security upgrade by switching from public key-based signature verification to address-based verification for the security council, using EIP-712 structured messages. The change involves recovering public keys from signatures and comparing the derived Ethereum addresses against expected addresses stored in initial values.

Key Changes:

  • Updated signature size from 64 to 65 bytes to include the recovery ID (v value) necessary for public key recovery
  • Implemented public key recovery from signatures with proper handling of low-s normalization and recovery ID extraction
  • Replaced all security council public key constants with Ethereum addresses across different networks (Mainnet, Testnet, Devnet, Nightly)

Reviewed changes

Copilot reviewed 8 out of 11 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
crates/sovereign-sdk/rollup-interface/src/state_machine/da.rs Updated signature size constant from 64 to 65 bytes with documentation
crates/light-client-prover/src/circuit/method_id_verifier.rs Implemented public key recovery logic with proper error handling and address comparison
crates/light-client-prover/src/circuit/initial_values.rs Replaced public key constants with Ethereum addresses for all networks
crates/light-client-prover/src/circuit/mod.rs Updated function signatures to accept addresses instead of public keys
crates/light-client-prover/src/tests/test_utils.rs Updated test utilities to generate addresses from public keys for testing
crates/light-client-prover/src/tests/mod.rs Updated all test cases to use new address-based constants
crates/light-client-prover/src/da_block_handler.rs Updated method call to use new address-based function
guests/risc0/light-client-proof/bitcoin/src/bin/light_client_proof_bitcoin.rs Updated to use address-based constants and added alloy-primitives import
guests/risc0/light-client-proof/bitcoin/Cargo.toml Added alloy-primitives dependency
guests/risc0/light-client-proof/bitcoin/Cargo.lock Reflected dependency addition in lock file
guests/risc0/light-client-proof/mock/src/bin/light_client_proof_mock.rs Updated to use new address-based constants

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link

github-actions bot commented Jan 7, 2026

Proving stats report

Comparing patch(cd734e6) to base(6ed8929).

Metric Base Patch Change
Total Cycles 5,920,260,096 5,920,260,096 -
User Cycles 4,812,631,396 4,812,631,396 -
Paging Cycles 943,234,084 943,234,084 -
Reserved Cycles 164,394,616 164,394,616 -
State Diff Size (bytes) 104,526 104,526 -

@codecov
Copy link

codecov bot commented Jan 7, 2026

Codecov Report

❌ Patch coverage is 71.91011% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.8%. Comparing base (6ed8929) to head (cd734e6).

Files with missing lines Patch % Lines
...ht-client-prover/src/circuit/method_id_verifier.rs 75.6% 18 Missing ⚠️
.../light-client-prover/src/circuit/initial_values.rs 30.0% 7 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
crates/light-client-prover/src/circuit/mod.rs 88.1% <100.0%> (-0.1%) ⬇️
crates/light-client-prover/src/da_block_handler.rs 97.6% <100.0%> (ø)
...reign-sdk/rollup-interface/src/state_machine/da.rs 65.9% <ø> (ø)
.../light-client-prover/src/circuit/initial_values.rs 47.2% <30.0%> (ø)
...ht-client-prover/src/circuit/method_id_verifier.rs 90.3% <75.6%> (-6.9%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants