Sepolia TokenStaking proxy deploy, ExtendedTokenStaking upgrade, and operator setup tooling#176
Open
lionakhnazarov wants to merge 35 commits into
Open
Sepolia TokenStaking proxy deploy, ExtendedTokenStaking upgrade, and operator setup tooling#176lionakhnazarov wants to merge 35 commits into
lionakhnazarov wants to merge 35 commits into
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.
lionakhnazarov
requested review from
lukasz-zimnoch,
pdyraga and
vzotova
as code owners
lrsaturnino
reviewed
Apr 13, 2026
Member
lrsaturnino
left a comment
lrsaturnino
left a comment
There was a problem hiding this comment.
Two critical findings from multi-model code review (3/3 models confirmed both).
- 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.
8 tasks
…and-correctness fix: security and correctness follow-ups for Sepolia operator tooling (PR #176)
lrsaturnino
previously approved these changes
Apr 17, 2026
Member
lrsaturnino
left a comment
lrsaturnino
left a comment
There was a problem hiding this comment.
Looks good overall — clean contract changes, CEI ordering solid, access controls in place.
One small nit: the operator setup scripts generate keystores and env files (operator-1-keystore/, spv-maintainer-keystore/, .env.new-operator, .env.operator-*) that aren't covered by .gitignore. Might be worth adding entries to prevent accidental commits.
lrsaturnino
reviewed
Apr 17, 2026
- 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
lrsaturnino
reviewed
Apr 18, 2026
Member
lrsaturnino
left a comment
lrsaturnino
left a comment
There was a problem hiding this comment.
On ExtendedTokenStaking in contracts/test/TokenStakingTestSet.sol — now that approveApplication and increaseAuthorization live on the base TokenStaking, the overrides at lines 227–268 and 280–294 are duplicates. The increaseAuthorization override also doesn't emit AuthorizationIncreased or guard application != address(0), while the base does both.
Since deploy/54 makes ExtendedTokenStaking the live Sepolia implementation, Sepolia will silently stop emitting AuthorizationIncreased (no test asserts it, so CI won't flag it). Dropping both overrides should be enough — the derived contract picks up the canonical behaviour from the base.
- 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.
4 tasks
…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.
…llowup fix: address review findings from #176
cast send and cast wallet address do not honor ETH_PRIVATE_KEY as an environment variable, so passing the key via --private-key (or as a positional argument to cast wallet address) leaves it visible in /proc/<pid>/cmdline and `ps auxww` output for every subprocess. Move signing through a v3 keystore + password file: - scripts/lib/wallet.js: read ETH_PRIVATE_KEY from env (never argv), derive the address, or write an encrypted v3 keystore. Foundry expects lower-case "crypto"; ethers v5 emits "Crypto", so the field name is rewritten before persisting. - scripts/lib/cast-helpers.sh: lazy-create a per-process keystore directory (mode 700, in $TMPDIR), encrypt each unique key on first use, cache by derived address, remove the directory via an EXIT trap that chains onto any existing trap. cast_send_ok now calls cast send with --keystore + --password-file. derive_address_safe replaces the `cast wallet address <KEY>` callers. - scripts/setup-multiple-operators.sh: switch the three cast wallet address callsites to derive_address_safe. Threat model is process argv exposure on multi-tenant runners; the keystore lives only for the lifetime of the sourcing shell and is never persisted across invocations.
The previous commit moved signing through a v3 keystore so the key
stopped appearing in argv (visible to all users via `ps auxww`). The
temporary env assignment used by callers,
ETH_PRIVATE_KEY="$KEY" cast_send_ok ...
is still propagated into every child process spawned inside
cast_send_ok, including the `cast send` invocation, so the key remained
readable from /proc/<cast-pid>/environ for the lifetime of the
subprocess (same-user/root only on Linux, but nonzero).
cast send and cast receipt sign / inspect via --keystore + --password-file
at this point and do not need the variable. Wrap both invocations with
`env -u ETH_PRIVATE_KEY` so the variable is dropped from their
environment. The wallet.js subprocesses still inherit it because they
read ETH_PRIVATE_KEY from env to derive the address and to encrypt the
keystore.
Member
|
Probably a quick follow-up — commit the manifest (from whoever ran |
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.
Sepolia TokenStaking proxy deploy, ExtendedTokenStaking upgrade, and operator setup tooling
Summary
This branch makes Sepolia a first-class network for TokenStaking: deploy the UUPS/transparent proxy the same way as mainnet, add a hardhat-deploy path to upgrade the proxy to
ExtendedTokenStaking(so operators can callstake()), and add scripts for funding, keystores, and end-to-end operator registration against deployedtbtc-v2Sepolia artifacts.It also aligns
IStaking/TokenStakingwithapproveApplicationandincreaseAuthorization, and updatesExtendedTokenStakingin the test harness withoverridewhere needed.Motivation
ExtendedTokenStakingso staking providers can stake native T and authorize Random Beacon / Wallet Registry apps—required for a working testnet operator stack.caststeps and document the flow (fund T + ETH, stake,increaseAuthorization, register operator, join sortition pools).Changes
Contracts
IStaking: declareapproveApplicationandincreaseAuthorization.TokenStaking: implementapproveApplicationandincreaseAuthorization(governance + authorizer flows, application callbacks).TokenStakingTestSet.sol: addoverrideonExtendedTokenStakingmethods that now override the base.Deploy / upgrade
deploy/07_deploy_token_staking.ts: use OpenZeppelin proxy deploy forsepoliaas well asmainnet.deploy/54_upgrade_token_staking_extended.ts(new): Sepolia-only upgrade of the TokenStaking proxy toExtendedTokenStaking, with Tenderly verify when tagged.scripts/upgrade-token-staking.ts: standalone upgrade + refreshdeployments/<network>/TokenStaking.jsonABI/address.package.json: addyarn upgrade:token-staking; pinpackageManager(Yarn 4).Artifacts / repo layout
TokenStaking.jsonadded at repo root (deployment/ABI snapshot).deployments/mainnet/TokenStaking.jsonremoved (superseded / relocated—confirm in review if anything still expects the old path).New scripts (operator / maintainer)
setup-new-staking-provider.js,fund-new-operator.sh,run-new-operator-setup.sh,setup-multiple-operators.shcreate-operator-keystore.js,get-operator-key.jscreate-spv-maintainer-keystore.js