fix: update BatchAuthenticator to use Espresso tee contract's verify function#363
Merged
Sneh1999 merged 4 commits intocelo-integration-rebase-14.2from Mar 3, 2026
Hidden character warning
The head ref may contain hidden characters: "use\u2013espresso-tee-verifier-function"
Merged
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request modernizes the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the BatchAuthenticator to use the verify function from the Espresso TEE contracts for authenticating batch information, removing the manual signature recovery logic. The changes are reflected in the contract interface, implementation, and tests. My review focuses on improving code maintainability by removing dead code and reducing code duplication in tests, and on strengthening test assertions.
QuentinI
added a commit
that referenced
this pull request
Mar 4, 2026
…tes32,address) PR #363 removed the 'address indexed signer' parameter from the BatchInfoAuthenticated event, but the Go derivation pipeline still used the old 2-parameter signature. The keccak256 hash of the wrong signature never matched actual log topics, causing CollectAuthenticatedBatches to find zero events and reject every batch. - Fix ABI string in batch_authenticator.go - Update test mocks to use 2-topic logs matching the actual event - Regenerate Go bindings from fresh forge artifacts Co-authored-by: OpenCode <noreply@opencode.ai>
QuentinI
added a commit
that referenced
this pull request
Mar 4, 2026
…tes32,address) PR #363 removed the 'address indexed signer' parameter from the BatchInfoAuthenticated event, but the Go derivation pipeline still used the old 2-parameter signature. The keccak256 hash of the wrong signature never matched actual log topics, causing CollectAuthenticatedBatches to find zero events and reject every batch. - Fix ABI string in batch_authenticator.go - Update test mocks to use 2-topic logs matching the actual event - Regenerate Go bindings from fresh forge artifacts Co-authored-by: OpenCode <noreply@opencode.ai>
QuentinI
added a commit
that referenced
this pull request
Mar 5, 2026
…tes32,address) PR #363 removed the 'address indexed signer' parameter from the BatchInfoAuthenticated event, but the Go derivation pipeline still used the old 2-parameter signature. The keccak256 hash of the wrong signature never matched actual log topics, causing CollectAuthenticatedBatches to find zero events and reject every batch. - Fix ABI string in batch_authenticator.go - Update test mocks to use 2-topic logs matching the actual event - Regenerate Go bindings from fresh forge artifacts Co-authored-by: OpenCode <noreply@opencode.ai>
QuentinI
added a commit
that referenced
this pull request
Mar 20, 2026
…357) * Move batch authentication into derivation pipeline, remove BatchInbox contract Move batch authentication from on-chain validation (BatchInbox contract calling BatchAuthenticator.validateBatch) to off-chain verification in the derivation pipeline via BatchInfoAuthenticated event scanning. Key changes: - Add batch_authenticator.go: event-based auth using CollectAuthenticatedBatches which scans a lookback window once per L1 block and returns authenticated hashes - Add isBatchTxAuthorized helper shared by calldata and blob data sources - Remove BatchInbox contract, interface, tests, bindings, and snapshots entirely (Espresso-introduced, never in Celo fork) - Remove validateBatch/validateTeeBatch/validateNonTeeBatch from BatchAuthenticator (authenticateBatchInfo and signer management remain) - Regenerate BatchAuthenticator Go bindings - Revert BatchInbox-related fields from L1Deployments, ChainState, DeployEspressoOutput, and calculateBatchInboxAddr back to upstream signatures (introduce+remove no-ops) - Update integration/devnet tests for EOA BatchInbox behavior Co-authored-by: OpenCode <noreply@opencode.ai> * Add FallbackBatcherAddress to derivation pipeline for non-TEE batcher auth When batch authentication is enabled, the TEE batcher requires a matching BatchInfoAuthenticated event on L1. The fallback (non-TEE) batcher does not call authenticateBatchInfo, so its transactions had no auth events and were rejected by the derivation pipeline. Add FallbackBatcherAddress to rollup.Config and the derivation pipeline's DataSourceConfig. When event-based auth finds no matching event for a batch, it falls back to sender verification against the fallback batcher address. This allows the non-TEE batcher to post batches without on-chain auth events while still requiring event-based auth for the TEE batcher. Co-authored-by: OpenCode <noreply@opencode.ai> * Remove validBatchInfo mapping from BatchAuthenticator The validBatchInfo mapping was written to on every authenticateBatchInfo call but no longer read by the derivation pipeline, which now uses BatchInfoAuthenticated events instead. Removing it saves ~20k gas per call. - Remove mapping declaration and write from BatchAuthenticator.sol - Remove from IBatchAuthenticator interface - Remove test assertions on validBatchInfo - Update SECURITY_ANALYSIS.md to reflect event-based auth model - Bump contract version to 1.1.0 - Regenerate ABI/storage snapshots and Go bindings Co-authored-by: OpenCode <noreply@opencode.ai> * Comments * Fix txmgr wrapper * Fix nonce collision: use separate account for fallback batcher The fallback batcher (op-batcher-fallback) was using Anvil account #3 (0x90F79bf6EB2c4f870365E785982E1f101E93b906), which is also the Deployer key used by devnet tests for admin transactions (SwitchBatcher, TransferOwnership, etc.). When both the fallback batcher service and test code sign L1 transactions from the same account concurrently, nonce collisions cause transactions to never be mined, leading to timeouts. Switch the fallback batcher to Anvil account #6 (0x976EA74026E726554dB657fA54763abd0C3a0aa9) across docker-compose, prepare-allocs (nonTeeBatcher in rollup config), and .env. Co-authored-by: OpenCode <noreply@opencode.ai> * Add batcher-side idle check: inactive batcher skips publishing Query BatchAuthenticator.activeIsTee() before publishing to L1. The inactive batcher (TEE or fallback) now skips publishStateToL1 instead of posting transactions that would be ignored by the derivation pipeline. This was previously unnecessary because the BatchInbox contract's validateBatch() would revert inactive batcher transactions during eth_estimateGas. With the move to an EOA batch inbox, both batchers can freely post, so an explicit idle check is needed. Co-authored-by: OpenCode <noreply@opencode.ai> * Fix event signature mismatch: BatchInfoAuthenticated(bytes32) not (bytes32,address) PR #363 removed the 'address indexed signer' parameter from the BatchInfoAuthenticated event, but the Go derivation pipeline still used the old 2-parameter signature. The keccak256 hash of the wrong signature never matched actual log topics, causing CollectAuthenticatedBatches to find zero events and reject every batch. - Fix ABI string in batch_authenticator.go - Update test mocks to use 2-topic logs matching the actual event - Regenerate Go bindings from fresh forge artifacts Co-authored-by: OpenCode <noreply@opencode.ai> * Cache L1BlockRef lookups in batch auth lookback window traversal CollectAuthenticatedBatches walks backwards ~100 L1 blocks per call, making an L1BlockRefByHash RPC call for each step. Since consecutive L1 blocks share ~99 blocks in their lookback windows, these calls are almost entirely redundant after the first traversal. Add a second global LRU cache (blockRefCache) mapping block hash to L1BlockRef, alongside the existing receipt cache. On steady state this reduces L1BlockRefByHash RPC calls from ~100 per L1 block to ~0-1, a ~50x reduction in total RPC overhead for the batch auth path. Co-authored-by: OpenCode <noreply@opencode.ai> * Increase batcher switch stabilization delay to 60s in TestBatcherActivePublishOnly In-flight sendTxWithEspresso goroutines spawned before deactivation can take ~25s to drain their queued Txmgr.Send calls. The previous 10s delay was insufficient, causing stale TEE batcher transactions to appear in the post-switch monitoring window and failing the assertion. Co-authored-by: OpenCode <noreply@opencode.ai> * Reduce lookback window * Pin eth2-val-tools * Update succinct versions --------- Co-authored-by: OpenCode <noreply@opencode.ai>
shenkeyao
pushed a commit
that referenced
this pull request
Mar 20, 2026
…function (#363) * fix: update BatchAuthenticator to use Espresso tee contracts verify function * fix: add v normalization to go code * fix: remove redundant code * fix: test and address comments
shenkeyao
pushed a commit
that referenced
this pull request
Mar 20, 2026
…357) * Move batch authentication into derivation pipeline, remove BatchInbox contract Move batch authentication from on-chain validation (BatchInbox contract calling BatchAuthenticator.validateBatch) to off-chain verification in the derivation pipeline via BatchInfoAuthenticated event scanning. Key changes: - Add batch_authenticator.go: event-based auth using CollectAuthenticatedBatches which scans a lookback window once per L1 block and returns authenticated hashes - Add isBatchTxAuthorized helper shared by calldata and blob data sources - Remove BatchInbox contract, interface, tests, bindings, and snapshots entirely (Espresso-introduced, never in Celo fork) - Remove validateBatch/validateTeeBatch/validateNonTeeBatch from BatchAuthenticator (authenticateBatchInfo and signer management remain) - Regenerate BatchAuthenticator Go bindings - Revert BatchInbox-related fields from L1Deployments, ChainState, DeployEspressoOutput, and calculateBatchInboxAddr back to upstream signatures (introduce+remove no-ops) - Update integration/devnet tests for EOA BatchInbox behavior Co-authored-by: OpenCode <noreply@opencode.ai> * Add FallbackBatcherAddress to derivation pipeline for non-TEE batcher auth When batch authentication is enabled, the TEE batcher requires a matching BatchInfoAuthenticated event on L1. The fallback (non-TEE) batcher does not call authenticateBatchInfo, so its transactions had no auth events and were rejected by the derivation pipeline. Add FallbackBatcherAddress to rollup.Config and the derivation pipeline's DataSourceConfig. When event-based auth finds no matching event for a batch, it falls back to sender verification against the fallback batcher address. This allows the non-TEE batcher to post batches without on-chain auth events while still requiring event-based auth for the TEE batcher. Co-authored-by: OpenCode <noreply@opencode.ai> * Remove validBatchInfo mapping from BatchAuthenticator The validBatchInfo mapping was written to on every authenticateBatchInfo call but no longer read by the derivation pipeline, which now uses BatchInfoAuthenticated events instead. Removing it saves ~20k gas per call. - Remove mapping declaration and write from BatchAuthenticator.sol - Remove from IBatchAuthenticator interface - Remove test assertions on validBatchInfo - Update SECURITY_ANALYSIS.md to reflect event-based auth model - Bump contract version to 1.1.0 - Regenerate ABI/storage snapshots and Go bindings Co-authored-by: OpenCode <noreply@opencode.ai> * Comments * Fix txmgr wrapper * Fix nonce collision: use separate account for fallback batcher The fallback batcher (op-batcher-fallback) was using Anvil account #3 (0x90F79bf6EB2c4f870365E785982E1f101E93b906), which is also the Deployer key used by devnet tests for admin transactions (SwitchBatcher, TransferOwnership, etc.). When both the fallback batcher service and test code sign L1 transactions from the same account concurrently, nonce collisions cause transactions to never be mined, leading to timeouts. Switch the fallback batcher to Anvil account #6 (0x976EA74026E726554dB657fA54763abd0C3a0aa9) across docker-compose, prepare-allocs (nonTeeBatcher in rollup config), and .env. Co-authored-by: OpenCode <noreply@opencode.ai> * Add batcher-side idle check: inactive batcher skips publishing Query BatchAuthenticator.activeIsTee() before publishing to L1. The inactive batcher (TEE or fallback) now skips publishStateToL1 instead of posting transactions that would be ignored by the derivation pipeline. This was previously unnecessary because the BatchInbox contract's validateBatch() would revert inactive batcher transactions during eth_estimateGas. With the move to an EOA batch inbox, both batchers can freely post, so an explicit idle check is needed. Co-authored-by: OpenCode <noreply@opencode.ai> * Fix event signature mismatch: BatchInfoAuthenticated(bytes32) not (bytes32,address) PR #363 removed the 'address indexed signer' parameter from the BatchInfoAuthenticated event, but the Go derivation pipeline still used the old 2-parameter signature. The keccak256 hash of the wrong signature never matched actual log topics, causing CollectAuthenticatedBatches to find zero events and reject every batch. - Fix ABI string in batch_authenticator.go - Update test mocks to use 2-topic logs matching the actual event - Regenerate Go bindings from fresh forge artifacts Co-authored-by: OpenCode <noreply@opencode.ai> * Cache L1BlockRef lookups in batch auth lookback window traversal CollectAuthenticatedBatches walks backwards ~100 L1 blocks per call, making an L1BlockRefByHash RPC call for each step. Since consecutive L1 blocks share ~99 blocks in their lookback windows, these calls are almost entirely redundant after the first traversal. Add a second global LRU cache (blockRefCache) mapping block hash to L1BlockRef, alongside the existing receipt cache. On steady state this reduces L1BlockRefByHash RPC calls from ~100 per L1 block to ~0-1, a ~50x reduction in total RPC overhead for the batch auth path. Co-authored-by: OpenCode <noreply@opencode.ai> * Increase batcher switch stabilization delay to 60s in TestBatcherActivePublishOnly In-flight sendTxWithEspresso goroutines spawned before deactivation can take ~25s to drain their queued Txmgr.Send calls. The previous 10s delay was insufficient, causing stale TEE batcher transactions to appear in the post-switch monitoring window and failing the assertion. Co-authored-by: OpenCode <noreply@opencode.ai> * Reduce lookback window * Pin eth2-val-tools * Update succinct versions --------- Co-authored-by: OpenCode <noreply@opencode.ai> # Conflicts: # espresso/docker-compose.yml # op-batcher/batcher/driver.go # op-e2e/system/e2esys/setup.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes Batch Authenticator to use the verify function of the Espresso tee contracts