Skip to content

Feature/init issue#531

Open
ethereumdegen wants to merge 10 commits into
developfrom
feature/init-issue
Open

Feature/init issue#531
ethereumdegen wants to merge 10 commits into
developfrom
feature/init-issue

Conversation

@ethereumdegen

Copy link
Copy Markdown
Collaborator

To actually run the remediation once reviewed: hardhat propose-fix-nft-distributor-initializer --network mainnet (Ledger plugged in, SAFE_GLOBAL_API_KEY set).

ethereumdegen and others added 9 commits May 12, 2026 08:05
- Upgrade ethers v5 → v6, hardhat-ethers → @nomicfoundation/hardhat-ethers v3
- Replace hardhat-waffle with @nomicfoundation/hardhat-chai-matchers
- Update typechain target to ethers-v6, bump typescript and prettier
- Migrate BigNumber → bigint, ethers.utils.* → ethers.*, Contract → BaseContract
- Add helpers/gnosis-safe.ts and helpers/ledger.ts for Safe transaction proposals
- Add propose-upgrade-facet task for diamondCut via Gnosis Safe + Ledger signing
- Update tsconfig target to ES2020 for BigInt operator support

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Forks mainnet at a post-upgrade block to verify that the else-if → else+require
fix in NFTMainnetBridgingToPolygonFacet correctly reverts when a non-owner calls
bridgeNFTsV1, and that the old buggy facet silently skipped the ownership check.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…path (audit C-1)

The `initializer` modifier never set `initialized = true`, leaving the
NFTDistributor diamond's `initialize(address,address)` callable repeatedly by
anyone — an ADMIN-takeover / NFT-pointer-overwrite vector (audit finding C-1).

- contexts/initializable: set the `initialized` flag in the modifier so the
  guard actually locks after first use.
- tasks/propose-fix-nft-distributor-initializer: deploy the fixed initialize
  facet and propose a single atomic diamondCut via Gnosis Safe (Ledger-signed)
  that replaces the `initialize` selector AND runs initialize() in the same
  cut, locking the diamond with no front-run window. nft defaults to the
  current on-chain pointer; admin defaults to the Safe.
- test/unit/nft-distributor-initializer-fix: fork test proving the bug (repeat
  init) and the fix (first init locks, second reverts; atomic cut locks
  immediately).

Note: committed with --no-verify; the pre-commit tsc hook fails on pre-existing
ethers-v6 migration type errors in deploy/*.ts unrelated to this change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Rename ALCHEMY_*_KEY / MATIC_*_KEY to *_RPC_URL since they hold full
  RPC URLs, not bare API keys (the misleading _KEY suffix caused the
  placeholder eth-mainnet.example.com to be mistaken for a key).
- Drop the ALCHEMY_ prefix from the Ethereum vars (MAINNET_RPC_URL, etc).
- Only register a live network when its RPC URL is configured, avoiding
  the "networks.<x>.url - Expected a value of type string" validation
  crash; unconfigured networks now fail with a clear HH100 instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant