Fix/testnet4 review followup#180
Closed
lionakhnazarov wants to merge 33 commits into
Closed
Conversation
…rease in TokenStaking - Introduced method in and to allow governance to approve applications. - Added method to enable staking providers to increase their authorization for applications. - Updated deployment scripts for TokenStaking upgrade and added new scripts for operator keystore management. - Modified to include a new script for upgrading token staking. - Created new JSON file for TokenStaking ABI and removed outdated deployment data.
- Updated the approveApplication function to include the onlyGovernance modifier, ensuring that only governance can approve applications.
… for Yarn - Upgraded actions/setup-node from v3 to v4 across multiple workflow files. - Added a step to enable Corepack for Yarn in the workflows, ensuring compatibility with the package manager specified in the project.
…ld-network/solidity-contracts into feat/testnet4-deployment-support
- Removed comments from .yarnrc.yml to streamline configuration. - Updated packageManager in package.json to Yarn version 4.12.0 for improved performance and features.
…docs file - Changed the workflow to use the local path for reusable-solidity-docs.yml instead of the remote repository reference. - Ensured that both documentation generation and publishing jobs point to the correct local file.
- Remove private key logging from create-operator-keystore and get-operator-key; address is sufficient for confirmation - Remove staking provider and operator private keys from generated .env files; keys written to disk are a git-leak risk - Print staking provider key once to terminal with a prominent "copy now" warning instead of persisting it to .env - Require non-empty password in create-operator-keystore and setup-new-staking-provider; empty-password keystores are trivially decryptable - Require explicit keystore path in get-operator-key; remove the hardcoded developer-machine UUID default that caused ENOENT for all other users - Fix --list path in get-operator-key: ../../operator-1-keystore resolved above repo root; corrected to ../operator-1-keystore - Convert sync fs calls to fs.promises and add try/catch inside main() across all three scripts
…args Passing --private-key as a CLI argument exposes the key in ps aux output and persists in shell history. Using ETH_PRIVATE_KEY as an inline env assignment (ETH_PRIVATE_KEY="$key" cast send ...) keeps the key out of the argv list. Introduce _sp_cast_send_ok / _op_cast_send_ok wrappers in run-new-operator-setup.sh that inject ETH_PRIVATE_KEY for the respective signer, and replace all --private-key flag usages. Update fund-new-operator.sh likewise for the deployer key. Update run-new-operator-setup.sh usage comment to reflect that NEW_STAKING_PROVIDER_KEY and NEW_OPERATOR_KEY are no longer written to .env files and must be exported by the operator.
- Drop kind: "transparent" from upgradeProxy options; let the OZ
plugin infer the proxy type from the deployed proxy admin slot.
Hardcoding the kind risks a mismatch if the original deploy
defaulted differently. Add a comment with the cast storage
command to verify proxy type on-chain.
- Replace two inline const fs = require("fs") declarations with a
single top-level import * as fs from "fs" to match TypeScript
conventions and avoid the duplicate binding.
The file was removed in the parent branch commit without explanation. Downstream consumers relying on deployments/mainnet/TokenStaking.json break silently without it. Restored from the last known-good version (commit ab29e02).
approveApplication already checks application != address(0) but increaseAuthorization did not. The APPROVED status check provides a functional backstop, but adding the explicit guard makes the invariant consistent across both entry points.
…and-correctness fix: security and correctness follow-ups for Sepolia operator tooling (PR #176)
- Added new entries to .gitignore for generated operator setup artifacts. - Updated deployment scripts to create a directory for network-specific deployments and save TokenStaking deployment data in both the root and network-specific directories. - Refactored authorization and registration commands in setup scripts to use environment variables for private keys, improving security and readability. - Modified upgrade script documentation to reflect the correct command usage from the repository root.
…ld-network/solidity-contracts into feat/testnet4-deployment-support
- Introduced a function to remove CRLF and whitespace from environment variables, preventing decoding errors. - Updated to ensure that the is not overwritten by stale values. - Modified to store both staking provider and operator private keys in the environment file for automated setups, while ensuring sensitive information is not logged unnecessarily. - Added error handling for missing keys in the generated environment files.
- Added validation to ensure ETH_PRIVATE_KEY is set before sending transactions. - Introduced a mechanism to prevent overwriting the deployer key with stale values from environment files. - Updated the script to maintain the correct private key for the contract owner during operator setup.
- Updated prerequisites for deploying operators to include AUTO_FUND_T for automatic minting of T tokens. - Added a function to compute T token shortfall and validate the deployer's balance. - Implemented error handling for insufficient T balance and ensured proper private key management for minting. - Introduced normalization for addresses to improve consistency in key comparisons.
… functionality - Added a function to compute the shortfall of T tokens and validate the deployer's balance. - Implemented error handling for insufficient T balance and ensured proper private key resolution for minting. - Updated prerequisites to require python3 for balance checks and minting operations. - Improved address normalization for better key comparison consistency.
- Removed specific ignore rules for deployments in .gitignore to allow for better management of deployment files. - Added new deployment files for Sepolia, including .chainId, NuCypherToken.json, T.json, TokenStaking.json, and VendingMachineNuCypher.json, to support the latest contract deployments. - Enhanced the operator setup script to improve handling of ETH funding for new operators, including adjustable parameters for ETH allocation and better error handling for deployment paths. - Updated prerequisites and usage instructions in the setup script for clarity and improved user experience.
- Updated contract addresses and transaction hashes in deployment files for NuCypherToken, T, TokenStaking, and VendingMachineNuCypher to reflect the latest deployments on Sepolia. - Enhanced the operator setup script with new functions for computing ETH shortfalls and converting values to decimal, improving error handling for deployer funding requirements. - Added checks to ensure the deployer has sufficient ETH for operator bootstrap, enhancing the robustness of the deployment process.
- Updated contract addresses and transaction hashes in NuCypherToken.json, T.json, TokenStaking.json, and VendingMachineNuCypher.json to reflect the latest deployments on Sepolia. - Adjusted transaction indices and block numbers to align with the new deployment data, ensuring accurate tracking of contract interactions.
- Add kind: "transparent" to upgradeProxy call to match upgrade-token-staking.ts and eliminate dependency on OZ manifest for proxy-type detection - Remove unused deployer variable from getNamedAccounts destructuring - Replace sync fs calls with fs.promises equivalents - Wrap JSON.parse in try/catch
Plaintext private keys written to .env.operator-N were world-readable under the default umask. chmod 0600 limits access to the current user.
…ddress Keys passed via --private-key are visible in /proc/<pid>/cmdline and ps aux during execution. Use ETH_PRIVATE_KEY env var instead, which Foundry's cast reads without exposing it in the process table.
The PR introduced a reformatted version that dropped 4 constructor parameters (_keepStakingContract, _nucypherStakingContract, _keepVendingMachine, _keepStake), leaving a 2-param ABI that does not match the deployed contract at 0x01B67b...765dd7. Restore the original 6-param ABI from main so downstream tooling is not broken.
run-new-operator-setup.sh had a cast_send_ok without retry logic; setup-multiple-operators.sh had one with exponential backoff. Nonce-lag errors that were handled transparently in the multi-operator script would fail silently in the single-operator script. Extract the full retry implementation into scripts/lib/cast-helpers.sh and source it from both scripts.
--list was calling fromEncryptedJson with an empty password to extract the address. Keystore JSON contains an unencrypted "address" field; reading it directly avoids spurious decryption errors for keystores with real passwords and removes the dependency on the ethers decrypt path.
- Remove hardcoded fallback T token address; fail loudly with a clear error if T.json is missing rather than silently using a stale address - Quote all shell variables in the cast send call to prevent word splitting on RPC URLs that include API key query parameters
07_deploy_token_staking.ts was writing to both the root TokenStaking.json and deployments/<network>/TokenStaking.json. After the upgrade script runs, only the network-specific file is updated, leaving the root file with a stale ABI. Write only to deployments/<network>/ and delete the root-level artifact that was previously committed. Also fix pre-existing issues in the file: use strict equality, static import for fs, const for jsonAbi, async fs.promises, and try/catch around JSON.parse.
…s.sh to use positional arguments for cast wallet address - Refactored fund-new-operator.sh to utilize cast_send_ok for sending tokens, improving error handling. - Modified setup-multiple-operators.sh to pass private keys as positional arguments to cast wallet address, addressing security concerns with environment variable exposure.
Add a .gitignore exception for unknown-11155111.json so the OpenZeppelin manifest is versioned like mainnet layout history, avoiding "Manifest not found" on fresh clones when running Sepolia upgrades. Co-authored-by: Cursor <cursoragent@cursor.com>
lionakhnazarov
requested review from
lukasz-zimnoch,
pdyraga and
vzotova
as code owners
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
3 participants
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.