Repository navigation
Enhancement/improvement and code cleanup - #114
Merged
Merged
Conversation
docs: create README for mint plans detailing bulk minting process docs: establish a hardened deployment guide for compiling and deploying contracts chore: initialize package.json for commonjs module type feat: implement sync-contract-artifacts script for managing contract artifacts feat: add DeploymentPreflight checks for deployment readiness feat: create MintRunner for executing and validating mint plans feat: implement SmokeRunner for testing mint, stake, and unstake flows feat: define NFT event DTOs for BaseURI, ContractURI, and DefaultRoyalty updates
…nd update documentation
…set generation instructions - Revised the verification troubleshooting guide to align with the current reusable protected deployment workflow, clarifying the verification process and failure responses. - Updated the manual verification section to reflect changes in how deployment manifests are handled and emphasized the importance of using the exact source revision. - Adjusted the NFT assets README to specify the new method of reading chain-specific values from deployment parameters, ensuring clarity on the asset generation process. - Highlighted the need for proper configuration of the royalty recipient address before mainnet deployment to prevent manual patches.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Solidity/Hardhat toolchain and significantly hardens the Booster NFT + staking deployment/integration surface by introducing explicit admin/minter/owner constructor parameters, safer ownership transfer, additional recovery paths/events, and a protected deployment + verification + smoke-test pipeline with richer manifests and configuration validation.
Changes:
- Hardened contract administration/ownership: NFT switches to delayed default-admin rules with explicit initial admin/minter/delay; staking switches to
Ownable2Step, constrains emergency withdrawals to the original staker, and adds untracked-token rescue + event. - Reworked artifact + deployment workflows: deterministic solc-driven artifact sync, Slither build prep, new protected deploy workflows, and expanded .NET deploy tooling (preflight, sanity checks, resumable manifests, smoke + mint runners).
- Updated tests/docs/config to match the new constructor args, events, and operational runbooks (including Node 22 / Hardhat 3 alignment).
Reviewed changes
Copilot reviewed 94 out of 97 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Move to ES2022 + NodeNext module/resolution for ESM/Hardhat 3. |
| tests/Tricksfor.Blockchain.Booster.UnitTests/ContractDefinitionTests.cs | Update constructor parameter ordering expectations; add staking InitialOwner param test. |
| tests/Tricksfor.Blockchain.Booster.IntegrationTests/DevChainIntegrationTests.cs | Pass new deployment parameters for NFT admin/minter/delay and staking initial owner. |
| tests/Tricksfor.Blockchain.Booster.Deploy.Tests/DeploymentConfigTests.cs | Add validation coverage for privileged addresses and admin delay bounds. |
| test/TricksforBoosterStaking.test.ts | Update Hardhat usage + constructor args; add tests for Ownable2Step, rescue, reentrancy, emergency recipient constraint. |
| test/TricksforBoosterStaking.invariant.test.ts | Add state-machine invariant test across mixed operations and pause behavior. |
| test/TricksforBoosterNFT.test.ts | Update constructor args; add tests for baseURI/defaultAdminDelay and admin update events + zero-address guards. |
| src/Tricksfor.Blockchain.Nethereum/Tricksfor.Blockchain.Nethereum.csproj | Remove legacy Nethereum integration project. |
| src/Tricksfor.Blockchain.Nethereum/Contracts/TricksforBoosterStaking/TricksforBoosterStakingService.cs | Remove legacy typed service. |
| src/Tricksfor.Blockchain.Nethereum/Contracts/TricksforBoosterStaking/ContractDefinition/TricksforBoosterStakingDefinition.cs | Remove legacy DTOs/definitions. |
| src/Tricksfor.Blockchain.Nethereum/Contracts/TricksforBoosterNFT/TricksforBoosterNFTService.cs | Remove legacy typed service. |
| src/Tricksfor.Blockchain.Nethereum/Contracts/TricksforBoosterNFT/ContractDefinition/TricksforBoosterNFTDefinition.cs | Remove legacy DTOs/definitions. |
| src/Tricksfor.Blockchain.Booster/Tricksfor.Blockchain.Booster.csproj | Add deterministic/source-link packaging settings; add dependencies for new tooling. |
| src/Tricksfor.Blockchain.Booster/Services/BoosterStakingService.cs | Add pendingOwner/rescue/2-step ownership transfer helpers + event decoding. |
| src/Tricksfor.Blockchain.Booster/Services/BoosterNFTService.cs | Add default-admin and baseURI query helpers. |
| src/Tricksfor.Blockchain.Booster/README.nuget.md | Document new deployment args and package major version bump. |
| src/Tricksfor.Blockchain.Booster/Contracts/Functions/TricksforBoosterStakingFunctions.cs | Add function messages for pending owner, rescue, acceptOwnership. |
| src/Tricksfor.Blockchain.Booster/Contracts/Functions/TricksforBoosterNFTFunctions.cs | Add function messages for defaultAdmin/defaultAdminDelay/baseURI. |
| src/Tricksfor.Blockchain.Booster/Contracts/Events/TricksforBoosterStakingEvents.cs | Add UntrackedTokenRescued event DTO. |
| src/Tricksfor.Blockchain.Booster/Contracts/Events/TricksforBoosterNFTEvents.cs | Add DTOs for NFT admin/royalty/URI update events. |
| src/Tricksfor.Blockchain.Booster/Contracts/Deployment/TricksforBoosterStakingDeployment.cs | Add BYTECODE constant and InitialOwner constructor arg. |
| src/Tricksfor.Blockchain.Booster/Contracts/Deployment/TricksforBoosterNFTDeployment.cs | Add BYTECODE constant and admin/minter/delay constructor args. |
| src/Tricksfor.Blockchain.Booster.Deploy/SmokeRunner.cs | New smoke runner for mint/stake/decode/unstake flow + result emission. |
| src/Tricksfor.Blockchain.Booster.Deploy/Program.cs | Add CLI subcommands (deploy/smoke/mint) and flags. |
| src/Tricksfor.Blockchain.Booster.Deploy/PostDeploySanityChecker.cs | Expand sanity checks (bytecode present, interfaces, URIs, roles, ownership). |
| src/Tricksfor.Blockchain.Booster.Deploy/MintRunner.cs | Add resumable reviewed mint-plan runner with JSON validation. |
| src/Tricksfor.Blockchain.Booster.Deploy/ManifestWriter.cs | Add manifest read helper for reuse/resume. |
| src/Tricksfor.Blockchain.Booster.Deploy/DeploymentRunner.cs | Add stricter config validation, manifest reuse, code-hash recording, preflight/sanity integration. |
| src/Tricksfor.Blockchain.Booster.Deploy/DeploymentPreflight.cs | Add RPC/account checks and optional metadata URL reachability validation. |
| src/Tricksfor.Blockchain.Booster.Deploy/DeploymentConfig.cs | Add privileged address fields, admin delay, smoke/mint settings, and flags. |
| src/Tricksfor.Blockchain.Booster.Deploy/ContractDeploymentRecord.cs | Enrich manifest schema with chain/tx/compiler/code-hash/admin/minter/owner metadata. |
| src/Tricksfor.Blockchain.Booster.Deploy/appsettings.localhost.json | Provide localhost defaults for new required fields. |
| src/Tricksfor.Blockchain.Booster.Deploy/appsettings.json | Expand default config schema; update localhost naming. |
| SmartContract.sln | Add a root solution for CI/operators; includes retained projects. |
| scripts/validate-nft-assets.js | Add manifest-only/output-only modes, placeholder allowances, collection image checks, env/name consistency. |
| scripts/sync-contract-artifacts.js | New deterministic solc compilation + HH3 artifact/build-info + C# ABI/bytecode sync. |
| scripts/prepare-slither-build.js | New HH3 build-info adapter for Slither/crytic-compile expectations. |
| scripts/package.json | Mark scripts directory as CommonJS within an ESM repo. |
| scripts/deploy.ts | Remove legacy Hardhat deploy script. |
| README.md | Update architecture/tooling/deployment docs; note mainnet blockers and new workflows. |
| package.json | Switch to ESM + Node 22 engine; Hardhat 3; new compile/test/security scripts. |
| nft-assets/README.md | Update guidance for protected workflow and fee-recipient/mainnet blockers. |
| hardhat.config.ts | Migrate to Hardhat 3 config style, http networks via config variables, verify config. |
| docs/verification-troubleshooting.md | Rewrite for the protected deployment workflow and new verification behavior. |
| docs/security-review-2026-07-19.md | Add reproducible automated security review summary and reproduction steps. |
| docs/README.md | Update docs index to current runbooks and security review. |
| docs/nuget-packaging.md | Update versioning/publishing flow for new packaging workflow. |
| docs/nft-metadata-generation.md | Update to protected workflow integration and mainnet blocker notes. |
| docs/nft-assets-contributor-guide.md | Update contributor guidance for protected deployment and routing constraints. |
| docs/nft-asset-validation.md | Update validation workflow integration (CI + protected deploy). |
| docs/github-environments-setup.md | Update environment setup for new workflows + required vars/secrets. |
| docs/generate-assets.md | Update generator integration and fee-recipient blocker guidance. |
| deployments/README.md | Update manifest expectations for new metadata and reuse behavior. |
| deployments/mint-plans/README.md | Add mint-plan review/validation instructions. |
| deployments/config/polygon-mainnet/nft-manifest.json | Update collection naming. |
| deployments/config/polygon-mainnet/deployment-params.json | Add new config fields; update collection name and admin delay. |
| deployments/config/polygon-amoy/nft-manifest.json | Update collection naming. |
| deployments/config/polygon-amoy/deployment-params.json | Add new config fields; update collection name. |
| deployments/config/optimism-sepolia/nft-manifest.json | Update collection naming. |
| deployments/config/optimism-sepolia/deployment-params.json | Add new config fields; update collection name. |
| deployments/config/optimism-mainnet/nft-manifest.json | Update collection naming. |
| deployments/config/optimism-mainnet/deployment-params.json | Add new config fields; update collection name and admin delay. |
| deployments/config/ethereum-sepolia/nft-manifest.json | Update collection naming. |
| deployments/config/ethereum-sepolia/deployment-params.json | Add new config fields; update collection name. |
| deployments/config/ethereum-mainnet/nft-manifest.json | Update collection naming. |
| deployments/config/ethereum-mainnet/deployment-params.json | Add new config fields; update collection name and admin delay. |
| deployments/config/bsc-testnet/nft-manifest.json | Update collection naming. |
| deployments/config/bsc-testnet/deployment-params.json | Add new config fields; update collection name. |
| deployments/config/bsc-mainnet/nft-manifest.json | Update collection naming. |
| deployments/config/bsc-mainnet/deployment-params.json | Add new config fields; update collection name and admin delay. |
| deployments/config/avalanche-mainnet/nft-manifest.json | Update collection naming. |
| deployments/config/avalanche-mainnet/deployment-params.json | Add new config fields; update collection name and admin delay. |
| deployments/config/avalanche-fuji/nft-manifest.json | Update collection naming. |
| deployments/config/avalanche-fuji/deployment-params.json | Add new config fields; update collection name. |
| contracts/TricksforBoosterStaking.sol | Switch to Ownable2Step; constructor owner param; constrain emergency withdrawals; add rescue + event. |
| contracts/TricksforBoosterNFT.sol | Switch to AccessControlDefaultAdminRules; add constructor args; add admin/royalty events; add baseURI view. |
| contracts/test/ReentrantERC721.sol | Add malicious ERC721 double for reentrancy regression testing. |
| artifacts/contracts/test/ReentrantERC721.sol/ReentrantERC721.json | Add compiled artifact for malicious test contract. |
| .gitignore | Ignore additional generated artifacts, slither outputs, and test artifacts. |
| .github/workflows/release-package.yml | Add standalone NuGet packaging/release workflow for stable tags. |
| .github/workflows/mint-mainnet.yml | Add protected mint-plan execution workflow. |
| .github/workflows/deploy-testnet.yml | Add manual protected testnet deployment dispatcher. |
| .github/workflows/deploy-mainnet.yml | Add manual protected mainnet deployment dispatcher with stable-tag gate. |
| .github/workflows/ci.yml | Add CI coverage for contracts, security analysis, .NET, metadata, and workflow lint. |
| .github/workflows/_deploy.yml | Add reusable protected deployment workflow (metadata upload, deploy/resume, verify, smoke, artifacts). |
Comment on lines
4
to
9
| import "@openzeppelin/contracts/token/ERC721/IERC721.sol"; | ||
| import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; | ||
| import "@openzeppelin/contracts/access/Ownable.sol"; | ||
| import "@openzeppelin/contracts/access/Ownable2Step.sol"; | ||
| import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; | ||
| import "@openzeppelin/contracts/utils/Pausable.sol"; |
| import { expect } from "chai"; | ||
| import hre from "hardhat"; | ||
| import { network } from "hardhat"; | ||
| import { TricksforBoosterNFT } from "../typechain-types"; |
| import { expect } from "chai"; | ||
| import hre from "hardhat"; | ||
| import { network } from "hardhat"; | ||
| import { TricksforBoosterNFT, TricksforBoosterStaking } from "../typechain-types"; |
Comment on lines
+47
to
+49
| string? owner = null; | ||
| try { owner = await nft.OwnerOfQueryAsync(tokenId); } catch { /* not minted */ } | ||
| if (owner is null) |
Comment on lines
+69
to
+71
| string? currentOwner = null; | ||
| try { currentOwner = await nft.OwnerOfQueryAsync(item.TokenId); } catch { /* not minted */ } | ||
|
|
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.