Repository navigation
Deploy contracts without EOAs involved - #811
Merged
Merged
Conversation
spalladino
marked this pull request as ready for review
June 13, 2023 01:10
spalladino
force-pushed
the
palla/deploy-no-eoa
branch
from
June 13, 2023 11:55
374aee9 to
0316b80
Compare
spalladino
enabled auto-merge (squash)
June 13, 2023 11:56
spalladino
force-pushed
the
palla/deploy-no-eoa
branch
from
June 13, 2023 12:57
0316b80 to
913f442
Compare
ludamad
reviewed
Jun 13, 2023
|
|
||
| boolean operator==(ContractDeploymentData<NCT> const& other) const | ||
| { | ||
| return constructor_vk_hash == other.constructor_vk_hash && function_tree_root == other.function_tree_root && |
Contributor
There was a problem hiding this comment.
Might want to include msgpack_derived_equals.hpp and replace this with return utils::msgpack_derived_equals<boolean(*this, other) while you're here
ludamad
reviewed
Jun 13, 2023
|
|
||
| template <typename NCT> std::ostream& operator<<(std::ostream& os, ContractDeploymentData<NCT> const& data) | ||
| { | ||
| return os << "constructor_vk_hash: " << data.constructor_vk_hash << "\n" |
Contributor
There was a problem hiding this comment.
Same this could be a msgpack derived output
ludamad
reviewed
Jun 13, 2023
| using serialize::read; | ||
|
|
||
| read(it, data.deployer_public_key); | ||
| read(it, data.constructor_vk_hash); |
Contributor
There was a problem hiding this comment.
... TBH I wonder if a practical measure for now is to also derive this from msgpack (thinking aloud, not a review comment). It would give some wins before moving fully over (which is a bit hard to do with being assigned other priority tasks)
ludamad
approved these changes
Jun 13, 2023
ludamad
left a comment
Contributor
There was a problem hiding this comment.
Mechanics look good to me, but a sanity review on the actual issue at hand would be good
iAmMichaelConnor
pushed a commit
that referenced
this pull request
Sep 22, 2026
…on (#25520) ## Independent of the v6 upgrade PR (#25496) This PR is **not** a dependency of, and does not depend on, the v6 upgrade PR #25496. Either can merge first, and neither needs a rebase on the other. - **No shared source.** `DeployRollupForUpgradeV6.s.sol`, `V6UpgradeSimulation.sol` and `V6UpgradePayload.sol` import only `src/` contracts, the generated `HonkVerifier`, and each other. None of them imports `DeployRollupLib`, `RollupConfiguration`, `DeployRollupForUpgrade` or `DeployAztecL1Contracts`, which are the only Solidity files this PR changes. The V6 script's one mention of `RollupConfiguration` is a comment about version derivation, and this PR does not touch that function. - **No shared tooling.** The V6 runbook runs `forge script` on the V6 files directly. It does not use `run_rollup_upgrade.sh`, `test_rollup_upgrade.sh` or `stress_test_deploy.sh`, which are the shell scripts this PR edits. - **No contract change.** Nothing under `src/` changes, so the bytecode v6 deploys is identical with or without this PR. ## Problem `l1-contracts/script/deploy/` is the env-driven deployer for tests, spartan/CLI and testnets (mainnet versions use the pinned `DeployRollupForUpgradeV<N>.s.sol` scripts, which hard-code and verify their configuration). The generic path forwarded whatever the environment supplied straight into the `Rollup` constructor, and the constructor accepts values that leave the deployed instance unusable or mis-bound. Several defaults were also fail-open: a missing `REAL_VERIFIER` bound a `MockVerifier`, and a missing genesis root silently became zero. ## Change All in the deployer, its wrappers and its tests. `script/deploy/RollupConfiguration.sol` - `REAL_VERIFIER` defaults to `true`; a stub verifier must be requested explicitly. - `VK_TREE_ROOT`, `PROTOCOL_CONTRACTS_HASH` and `GENESIS_ARCHIVE_ROOT` must be set and non-zero. - `NETWORK=mainnet` requires chain id 1 and chain id 1 requires `NETWORK=mainnet`. `script/deploy/DeployRollupLib.sol` — `validateRollupConfig`, run before `new Rollup`: - slot, epoch and Ethereum slot durations non-zero; `slot × epoch` fits `uint32` (`TimeLib` multiplies without widening); - `exitDelaySeconds` non-zero; - `lagInEpochsForRandao ≥ 1`, validator-set lag not below it, and lag in seconds fits `uint32`; - each slash amount fits `uint96` (`SlashingProposer` encodes payload amounts as `uint96`); - `rewardDistributor` non-zero; `stakingAsset == GSE.ASSET()`. Two values are deliberately not rejected because the node's test paths rely on them: a zero `targetCommitteeSize` (`ValidatorSelectionLib` treats it as "no committee"; the local network and e2e fixtures use it) and an epoch longer than `MAX_CHECKPOINTS_PER_EPOCH` (e2e suites use 64 and 1000 to keep a run inside epoch 0). `script/deploy/DeployRollupForUpgrade.s.sol` — `REGISTRY_ADDRESS` must have code on the target chain. `scripts/run_rollup_upgrade.sh` no longer needs to override `REAL_VERIFIER`. `scripts/test_rollup_upgrade.sh` and `scripts/stress_test_deploy.sh` export the three genesis roots, and the two script tests set them in `setUp`. The node's TypeScript deployer already passes `REAL_VERIFIER` and all three genesis roots explicitly, and no in-tree, e2e or spartan configuration uses a value these checks reject. ## Tests New `test/script/DeployConfigValidation.t.sol` exercises each check on values via small harnesses rather than through `vm.setEnv`, because forge runs tests in parallel and the process environment is shared; its `setUp` sets the same env values as the other script tests for the same reason. Locally: ``` forge test --match-path 'test/script/*' Ran 3 test suites: 22 tests passed, 0 failed ``` `forge fmt --check` and `forge lint` are clean on the changed files apart from the pre-existing `unsafe-cheatcode` notes for `vm.setEnv`. CI covers the rest of the `l1-contracts` suite and the e2e suite. Not in this PR: wiring an `EscapeHatch` into the generic upgrade script, which needs a configuration surface for the hatch parameters rather than a guard. Closes AztecProtocol/aztec-claude#840 Closes AztecProtocol/aztec-claude#845 Closes AztecProtocol/aztec-claude#1371 Closes AztecProtocol/aztec-claude#688 Closes AztecProtocol/aztec-claude#684 Closes AztecProtocol/aztec-claude#707 Closes AztecProtocol/aztec-claude#865 Closes AztecProtocol/aztec-claude#657 Closes AztecProtocol/aztec-claude#689 Closes AztecProtocol/aztec-claude#1003 Closes AztecProtocol/aztec-claude#1355 Refs AztecProtocol/aztec-claude#686 Refs AztecProtocol/aztec-claude#811 Refs AztecProtocol/aztec-claude#1145 The eight constructor-input findings are closed at the deployer layer on purpose: the constructors in `src/` are unchanged because the mainnet pinned scripts assert these values themselves. If contract-level guards are wanted as well, change those lines to `Refs` before merging. #686 (zero committee) and #811 (epoch above the proof cap) are not closed here: both values are used by the node's own test paths, so any guard for them has to live in the pinned mainnet scripts, which already assert them. --- *Created by [claudebox](https://claudebox.work/v2/sessions/c27ea1045602719d/jobs/43) · group: `slackbot` · requested by Mike (@iAmMichaelConnor) · [Slack thread](https://aztecfoundation.slack.com/archives/D0B2N7W1WJD/p1789737469555569?thread_ts=1789737469.555569&cid=D0B2N7W1WJD)*
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
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.
Removes the need for an EOA address to deploy a contract. However, since the deployer public key is now part of the deployment address (so we can reconstruct it during signature validation instead of hardcoding it, see #662), we do need to include it. We are passing it now as part of the
ContractDeploymentDatastruct.Also, since deployments no longer need to be authenticated, the API in the rpc-server for creating a tx is further simplified. We now just have a
createTxandcreateDeployTxmethods, without the separation betwen creating the request and the tx itself.Fixes #661