Add TRP and Matic voting items (mvp) #1550
Open
Conversation
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
|
Hardhat Unit Tests Coverage SummaryDiff against masterResults for commit: 461ad23 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
tamtamchik
reviewed
Nov 10, 2025
contracts/upgrade/V3VoteScript.sol
Outdated
Comment on lines
11
to
12
| import {Duration} from "./utils/Duration.sol"; | ||
| import {Timestamp} from "./utils/Timestamp.sol"; |
Member
There was a problem hiding this comment.
Do we need a Duration and Timestamp here? Looks like we can just use plain types uint32 and uint40?
Member
There was a problem hiding this comment.
Do we need to remove the file?
Contributor
Author
There was a problem hiding this comment.
No more V3 upgrades on Hoodi, so why keep it?
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the V3 upgrade mechanism to use a time-based constraint system instead of a simple expiration timestamp. The upgrade can now only be executed within specific time windows, enforced by a predeployed TimeConstraints contract.
- Replaces the simple
EXPIRE_SINCE_INCLUSIVEtimestamp with a comprehensive time constraint system that includes disabled before/after dates and daily time windows - Adds new vote script actions for treasury transfers, staking module updates, and Easy Track registry configuration
- Removes support for the Hoodi testnet configuration
- Refactors time delay calculations to fetch values dynamically rather than caching them
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/upgrade/V3Template.sol | Replaces expiration timestamp with time constraint parameters (DISABLED_BEFORE, DISABLED_AFTER, ENABLED_DAY_SPAN_START, ENABLED_DAY_SPAN_END) |
| contracts/upgrade/V3VoteScript.sol | Adds time constraint checks, treasury transfer actions, staking module updates, and Easy Track registry configuration to vote items |
| contracts/upgrade/V3Addresses.sol | Adds treasury transfer and TRP-related parameters to the addresses struct |
| contracts/upgrade/utils/Timestamp.sol | Introduces new Timestamp user-defined type with comparison operations and helper functions |
| contracts/upgrade/utils/Duration.sol | Introduces new Duration user-defined type with arithmetic operations and conversion utilities |
| scripts/upgrade/steps/0200-deploy-v3-upgrading-contracts.ts | Updates deployment script to pass new parameters including time constraints to V3Template and V3VoteScript |
| scripts/upgrade/steps/0500-mock-v3-aragon-voting.ts | Adds helper functions to calculate valid upgrade timestamps and set blockchain time accordingly for tests |
| scripts/upgrade/steps/0100-deploy-v3-contracts.ts | Updates OracleReportSanityChecker deployment to use new simulatedShareRateDeviationBPLimit parameter |
| scripts/upgrade/upgrade-params-mainnet.toml | Adds comprehensive configuration for sanity checker, staking module, treasury transfers, Easy Track TRP registry, and time constraints |
| scripts/upgrade/upgrade-params-hoodi.toml | Removes Hoodi testnet configuration file entirely |
| scripts/utils/upgrade.ts | Refactors to fetch timelock delays dynamically instead of caching them |
| lib/config-schemas.ts | Updates schema to include sanityChecker, timeConstraints, and additional v3VoteScript parameters |
| tasks/validate-configs.ts | Adds validation rules for new configuration parameters in the expected missing list |
| test/integration/upgrade/upgrade-template-v3.integration.ts | Updates tests to use setValidUpgradeTimestamp helper and adds tests for time constraint immutables |
| package.json | Removes test scripts for Hoodi testnet configuration |
| .gitmodules | Adds dual-governance submodule reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let agentSigner: HardhatEthersSigner; | ||
| let agentMock: V3Template__Harness; | ||
|
|
||
| // Helper function to set timestamp within valid upgrade window |
There was a problem hiding this comment.
The comment "Helper function to set timestamp within valid upgrade window" is misleading as it's placed above the before hook which doesn't call the helper function setValidUpgradeTimestamp. The actual calls to the helper are in the individual tests. Consider removing this comment or moving it to where the function is defined/called.
Suggested change
| // Helper function to set timestamp within valid upgrade window | |
package.json
Outdated
| "test:integration:upgrade:helper-hoodi": "cp deployed-hoodi.json deployed-hoodi-upgrade.json && NETWORK_STATE_FILE=deployed-hoodi-upgrade.json UPGRADE_PARAMETERS_FILE=scripts/upgrade/upgrade-params-hoodi.toml MODE=forking UPGRADE=true GAS_PRIORITY_FEE=1 GAS_MAX_FEE=100 DEPLOYER=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 hardhat test --trace --disabletracer", | ||
| "test:integration:upgrade-template": "cp deployed-mainnet.json deployed-mainnet-upgrade.json && NETWORK_STATE_FILE=deployed-mainnet-upgrade.json UPGRADE_PARAMETERS_FILE=scripts/upgrade/upgrade-params-mainnet.toml MODE=forking TEMPLATE_TEST=true GAS_PRIORITY_FEE=1 GAS_MAX_FEE=100 DEPLOYER=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 hardhat test test/integration/upgrade/*.ts --fulltrace --disabletracer", | ||
| "test:integration:scratch": "DEPLOYER=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 SKIP_INTERFACES_CHECK=true SKIP_CONTRACT_SIZE=true SKIP_GAS_REPORT=true GENESIS_TIME=1639659600 GAS_PRIORITY_FEE=1 GAS_MAX_FEE=100 yarn test:integration:scratch:helper test/integration/**/*.ts", | ||
| "test:integration:upgrade-template-hoodi": "cp deployed-hoodi.json deployed-hoodi-upgrade.json && NETWORK_STATE_FILE=deployed-hoodi-upgrade.json UPGRADE_PARAMETERS_FILE=scripts/upgrade/upgrade-params-hoodi.toml MODE=forking TEMPLATE_TEST=true GAS_PRIORITY_FEE=1 GAS_MAX_FEE=100 DEPLOYER=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 hardhat test test/integration/upgrade/*.ts --fulltrace --disabletracer", |
There was a problem hiding this comment.
The script test:integration:upgrade-template-hoodi at line 39 references UPGRADE_PARAMETERS_FILE=scripts/upgrade/upgrade-params-hoodi.toml, but the upgrade-params-hoodi.toml file has been deleted in this PR. This script should either be removed or updated to use a different configuration file.
Suggested change
| "test:integration:upgrade-template-hoodi": "cp deployed-hoodi.json deployed-hoodi-upgrade.json && NETWORK_STATE_FILE=deployed-hoodi-upgrade.json UPGRADE_PARAMETERS_FILE=scripts/upgrade/upgrade-params-hoodi.toml MODE=forking TEMPLATE_TEST=true GAS_PRIORITY_FEE=1 GAS_MAX_FEE=100 DEPLOYER=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266 hardhat test test/integration/upgrade/*.ts --fulltrace --disabletracer", |
contracts/upgrade/V3VoteScript.sol
Outdated
Comment on lines
480
to
506
| description: "21. Raise SDVT (MODULE_ID = 2) stake share limit from 400 bps to 430 bps in Staking Router 0xFdDf38947aFB03C621C71b06C9C70bce73f12999", | ||
| call: _forwardCall( | ||
| TEMPLATE.AGENT(), | ||
| TEMPLATE.STAKING_ROUTER(), | ||
| abi.encodeCall(IStakingRouter.updateStakingModule, ( | ||
| params.stakingModuleId, | ||
| params.stakeShareLimit, | ||
| currentModule.priorityExitShareThreshold, | ||
| currentModule.stakingModuleFee, | ||
| currentModule.treasuryFee, | ||
| currentModule.maxDepositsPerBlock, | ||
| currentModule.minDepositBlockDistance | ||
| )) | ||
| ) | ||
| }); | ||
|
|
||
| voteItems[index++] = VoteItem({ | ||
| description: "22. Set spent amount for Easy Track TRP registry 0x231Ac69A1A37649C6B06a71Ab32DdD92158C80b8 to 0 LDO", | ||
| call: _forwardCall( | ||
| TEMPLATE.AGENT(), | ||
| params.easyTrackTrpRegistry, | ||
| abi.encodeCall(IAllowedRecipientsRegistry.unsafeSetSpentAmount, (0)) | ||
| ) | ||
| }); | ||
|
|
||
| voteItems[index++] = VoteItem({ | ||
| description: "23. Set limit for Easy Track TRP registry 0x231Ac69A1A37649C6B06a71Ab32DdD92158C80b8 to 15'000'000 LDO with unchanged period duration of 12 months", |
There was a problem hiding this comment.
The descriptions at lines 480, 497, and 506 contain hardcoded addresses and values. Consider making these descriptions more maintainable by either:
- Deriving them from the actual parameter values
- Removing the hardcoded addresses since they're already in the function call parameters
This will prevent inconsistencies between descriptions and actual values.
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
reviewed
Nov 11, 2025
iamnp
requested changes
Nov 11, 2025
| }); | ||
|
|
||
| votingVoteItems[index++] = VoteItem({ | ||
| description: "11. Transfer 508,106 MATIC from Aragon Agent to Liquidity Observation Lab (LOL) Multisig 0x87D93d9B2C672bf9c9642d853a8682546a5012B5", |
There was a problem hiding this comment.
We in DAO Ops really love specifying things as explicit as possible, so if the addresses were removed because of the Copilot suggestions - I suggest getting them back for explicitness and transparency
Suggested change
| description: "11. Transfer 508,106 MATIC from Aragon Agent to Liquidity Observation Lab (LOL) Multisig 0x87D93d9B2C672bf9c9642d853a8682546a5012B5", | |
| description: "11. Transfer 508,106 MATIC 0x7d1afa7b718fb893db30a3abc0cfc608aacfebb0 from Aragon Agent 0x3e40D73EB977Dc6a537aF587D48316feE66E9C8c to Liquidity Observation Lab (LOL) Multisig 0x87D93d9B2C672bf9c9642d853a8682546a5012B5", |
|
|
||
| voteItems[index++] = VoteItem({ | ||
| description: "1. Call UpgradeTemplateV3.startUpgrade", | ||
| description: "1.1. Check DG voting enactment is after December 4, 2025 00:00:00 UTC", |
There was a problem hiding this comment.
Suggested change
| description: "1.1. Check DG voting enactment is after December 4, 2025 00:00:00 UTC", | |
| description: "1.1. Ensure DG proposal execution is after December 4, 2025 00:00:00 UTC", |
| }); | ||
|
|
||
| voteItems[index++] = VoteItem({ | ||
| description: "1.2. Check DG voting enactment is before December 10, 2025 00:00:00 UTC", |
There was a problem hiding this comment.
Suggested change
| description: "1.2. Check DG voting enactment is before December 10, 2025 00:00:00 UTC", | |
| description: "1.2. Ensure DG proposal execution is before December 10, 2025 00:00:00 UTC", |
| }); | ||
|
|
||
| voteItems[index++] = VoteItem({ | ||
| description: "1.3. Check DG voting enactment is within daily time window (14:00 UTC - 23:00 UTC)", |
There was a problem hiding this comment.
Suggested change
| description: "1.3. Check DG voting enactment is within daily time window (14:00 UTC - 23:00 UTC)", | |
| description: "1.3. Ensure DG proposal execution is within daily time window (14:00 UTC - 23:00 UTC)", |
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.