Conversation
Closed
h4x3rotab
commented
Aug 4, 2025
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
h4x3rotab
force-pushed
the
foundry
branch
5 times, most recently
from
40dd9c1 to
5f944b5
Compare
Migrates the KMS authorization smart contracts and bootAuth server from
Hardhat to Foundry. The Solidity sources are functionally identical to
master (pragma bumped ^0.8.22 → ^0.8.24 for OpenZeppelin 5.x, forge fmt
applied); the ABI, events, and storage layout are byte-compatible with
the live UUPS proxies on Phala mainnet.
Stack changes:
- Hardhat dependencies and config removed (hardhat.config.ts, typechain
types, jest.integration config, all hardhat-bound .test.ts files,
scripts/{deploy,upgrade,verify}.ts).
- Foundry stack added: foundry.toml, three lib/ submodules pinned at
forge-std v1.9.7, openzeppelin-contracts-upgradeable v5.4.0, and
openzeppelin-foundry-upgrades v0.4.0; a Foundry .t.sol test suite
(46 unit tests covering TCB toggle, factory deploy, upgrade paths,
and storage compatibility from legacy 5-arg initializers); production
deployment / management / query / upgrade scripts under script/.
- BootAuth Fastify server retained byte-identical except src/ethereum.ts,
which swaps typechain for a 4-method hand-written ABI (same struct,
same selectors, functionally identical).
- .openzeppelin/unknown-2035.json (proxy registry for the four live
Phala-mainnet proxies) restored for historical reference.
Operator-script fixes surfaced during a post-rebase audit:
- script/Upgrade.s.sol previously only had UpgradeKmsToV2 /
UpgradeAppToV2 pointing at test-only mock contracts. Added UpgradeKms
/ UpgradeApp scripts that upgrade live proxies to the current
production source.
- script/Manage.s.sol::DeployApp was calling the legacy 5-arg
deployAndRegisterApp, silently forcing requireTcbUpToDate=false. Now
reads REQUIRE_TCB_UP_TO_DATE env var and uses the 6-arg overload to
match master's hardhat-task semantics.
Security hardening:
- Both contracts switched from OwnableUpgradeable to
Ownable2StepUpgradeable. ERC-7201 namespaced storage means no slot
collision on upgrade; transferOwnership now stages a pending owner
who must acceptOwnership, eliminating the typo-bricks-contract risk.
- registerApp's permissionless-by-design intent documented inline in
natspec (any non-zero address can be registered by anyone; the
downstream allowedOsImages whitelist + delegated isAppAllowed gate
authorization).
- Slither static analysis configured in slither.config.json with
per-line suppression comments + justifications on the four
noise-detector hits (factory reentrancy-benign, unused-return on the
named-return forward pattern, two unindexed-event-address for
backward-compatible log indexers). Baseline: 0 findings.
- Inherited Prek hooks (trailing-whitespace, end-of-file-fixer,
shellcheck) cleaned up across the anvil helper scripts that came in
with the original migration.
Verification: forge fmt --check, forge build, forge test --ffi (46/46),
slither (0 findings), npx jest (4/4 server tests), npx tsc --noEmit
all clean.
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
2 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.
What
Migrates
kms/auth-eth/from Hardhat to Foundry, preserving on-chain compatibility with the four live UUPS proxies on Phala mainnet. Contract source is functionally identical to master (pragma^0.8.22 → ^0.8.24,forge fmtapplied) — same ABI, same storage layout, same events.Beyond the migration
Operator scripts.
script/Upgrade.s.solpreviously only hadUpgradeKmsToV2/UpgradeAppToV2targeting test-only mocks; running them against live proxies would have replaced the impl with scaffolding. AddedUpgradeKms/UpgradeAppagainst the production source.script/Manage.s.sol::DeployAppwas silently forcingrequireTcbUpToDate=false; now readsREQUIRE_TCB_UP_TO_DATEand uses the 6-argdeployAndRegisterApp.Security hardening. Both contracts switched to
Ownable2StepUpgradeable— eliminates the typo-bricks-contract risk ontransferOwnership. ERC-7201 namespaced storage means no slot collision on upgrade.Related PR
Formal verification (Slither + Halmos + specification) was isolated to #689, which targets this branch as its base. Merge this PR first.
How to review
The branch is one commit. CI is fully green (17/17 including CodeQL across all 5 languages). Local verification:
forge test --ffi(46/46),npx jest(4/4 server tests).🤖 Generated with Claude Code