Skip to content

Register privacy L3 devnet chain 84534 for nitro proving#3435

Draft
jowparks wants to merge 1 commit into
l3from
feat/privacy-sepolia-chain-config
Draft

Register privacy L3 devnet chain 84534 for nitro proving#3435
jowparks wants to merge 1 commit into
l3from
feat/privacy-sepolia-chain-config

Conversation

@jowparks

Copy link
Copy Markdown
Collaborator

Summary

  • Adds privacy-sepolia (ChainConfig chain ID 84534, parent L1 84532) so nitro host/enclave can resolve rollup_config!(84534) and precompute config hashes.
  • Adds Base Sepolia (84532) to L1_CONFIGS as an OP-stack L1 parent config (required by nitro-host prover_config() when proving L3 batches derived from Base Sepolia).
  • Extends config-hash regression tests for chain 84534.

Placeholder fields (follow-up required)

Genesis anchors, L1 contract addresses, and azul/beryl timestamps currently use zero/derived placeholders. They must be updated after privacy-enclave pins a deploy-once genesis so PerChainConfig::hash() matches the live rollup config at runtime.

Privacy devnet params encoded today:

  • block_time=2, seq_window=3600, azul block 20 / beryl block 21 → timestamps 40/42 (assumes genesis_l2_time=0)
  • batch_inbox: 0xff...84534
  • activation_admin: devnet Anvil account 0x9965...

Test plan

  • cargo test -p base-common-chains --lib
  • cargo test -p base-proof-tee-nitro-enclave config_hashes_match_chain_configs
  • cargo test -p base-proof-succinct-client-utils test_config_hash_matches_nitro_enclave
  • Rebuild base-proofs nitro EIF with this BASE_COMMIT and bump privacy-enclave mirror digests (after merge)
  • Pin privacy devnet genesis and update placeholder addresses/hashes in PRIVACY_SEPOLIA

Made with Cursor

Adds privacy-sepolia rollup config with Base Sepolia (84532) as L1 parent,
including L1_CONFIGS entry for OP-stack parent proving and config-hash tests.
Genesis anchors and contract addresses are placeholders until privacy devnet
pins deploy-once genesis.

Co-authored-by: Cursor <cursoragent@cursor.com>
spin.workspace = true
auto_impl.workspace = true
serde = { workspace = true, optional = true }
serde_json = { workspace = true, default-features = false, features = ["alloc"] }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

serde_json is added as a required (non-optional) runtime dependency to this #![no_std] crate. The existing Ethereum L1 configs are all built programmatically — no JSON deserialization needed at runtime. This new dependency is only used by BaseSepolia::l1_config() to parse a single embedded JSON blob at lazy-init time.

Consider one of these alternatives to avoid inflating the dependency graph for all consumers (including zkVM proof clients):

  1. Feature-gate it: make serde_json optional behind a feature (e.g., op-l1-configs) and gate the op module on that feature.
  2. Build-time codegen: deserialize in build.rs and emit the ChainConfig as a const (matching the pattern used for the other chains).

map.insert(NamedChain::Sepolia.into(), Sepolia::l1_config());
map.insert(NamedChain::Holesky.into(), Holesky::l1_config());
map.insert(NamedChain::Hoodi.into(), Hoodi::l1_config());
map.insert(84532, BaseSepolia::l1_config());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: Base Sepolia (84532) is an OP-stack L2, not an Ethereum L1 chain. The module doc comment on line 1 says "Static Ethereum L1 chain configuration mapping" and this lives under the ethereum/ module, which makes the placement a bit misleading.

L1_CONFIGS is semantically correct from the L3's perspective (Base Sepolia is the L3's "L1 parent"), but you may want to either:

  • Update the module doc to say "parent chain configurations" instead of "Ethereum L1 chain configurations", or
  • Rename this map / move it out of the ethereum module to reflect it now holds non-Ethereum entries too.

Not blocking, but worth considering as more OP-stack L1 parents may follow.

@github-actions

Copy link
Copy Markdown
Contributor

Review Summary

The PR correctly registers a new privacy L3 devnet chain (84534) and its OP-stack parent (Base Sepolia 84532) for nitro proving. The chain config follows established patterns, tests are solid, and config hash regression tests are included. Two items flagged:

Findings

  1. serde_json added as unconditional runtime dependency to a no_std crate (Cargo.toml:37) — The existing Ethereum L1 configs are built programmatically with zero runtime deserialization. This PR adds serde_json as a required dependency for all consumers just to parse one embedded JSON blob for BaseSepolia. Consider feature-gating it or using build-time codegen.

  2. OP-stack chain registered in ethereum/ module (ethereum/config.rs:17) — Base Sepolia is an OP-stack L2 chain, not an Ethereum L1 chain. The BaseSepolia type lives correctly in op/, but it's imported into and registered within the ethereum/config module whose doc says "Ethereum L1 chain configurations". Non-blocking, but worth revisiting the naming/location as more OP-stack parents may appear.

No correctness, safety, or concurrency issues found.

@github-actions

Copy link
Copy Markdown
Contributor

❌ base-std fork tests: 4 failed, 611 passed

base/base diverges from the base-std spec.

Failing tests
  • test_updateMultiplier_revertOrder(address): next call did not revert as expected; counterexample: calldata=0x05e3b432000000000000000000000000d5ab9b58f295186f6baba06ff11b92ed0c486558 args=[0xD5ab9b58f295186F6BabA06Ff11b92eD0c486558]
    [FAIL: next call did not revert as expected] test_updateMultiplier_revert_zeroMultiplier() (gas: 49033)

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.

1 participant