Add Dual Governance to scratch deploy - #1816
Open
arwer13 wants to merge 52 commits into
Open
Conversation
|
Hardhat Unit Tests Coverage SummaryDetailsDiff against masterResults for commit: fe5aa49 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
…ling - justfile: scratch/mainnet-fork/sepolia-fork deploy+test matrix (with/without DG) - docs: testing landscape, scratch-deploy refresh, external-node (anvil vs hardhat) compat - in-process scratch forces DG off (can't broadcast forge); run-logged quiet logging
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves scratch deployment and integration testing across backends (in-process Hardhat vs external nodes like anvil/hardhat-node), adds Dual Governance (DG) scratch deploy support (via Foundry submodule), and introduces Sepolia-specific handling for the beacon deposit contract via an adapter plus test skips for unsupported variable deposit amounts.
Changes:
- Add DG scratch deploy step (0160) backed by a Foundry submodule, plus role/permission handoff updates and new DG integration coverage.
- Improve external-node test reliability (snapshot restore guards, revert-message assertions, deterministic role signers, timestamp/time-warp guards, and mined-tx waits).
- Add Sepolia deposit adapter wiring + provisioning/funding logic, and skip variable-deposit flows on Sepolia.
Reviewed changes
Copilot reviewed 62 out of 63 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/suite/snapshot.ts | Guard snapshot restore when suite snapshot was never taken |
| test/integration/vaults/scenario/pdg-paused-happy-path.integration.ts | Add time nudges to keep timestamp assertions meaningful on external nodes |
| test/integration/vaults/scenario/pdg-deposit.integration.ts | Skip scenario when variable deposit amounts aren’t supported |
| test/integration/vaults/scenario/node-operator.happy-path.integration.ts | Skip scenario when variable deposit amounts aren’t supported |
| test/integration/vaults/scenario/happy-path.integration.ts | Skip PDG-related test when variable deposit amounts aren’t supported |
| test/integration/vaults/pdg.integration.ts | Skip PDG shortcut test when variable deposit amounts aren’t supported |
| test/integration/vaults/disconnected.integration.ts | Skip beacon-deposit tests when variable deposit amounts aren’t supported |
| test/integration/vaults/dashboard.integration.ts | Skip beacon-deposit test when variable deposit amounts aren’t supported |
| test/integration/report-validator-exit-delay.ts | Add +1s time warp margin for inclusive threshold behavior on external nodes |
| test/integration/dual-governance/dg-scratch.integration.ts | New DG “scratch launch state” integration coverage |
| test/integration/core/second-opinion.integration.ts | Factor Sepolia constants into shared module |
| test/integration/core/negative-rebase.integration.ts | Factor Sepolia constants into shared module |
| test/hooks/assertion/revertedWithOZAccessControlError.ts | Make revert assertion backend-agnostic (Hardhat vs external node wording) |
| test/0.8.9/ISepoliaDepositContract.sol | Remove Sepolia deposit contract interface from test tree |
| scripts/utils/upgrade.ts | Add reusable DG proposal execution helper with time-constraint retries + Fusaka gas-cap checks |
| scripts/scratch/steps/0160-deploy-dual-governance.ts | New DG scratch deploy + wiring step (forge deploy, state persistence, permission finalize) |
| scripts/scratch/steps/0150-transfer-roles.ts | Defer renounce on sealables when DG enabled; include Sepolia deposit adapter ownership/admin transfer; move template ownership transfer to 0160 |
| scripts/scratch/steps/0145-unpause-sealables.ts | New step to resume sealables prior to DG registration |
| scripts/scratch/steps/0010-deploy-deposit-contract.ts | Deploy Sepolia deposit adapter on Sepolia chainId; otherwise deploy normal deposit contract |
| scripts/scratch/steps.json | Insert new scratch steps 0145 and 0160 |
| scripts/scratch/deploy-params-testnet.toml | Add [dualGovernance] scratch configuration section |
| scripts/run-logged.sh | Add filtered logging wrapper with heartbeat and summary |
| scripts/dao-sepolia-fork-deploy.sh | Add Sepolia fork deploy helper using run-logged wrapper |
| scripts/dao-local-deploy.sh | Add run-logged wrapper and switch to in-process fork test flow |
| scripts/dao-deploy.sh | Force-disable extra checks (gas report/size/interface check) during deploy |
| scripts/archive/stvaults/steps/0500-mock-v3-aragon-voting.ts | Switch to new DG fork proposal execution helper |
| package.json | Add “agent” logged test scripts; ensure scratch helper disables DG by default |
| lib/state-file.ts | Add DG + Sepolia adapter keys; add tryGetAddress; adjust getAddress rules for DG entries |
| lib/scratch.ts | Add robust DG enable/disable env parsing helper |
| lib/protocol/types.ts | Add supportsVariableDepositAmounts flag to protocol context |
| lib/protocol/sepolia.ts | Add Sepolia chain constants (chainId, deposit contract, BEPOLIA whale) |
| lib/protocol/provision.ts | Provision Sepolia deposit adapter funding and clarify predeploy semantics |
| lib/protocol/networks.ts | Provide EasyTrack fallback for scratch state files in forking mode |
| lib/protocol/helpers/vaults.ts | Make roles deterministic across external nodes; fund role accounts; compute safe report timestamps for external nodes |
| lib/protocol/helpers/sepolia-deposit-adapter.ts | Add helper to fund Sepolia deposit adapter with BEPOLIA |
| lib/protocol/helpers/index.ts | Export Sepolia deposit adapter funding helper |
| lib/protocol/helpers/dsm.ts | Await DSM addGuardians tx receipt for deterministic reads |
| lib/protocol/helpers/accounting.ts | Fix HashConsensus initial epoch init to be chain-config-derived and mined; nudge time when initial epoch hasn’t arrived |
| lib/protocol/context.ts | Treat PROVISION_ON_FORK as “scratch semantics”; add Sepolia support flag; provision on scratch/forked scratch |
| lib/index.ts | Restore stable export ordering for ./string |
| lib/eips/predeploy.ts | Add shared helper to ensure predeploy bytecode exists without overwriting unexpected code |
| lib/eips/eip7002.ts | Use shared predeploy helper instead of bespoke code checks |
| lib/eips/eip4788.ts | Add EIP-4788 runtime bytecode constant and ensure it via shared predeploy helper |
| lib/constants.ts | Remove hardcoded HashConsensus far-future epoch constant |
| lib/config-schemas.ts | Add Zod schema for [dualGovernance] section with cross-field validation |
| lib/account.ts | Wrap impersonated signers on external nodes to wait for receipts (deterministic provisioning) |
| justfile | Add reproducible deploy/test recipes across blank nodes and forks (with/without DG), routed through run-logged |
| hardhat.config.ts | Add env-driven hardhat chainId; increase RPC timeouts; add solc settings override to keep template under size limit |
| foundry/lib/dual-governance | Add DG Foundry submodule pointer |
| docs/testing.md | New doc: testing modes, topologies, and recommended commands |
| docs/scratch-deploy.md | Expand scratch deploy docs (DG, Sepolia fork, output modes, verification guidance) |
| docs/external-node-test-compat.md | New doc: external node compatibility and hardhat-node fork-of-fork caveats |
| deployed-mainnet.json | Update DG entries to { address } (non-proxy) layout |
| deployed-hoodi.json | Update DG entries to { address } (non-proxy) layout |
| contracts/upgrade/interfaces/IEmergencyProtectedTimelock.sol | Add missing view methods used by tests/helpers |
| contracts/0.4.24/template/LidoTemplate.sol | Add DG/non-DG permission finalization functions; adjust permission manager flows |
| accounts.sample.json | Add sample accounts config file for network private keys |
| .gitmodules | Add DG submodule entry |
| .gitignore | Ignore logs/ and editor swap files |
| .github/workflows/tests-integration-scratch.yml | Expand scratch CI matrix (with/without DG; blank and Sepolia fork), update node image, checkout submodules |
| .github/workflows/tests-integration-hoodi.yml | Pin Hoodi fork block number for pre-upgrade v3 artifact compatibility |
| .github/workflows/setup/action.yml | Install Foundry toolchain in CI setup |
| .env.example | Document DG toggle and optional SKIP_* env vars |
Comments suppressed due to low confidence (3)
scripts/scratch/steps/0010-deploy-deposit-contract.ts:1
state.chainIdis likely not guaranteed to be a number (elsewhere it’s cast viaNumber(state.chainId)). If it’s a string, this strict comparison won’t match and Sepolia will incorrectly deploy the regular DepositContract. Convertstate.chainIdto a number (or compare as BigInt consistently) before branching.
scripts/dao-deploy.sh:1- These unconditional exports override user intent and make it impossible to run a deploy with gas report / contract size / interface checks enabled. Prefer setting defaults only when the variables are unset (so callers can opt back in), or move these defaults to the higher-level convenience scripts (
dao-local-deploy.sh, CI) rather than the core deploy entrypoint.
test/integration/core/second-opinion.integration.ts:1 - The variable name
bepiloaSignerlooks like a typo (likely intendedbepoliaSigner). Renaming would improve readability and reduce confusion when scanning Sepolia-specific setup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+16
to
+19
| const [adapterAddress, bepolia] = await Promise.all([ | ||
| ctx.contracts.stakingRouter.DEPOSIT_CONTRACT(), | ||
| ethers.getContractAt("ISepoliaDepositContract", SEPOLIA_ORIGINAL_DEPOSIT_CONTRACT), | ||
| ]); |
Comment on lines
+17
to
+20
| - name: Install Foundry | ||
| uses: foundry-rs/foundry-toolchain@v1 | ||
| with: | ||
| version: stable |
Member
There was a problem hiding this comment.
This is redundant, by default version in stable
tamtamchik
reviewed
Jun 9, 2026
tamtamchik
left a comment
tamtamchik
left a comment
Member
There was a problem hiding this comment.
Wow, it's huge. Thank you! Didn't finish yet, but left some comments.
| isScratch, | ||
| isMainnet: contracts.locator.address.toLowerCase() === MAINNET_LOCATOR_ADDRESS.toLowerCase(), | ||
| // see the comment on this field in ProtocolContext (lib/protocol/types.ts) | ||
| supportsVariableDepositAmounts: chainId !== BigInt(SEPOLIA_CHAIN_ID), |
Member
There was a problem hiding this comment.
supportsVariableDepositAmounts keys off chainId === SEPOLIA_CHAIN_ID, but a Sepolia fork reports 31337 → false, while the deposit adapter is the same. Is the fork case covered?
| [submodule "foundry/lib/forge-std"] | ||
| path = foundry/lib/forge-std | ||
| url = https://github.com/foundry-rs/forge-std | ||
| [submodule "foundry/lib/dual-governance"] |
Member
There was a problem hiding this comment.
Is the dual-governance submodule pin (ba9dfc9…) a released/audited ref or an arbitrary branch tip?
|
|
||
| export SKIP_GAS_REPORT=true | ||
| export SKIP_CONTRACT_SIZE=true | ||
| export SKIP_INTERFACES_CHECK=true |
Member
There was a problem hiding this comment.
SKIP_*=true is now unconditional for all callers who use this file — intended?
Contributor
Author
There was a problem hiding this comment.
yes, these are dev checks - no need to redo them and bloat the output on deployment
| triggerableWithdrawalsGateway: TriggerableWithdrawalsGatewaySchema, | ||
| predepositGuarantee: PredepositGuaranteeSchema.omit({ genesisForkVersion: true }), | ||
| operatorGrid: OperatorGridSchema, | ||
| dualGovernance: DualGovernanceConfigSchema.optional(), |
Member
There was a problem hiding this comment.
dualGovernance is .optional(). If DG is enabled but this section is missing, does the 0160 guard fire before the schema would otherwise pass it through?
| dgConfigProvider = "dg:configProvider", | ||
| dgEmergencyGovernance = "dg:emergencyGovernance", | ||
| dgTiebreakerCoreCommittee = "dg:tiebreakerCoreCommittee", | ||
| dgTiebreakerSubCommittees = "dg:tiebreakerSubCommittees", |
Member
There was a problem hiding this comment.
getAddress(Sk.dgTiebreakerSubCommittees) not in the switch. Intended?
Comment on lines
+14
to
+19
| # Output mode: by default this SELF-LOGS — the full combined deploy + test | ||
| # output is written to $LOG_FILE (default logs/scratch-deploy-sepolia-fork.log) | ||
| # while only milestones, mocha counts, and failures (plus a periodic heartbeat) | ||
| # reach the terminal. For raw, unfiltered output straight to the terminal: | ||
| # FULL_OUTPUT=1 bash scripts/dao-sepolia-fork-deploy.sh | ||
| # bash scripts/dao-sepolia-fork-deploy.sh --full # equivalent |
Comment on lines
+56
to
+64
| # Run the integration suite against an IN-PROCESS hardhat node that FORKS the | ||
| # anvil we just deployed to (MODE=forking: the default `hardhat` network forks | ||
| # $RPC_URL at latest; the deployment is read from $NETWORK_STATE_FILE). We do | ||
| # NOT drive anvil directly (`--network local`): the suite isolates tests with | ||
| # evm_snapshot/evm_revert plus month-scale time jumps, and that isolation is only | ||
| # reliable on the in-process node. Driving the external anvil over a long run | ||
| # lets snapshot state degrade (cf. the ~6k-block caveat in | ||
| # test/integration/core/dsm-pause-deposits.integration.ts), surfacing as | ||
| # cascading failures and an eventual mid-suite deadlock. |
Member
There was a problem hiding this comment.
Duplication, looks like need to be held somewhere else.
Comment on lines
+8
to
+18
| # Output mode: by default this SELF-LOGS — the full combined deploy + test | ||
| # output is written to $LOG_FILE (default logs/scratch-deploy.log) while only | ||
| # milestones, mocha counts (N passing/pending/failing), and failures reach the | ||
| # terminal. This keeps automation / LLM agents (and humans who just want signal) | ||
| # out of the gas reports, per-tx traces, and ~700 mocha test ticks. The log also | ||
| # emits a periodic heartbeat so a long, quiet phase (e.g. a slow integration | ||
| # test) isn't mistaken for a freeze. | ||
| # | ||
| # For raw, unfiltered output straight to the terminal: | ||
| # FULL_OUTPUT=1 bash scripts/dao-local-deploy.sh | ||
| # bash scripts/dao-local-deploy.sh --full # equivalent |
Member
There was a problem hiding this comment.
Looks like some scratch ideas, do we need it?
Member
There was a problem hiding this comment.
Looks like cab be refactored to separate docs.
- 0145/0160: catch up on deferred role renounces when resuming (leftover RESUME_ROLE, deployer DEFAULT_ADMIN_ROLE after DG is flipped off) - state-mate: resolve RPC per network, compute HC initial epoch from genesisTime, handle Sepolia deposit adapter / chainSpec deposit contract - dg-checks: drop the RPC_URL fallback for forge (in-process node is unreachable) - sepolia fork: set HARDHAT_CHAIN_ID so Sepolia-only paths trigger - reuse isTruthyEnv, warn on missing EasyTrack executor in real forks, drop unused deployEIP4788BeaconBlockRootContract; docs on fork-of-fork limits
…ier-deployment-artifacts chore: add NEST token rate notifier and Lido locator deployment artefacts
…ier-hoodi-deployment chore: add Hoodi TokenRateNotifier deployment artifacts
feat: add token rate pusher with args
Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.14.1 to 3.14.3. - [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst) - [Commits](aio-libs/aiohttp@v3.14.1...v3.14.3) --- updated-dependencies: - dependency-name: aiohttp dependency-version: 3.14.3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 3.14.2 to 3.15.1. - [Changelog](https://github.com/nodeca/js-yaml/blob/3.15.1/CHANGELOG.md) - [Commits](nodeca/js-yaml@3.14.2...3.15.1) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 3.15.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
PROTOCOL_ACTIVATION_ENABLED (default off) adds step 0155, which forwards Lido.resume() + setStakingLimit through LidoTemplate.activateProtocol before the Agent is handed to DG; 0145 also resumes WQ/VEBO.
build(deps-dev): bump aiohttp from 3.14.1 to 3.14.3
…yaml-3.15.1 build(deps): bump js-yaml from 3.14.2 to 3.15.1
On a live hoodi fork the well-known hardhat account #0 already holds ~0.05 stETH (anyone can fund public testnet addresses), which broke three integration tests comparing absolute post-submit/post-mint balances. Compare deltas instead.
Drop TokenRateNotifier NEST deploy and mock-upgrade steps
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.
No description provided.