feat: pro 541 improve wrapper deployment - #245
Conversation
|
@claude review |
|
Claude timed out while processing the request. — View run |
| // Get the proxy deployment name for a confidential wrapper | ||
| export function getConfidentialWrapperProxyName(tokenName: string): string { | ||
| return `ConfidentialWrapper_${tokenName}_Proxy`; | ||
| export function getConfidentialWrapperUpgradeImplName(label: string, name?: string): string { |
There was a problem hiding this comment.
I think instead f label, version or at least versionTag would be a bit more precise.
Also not sure why we consider an (optiona ?) name here and not a symbol like you've updated for the other functions
There was a problem hiding this comment.
I wanted to include the option for a name here because of the planned treasury changes that will likely require us to have multiple (probably 2) implementations at the same time per version.
I think versionTag makes sense here!
| // --blocked-users '["0x1111111111111111111111111111111111111111"]' \ | ||
| // --underlying-deny-list-selector "0xfe575a87" \ | ||
| // --network testnet | ||
| // --initial-observers '[]' \ |
| const initialObservers = | ||
| initialObserversEnv === undefined || initialObserversEnv.trim() === '' | ||
| ? [] | ||
| : (JSON.parse(initialObserversEnv) as string[]); |
| // npx hardhat task:deployConfidentialWrapperImpl --network testnet | ||
| async function deployConfidentialWrapperImpl(hre: HardhatRuntimeEnvironment) { | ||
| const { getNamedAccounts, ethers, deployments, network } = hre; | ||
| // Deploy a bare ConfidentialWrapper implementation (no proxy), for an upgrade proposal: deploy it, |
There was a problem hiding this comment.
this comment is obsolete / not true
There was a problem hiding this comment.
Removed the obsolete comment here: 47ed7b7#diff-ad221c925ae48c23ae1871c40a16a5020c07987aef1e163416980a7cbc1fdf1aL237
|
since claude did not want to review here, I made it review locally. I removed some findings so hence the wrong ordering. Basically: I agree I'm not convinced about the new 0.8.29 compiler version added + let's make sure the verify change makes sense: HIGH1.
|
|
Regarding the claude review:
|
738bac5 to
975375a
Compare
|
Regarding the Claude review: HIGH
The summary message is split into its individual MEDIUM
Required env vars restored for the batch path, and the CLI task keeps an explicit
The task moved into
Dropped the 0.8.29 compiler entirely back to a single
Now
No change, this branch is what starts committing those artifacts. LOW
The override is gone, each network reads only its own
No change. It is intentionally exported as a hook so the internal repo can plug in its own account derivation instead of us reproducing it here.
The whole
README now says
|
Introduce deploy-params/<tier>/<network> as the reviewed source of deploy intent, rename Hardhat networks to ethereum/sepolia, and pass V4 initialObservers through deploy tasks so initialize arity matches main.
…e and version label
… testnet wrapper configurations
…E for network URLs
975375a to
e05b4b1
Compare
2 participants
Summary
Introduces a params-driven deployment workflow for ConfidentialWrapper, so live deployments are driven by reviewed, committed JSON config rather than ad hoc
.envvalues. Hardhat tasks and network naming are updated to support this model, and documentation is consolidated around the new entry process.deploy-params/withnetwork.jsonandwrappers.jsonfor Ethereum mainnet and Sepolia, seeded with all existing wrapper configurations (includinginitialObserversand deny-list selectors).deployParams.tsto resolve network tier, load network config, and look up wrapper entries by underlying address.--name/--labelontask:deployConfidentialWrapperImpl, and removal of redundant batch implementation deploy/verify tasks.mainnet/testnettoethereum/sepolia; enable dual Solidity compilers (0.8.27 + 0.8.29) to support proxy verification; commit live-network deployment and OpenZeppelin manifests.