diff --git a/contracts/script/deployment/14_RedeployAPNTs.s.sol b/contracts/script/deployment/14_RedeployAPNTs.s.sol index 28760681..855672ca 100644 --- a/contracts/script/deployment/14_RedeployAPNTs.s.sol +++ b/contracts/script/deployment/14_RedeployAPNTs.s.sol @@ -126,6 +126,35 @@ contract RedeployAPNTs is Script { console.log("factory :", address(factory), factory.version()); console.log("factory owner:", factory.owner()); console.log("implementation:", factory.implementation()); + // Record what was DECLARED, so verification has a source that is not the + // chain. 15_VerifyAPNTs previously took the expected supply from whoever + // ran it, and the natural way to answer "what should it be?" is to read + // the chain — making the assertion an identity. An artifact written here, + // before the chain is consulted, is the only reading that can disagree + // with it. Issue #407; repo:dvt landed rotation-readback.json for the + // same reason. + string memory rec = "apnts"; + vm.serializeAddress(rec, "aPNTs", token); + vm.serializeAddress(rec, "factory", address(factory)); + vm.serializeAddress(rec, "mintTo", mintTo); + vm.serializeUint(rec, "chainId", block.chainid); + string memory recJson = vm.serializeUint(rec, "mintAmount", mintAmount); + // The path carries the chain id. One record per chain: deploying a second + // chain must not overwrite the first chain's declaration and leave + // 15_VerifyAPNTs checking a supply against the wrong number. + string memory recPath = + string.concat(vm.projectRoot(), "/deployments/apnts-deploy-record.", vm.toString(block.chainid), ".json"); + // And only a real broadcast may write it. A dry run deploys to SIMULATED + // addresses; writing those over a committed record would replace a true + // declaration with a fictional one — and the declaration is the entire + // point of the artifact. Both raised by pr-daemon on #417. + if (vm.isContext(VmSafe.ForgeContext.ScriptBroadcast)) { + vm.writeJson(recJson, recPath); + console.log("declared mint recorded to", recPath); + } else { + console.log("DRY RUN: deploy record NOT written; a broadcast would write", recPath); + } + console.log("aPNTs (new) :", token, xPNTsToken(token).version()); console.log(" minted :", mintAmount, "to", mintTo); console.log("communityOwner:", xPNTsToken(token).communityOwner()); diff --git a/contracts/script/deployment/15_VerifyAPNTs.s.sol b/contracts/script/deployment/15_VerifyAPNTs.s.sol index 80708db6..5d009d4d 100644 --- a/contracts/script/deployment/15_VerifyAPNTs.s.sol +++ b/contracts/script/deployment/15_VerifyAPNTs.s.sol @@ -99,16 +99,38 @@ contract VerifyAPNTs is Script { // just against its owner. Every value below is what `initialize` leaves, or // what our factory arguments (superPaymaster = 0, paymasterAOA = 0) imply. // Found by Codex at stop-time review. - if (!vm.envOr("ALLOW_POST_DEPLOY_ACTIVITY", false)) { - // The deploy mints a starting float while the EOA still owns the token, so - // "supply is zero" is no longer the fresh-clone invariant. What still holds - // is that supply equals EXACTLY what the deploy was told to mint: anything - // else means a second mint happened in the ownership gap. Pass the same - // MINT_AMOUNT the deploy used; the default of 0 keeps the old meaning. + bool checkedDefaults = !vm.envOr("ALLOW_POST_DEPLOY_ACTIVITY", false); + if (checkedDefaults) { + // The expectation comes from the DEPLOY RECORD, not from whoever runs + // this. It used to read MINT_AMOUNT from the environment, and the + // natural way to answer "what should it be?" is to read the chain — + // at which point the assertion compares the chain to itself. Same + // family as three other defects fixed this week: an expected value + // taken from a source that cannot disagree with the thing under test. + // Issue #407. + // + // A missing record FAILS. "No record" and "record says zero" are not + // the same reading, and treating them alike is the absence-as-consent + // this repo has now been bitten by four times. + string memory recPath = + string.concat(vm.projectRoot(), "/deployments/apnts-deploy-record.", vm.toString(block.chainid), ".json"); + // vm.exists is non-view, and run() is deliberately `view` — that is what + // makes this script unable to deploy, broadcast or repair anything. + // Keeping the guarantee is worth more than the convenience, so a missing + // record is caught by vm.readFile itself: measured, it REVERTS on absence + // ("failed to open file ... No such file or directory") rather than + // returning empty. So the require below covers only the present-but-empty + // case; both are fail-closed, which is what matters. The earlier comment + // here described a mechanism that does not exist. pr-daemon, #417. + string memory rec = vm.readFile(recPath); + require(bytes(rec).length > 0, "deploy record is empty: cannot verify supply against a declared amount"); require( - supply == vm.envOr("MINT_AMOUNT", uint256(0)), - "supply does not equal the declared mint: something else minted" + stdJson.readAddress(rec, ".aPNTs") == token, + "the deploy record is for a different token than the one being verified" ); + require(stdJson.readUint(rec, ".chainId") == block.chainid, "the deploy record is from a different chain"); + uint256 declared = stdJson.readUint(rec, ".mintAmount"); + require(supply == declared, "supply does not equal the DECLARED mint: something else minted"); require(xPNTsToken(token).SUPERPAYMASTER_ADDRESS() == address(0), "a SuperPaymaster was set"); require(xPNTsToken(token).issuanceCap() == 0, "an issuance cap was set"); require(!xPNTsToken(token).emergencyDisabled(), "the token is in emergency state"); @@ -187,8 +209,18 @@ contract VerifyAPNTs is Script { console.log(" APNTS_PRICE_MAX: (not exposed by this factory version)"); } console.log(""); - console.log("RESULT: OK - both owners are the Safe; every enumerable value is at"); - console.log(" its post-deploy default"); + // The claim has to shrink when the checks do. ALLOW_POST_DEPLOY_ACTIVITY + // skips the whole fresh-clone block above — including the deploy-record + // supply comparison — and this line went on asserting the defaults anyway. + // A green line that outlived its checks. pr-daemon/Codex, #417. + if (checkedDefaults) { + console.log("RESULT: OK - both owners are the Safe; every enumerable value is at"); + console.log(" its post-deploy default"); + } else { + console.log("RESULT: OK (REDUCED) - both owners are the Safe. ALLOW_POST_DEPLOY_ACTIVITY"); + console.log(" was set: the fresh-clone defaults and the supply-vs-deploy-record"); + console.log(" comparison were NOT checked."); + } console.log(""); console.log(" WHAT THIS DOES NOT PROVE. autoApprovedSpenders, approvedFacilitators"); console.log(" and spenderDailyCapOverride are mappings: a view call can ask about an"); diff --git a/deployments/apnts-deploy-record.11155111.json b/deployments/apnts-deploy-record.11155111.json new file mode 100644 index 00000000..c5f1cb55 --- /dev/null +++ b/deployments/apnts-deploy-record.11155111.json @@ -0,0 +1,7 @@ +{ + "aPNTs": "0x948C9d1Bd99B39DEE482C23d6A3BD26210B56040", + "chainId": 11155111, + "factory": "0xc83EDcb81964a259Eb9392BC8b4B5B2929a89774", + "mintAmount": 2000000000000000000000000, + "mintTo": "0xb5600060e6de5E11D3636731964218E53caadf0E" +} diff --git a/deployments/deploy-record-apnts-3.5.0-sepolia.md b/deployments/deploy-record-apnts-3.5.0-sepolia.md index d6634516..dc1bcf00 100644 --- a/deployments/deploy-record-apnts-3.5.0-sepolia.md +++ b/deployments/deploy-record-apnts-3.5.0-sepolia.md @@ -45,15 +45,21 @@ RESULT: OK exit 0 Reproduce: ``` -EXPECT_CHAIN_ID=11155111 MINT_AMOUNT=2000000000000000000000000 \ +EXPECT_CHAIN_ID=11155111 \ APNTS=0x948C9d1Bd99B39DEE482C23d6A3BD26210B56040 \ FACTORY=0xc83EDcb81964a259Eb9392BC8b4B5B2929a89774 \ forge script contracts/script/deployment/15_VerifyAPNTs.s.sol:VerifyAPNTs \ --rpc-url "$SEPOLIA_RPC_URL" ``` -`MINT_AMOUNT` must match what the deploy minted: the check is that supply equals -**exactly** the declared amount, so a second mint in the ownership gap fails it. +The expected supply is no longer passed in. It is read from +`deployments/apnts-deploy-record..json`, which `14_RedeployAPNTs` writes at +deploy time from the amount it was told to mint — a declaration made before the chain +is consulted, and the only source that can disagree with the chain. A missing record +fails the run; it is not treated as "nothing to check". The check is still that supply +equals **exactly** the declared amount, so a second mint in the ownership gap fails it. +(`EXPECT_CHAIN_ID` is still read, at `15_VerifyAPNTs.s.sol:47`; it defaults to OP +mainnet.) ## What this fixes, and what it does not @@ -101,7 +107,12 @@ stop-time review. 1. **Safe** (it owns the token now): `setSuperPaymasterAddress(0x09DF0d2e…)` and `addAutoApprovedSpender(0x09DF0d2e…)` on `0x948C9d1B…`. 2. **SP owner**: `setAPNTsToken(0x948C9d1B…)`, wait 7 days, then apply. -3. Only then flip `config.sepolia.json`, and re-run `15_VerifyAPNTs`. +3. Only then flip `config.sepolia.json`, and re-run `15_VerifyAPNTs` — with + `ALLOW_POST_DEPLOY_ACTIVITY=true`, because steps 1 and 2 deliberately leave the + fresh-clone state the default run asserts (a SuperPaymaster is now set, and a + spender is auto-approved). That run prints `RESULT: OK (REDUCED)` and names what + it did not check: the defaults and the supply-vs-record comparison. It is an + owner check at that point, not a deploy check. Steps 1 and 2 can run in parallel; step 3 must be last. Both halves of step 1 are Safe transactions, which is the intended steady state — but it is why the wiring