Espresso 1: Contracts#443
Conversation
QuentinI
force-pushed
the
ag/contracts
branch
2 times, most recently
from
4bc5939 to
cbd97f9
Compare
Adds the Espresso-introduced contracts and the minimum supporting changes
required for them to compile, test, and pass the contract checks.
New contracts and scripts:
- src/L1/BatchAuthenticator.sol and interfaces/L1/IBatchAuthenticator.sol
(upgradeable contract that authenticates batch transactions, with switching
between Espresso and fallback batchers)
- scripts/deploy/DeployBatchAuthenticator.s.sol and
scripts/deploy/DeployEspresso.s.sol
- test/L1/BatchAuthenticator.t.sol and test/mocks/MockEspressoTEEVerifiers.sol
- snapshots/{abi,storageLayout}/BatchAuthenticator.json
- snapshots/semver-lock.json entry for BatchAuthenticator
New submodules:
- lib/espresso-tee-contracts (interfaces required by BatchAuthenticator)
- lib/openzeppelin-contracts-upgradeable-v5 (OZ v5 used by BatchAuthenticator
via OwnableUpgradeable)
Supporting changes (Espresso-driven):
- foundry.toml: remappings for OZ v5 and espresso-tee-contracts; ignored
warning codes for vendored libs; OOM-safe jobs settings; via-ir profile.
- justfile: fix-proxy-artifact recipe to handle OZ v5 shadowing Proxy/ProxyAdmin
artifacts; build/coverage hooks.
- src/universal/Proxy.sol, src/universal/ProxyAdmin.sol: pin pragma to exact
0.8.15 so they stay in their own compilation group and never emit PUSH0.
- src/universal/ReinitializableBase.sol: loosen pragma to ^0.8.15 so
BatchAuthenticator (compiled with OZ v5) can import it.
- scripts/* and test/*: disambiguate Proxy artifact lookups to
src/universal/Proxy.sol:Proxy (avoids OZ v5 proxy/Proxy.sol shadow).
- scripts/checks: bypass interface checks for artifacts originating from lib/;
add Espresso-related contract names to exclude lists; pragma exclusions for
Proxy/ProxyAdmin/BatchAuthenticator.
- test/vendor/Initializable.t.sol: exclude BatchAuthenticator (deployed by a
separate Espresso script).
Co-authored-by: OpenCode <noreply@opencode.ai>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee2d984108
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| address proxyAdminOwner = _input.proxyAdminOwner(); | ||
| if (proxyAdminOwner == address(0)) proxyAdminOwner = msg.sender; | ||
|
|
||
| vm.broadcast(msg.sender); | ||
| IProxyAdmin proxyAdmin = _deployProxyAdmin(msg.sender); |
There was a problem hiding this comment.
run takes a _deployerAddress and _deployProductionTEEContracts already uses it as the fallback proxyAdminOwner, but deployBatchAuthenticator falls back to msg.sender instead. If this script is invoked through another contract/tooling layer where msg.sender != _deployerAddress, the TEE verifier and BatchAuthenticator will be deployed under different admin ownership assumptions, and BatchAuthenticator ownership can end up on the caller address rather than the intended deployer. Use the same default source for both paths to avoid inconsistent or unintended ownership.
Useful? React with 👍 / 👎.
| import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; | ||
| import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol"; |
There was a problem hiding this comment.
Should these imports be using the v5 openzeppelin contracts?
|
|
||
| function setUp() public { | ||
| // Create a fork of Sepolia using the execution layer RPC endpoint. | ||
| string memory forkUrl = "https://theserversroom.com/sepolia/54cmzzhcj1o/"; |
There was a problem hiding this comment.
This should probably not be hardcoded, is there a way to make this configurable?
|
|
||
| [fuzz] | ||
| runs = 64 | ||
| failure_persist_file = "~/Desktop/failures.txt" |
There was a problem hiding this comment.
Looks like this could be deleted, as the default is cache/fuzz/failures.txt which is a repo relative path.
| vm.prank(proxyAdminOwner); | ||
| authenticator.switchBatcher(); | ||
| assertFalse(authenticator.activeIsEspresso()); | ||
| } |
There was a problem hiding this comment.
The tests currently seem to be missing tests of batch authentication behaviour when running in fallback mode and also a test of authentication behaviour when in fallback and paused.
I've added two tests which I think covers these cases in this commit 5d0a803
| emit BatchInfoAuthenticated(_commitment); | ||
| } | ||
|
|
||
| function registerSigner(bytes calldata _verificationData, bytes calldata _data) external { |
There was a problem hiding this comment.
Could do with a comment, just to clarify that it is secure despite lack of restrictions.
| function registerSigner(bytes calldata _verificationData, bytes calldata _data) external { | |
| /// @notice Permissionless registration of a TEE-generated signer. | |
| /// Anyone may call this; safety is enforced by the verifier: | |
| /// 1. `verificationData` must contain a valid AWS Nitro attestation, verified via Succinct ZK proof. | |
| /// 2. The attestation's PCR0 measurement must match an enclave hash pre-approved by the TEE | |
| /// verifier's owner/guardian. | |
| /// 3. The registered signer address is derived from the public key inside the attestation | |
| /// — the caller cannot choose it. | |
| /// An attacker would need to compromise governance (to whitelist a malicious enclave hash), forge | |
| /// an AWS Nitro signature, or break the Succinct ZK proof — all outside the contract's threat model. | |
| function registerSigner(bytes calldata _verificationData, bytes calldata _data) external { |
3 participants
This PR pulls in the
BatchAuthenticatorpart of Espresso integration atpackages/contracts-bedrock/src/L1/BatchAuthenticator.sol. All other changes are deployment scripts, tests, and other wiring.BatchAuthenticatoris the contract that the batchers will have to authenticate their batches with; corresponding changes for the derivation pipeline and batchers forthcoming as subsequent PRs.