fix: harden delegated signatures and add Foundry tooling#12
Merged
Conversation
f3c0f57 to
180f9d7
Compare
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.
Summary
v1.1.4: all delegated flows now use an EIP-712 authorization envelope while preserving the existing state-changing function ABIs.uint64signature deadlines, and domain binding tochainId+verifyingContract; old raw delegated signatures are intentionally rejected.foundry-deployerhelper.bun run patch:spas a chain-aware wrapper for patching existing deployments without manually setting Alchemy RPC URLs or proxy addresses per chain.PROD_OWNERdeployments, upgrades assert the ERC-1967 implementation slot and resolvedversion(), no-op upgrades without calldata are rejected, and unreadable/emptyversion()checks are hard failures.Delegated Signature Migration
delegateSignature.nonce = delegationNonces(delegateAttester);getDelegated*Hash(...)helper;getDelegatedAuthorizationDigest(delegateAttester, actionHash, nonce, deadline);delegateSignature = abi.encode(nonce, deadline, signature).indexingKey, andextraData, but that is a delegated signing semantics change for SDKs and relayers.Deployment / Upgrade Plan
bun run patch:sp -- <chain>to simulate, thenbun run patch:sp -- <chain> --broadcastto patch.ALCHEMY_API_KEY, uses the official address-book proxy for the selected chain, verifies RPC chain ID and proxy code, checks owner/version, and invokes the existing Foundry scripts.PLUME_TESTNET_RPC_URL, or--rpc-url.--broadcast, it deploys/resolves the implementation and prints theupgradeToAndCall(implementation, 0x)calldata for the owner contract or Safe.script/DeploySPImplementation.s.solusing the sameDEPLOYERandSP_IMPL_SALT_IDacross chains so the implementation address is consistent when bytecode is identical.script/DeploySPProxy.s.sol; it initializesSP.initialize(INITIAL_SCHEMA_COUNTER, INITIAL_ATTESTATION_COUNTER).PROD_OWNERis set, fresh proxy deployment transfers ownership toPROD_OWNEReven when the chain is not listed inMAINNET_CHAIN_IDS. If the chain is listed inMAINNET_CHAIN_IDS,PROD_OWNERis required.script/SPProxyRegistry.sol. Deprecated, struck-through address-book entries are excluded from batch upgrades; useSP_PROXYfor any non-address-book or deprecated proxy.version(). By default, the expected version is read from the resolved implementation; setSP_EXPECTED_VERSIONonly when you want an explicit override. Missing or empty implementation/proxyversion()now reverts.Review Follow-Up
fs_permissionsto./deployments/, deriving the upgrade script's expected version from the implementation, fixing theMockResolversolhint nit, documenting the supported CreateX salt mode, and verifyingDelegationExpired()is selector0x30d3ba07.vm.setEnvfrom deploy-script tests, using a harness-local expected-version override, and making unreadable/emptyversion()a hard upgrade-script failure instead of allowing"unknown" == "unknown".indexingKey, andextraDatainto the signature would further reduce relayer discretion but requires an SDK/signing migration beyond replay protection.forge inspect SP storage-layoutgate is not equivalent to the removed Hardhat upgrades validation for this ERC-7201 storage pattern.Package Tooling
package.jsonnow declarespackageManager: bun@1.3.13and version1.1.4.bun.locknow contains only the OpenZeppelin packages required by Foundry remappings..npmrc, Prettier config/ignore,hardhat.config.ts,tsconfig.json, and the legacyscripts/*.tsHardhat deploy/upgrade scripts.bun run fmt,bun run fmt:check, and CIforge fmt --check.Verification
bash -n script/patch-sp-deployment.shbun run patch:sp -- --helpbun run patch:sp -- --listenv -u ALCHEMY_API_KEY DEPLOYER=0x0000000000000000000000000000000000001234 bun run patch:sp -- basefailed with the expected missingALCHEMY_API_KEYerror.forge test --match-test delegatedforge test --match-path test/SPDeployScripts.test.solzsh -lc 'for i in {1..20}; do forge test --match-path test/SPDeployScripts.test.sol >/tmp/spdeployscripts-race-check.log || { cat /tmp/spdeployscripts-race-check.log; exit 1; }; done; echo "20 focused runs passed"'bun run fmtbun run fmt:checkbun run buildbun run testgit diff --checkcast sig "DelegationExpired()"bun install --lockfile-onlybun install --frozen-lockfilebun run cleanDEPLOYER=0x0000000000000000000000000000000000001234 forge script script/UpgradeSPProxies.s.sol --tc UpgradeSPProxiesreverted with the expectedProxySelectionRequired(31337)guard.DEPLOYER=0x0000000000000000000000000000000000001234 forge script script/DeploySPImplementation.s.sol --tc DeploySPImplementationDEPLOYER=0x0000000000000000000000000000000000001234 forge script script/DeploySPProxy.s.sol --tc DeploySPProxy