Nothing enforces the 72-hour activation delay at a production deploy: an env default, a 60s floor, and a self-skipping assertion
Nothing enforces the 72-hour activation delay at a production deploy: an env default, a 60s floor, and a self-skipping assertion. Final disposition: Fixed.
Tracker: issue #40 · Re-review ID: RR-05 · Status: Fixed · Verified at: generation-d
Severity: Low
Location: script/DeployBase.s.sol _activationDelayParam; test/DeployDefaults.t.sol:90; src/PlakxioBadge.sol MIN_SETTLEMENT_ACTIVATION_DELAY
Verified at: audit-v3-rereview-handoff, plus the live Base Sepolia deployment
Raised after verifying the deployed bytecode. Your §9 discloses the testnet divergence fully and this is not a challenge to that disclosure — the question is narrower: what would stop a mainnet deploy from taking a short delay.
What we checked
The live deployment reads settlementActivationDelay() = 120, against MIN_SETTLEMENT_ACTIVATION_DELAY() = 60. Both expected per §9. Tracing what defends the 72-hour figure:
- The deploy script defaults to it but does not require it.
vm.envOr("SETTLEMENT_ACTIVATION_DELAY", uint256(72 hours))— omitting the variable yields 72 hours, which is the right direction for the default. Setting it to60is accepted on any network, including a productionNETWORK_KEY. - The contract floor is 60 seconds, so the code refuses nothing in that range (see #3 — the 1-hour commit
7d74f21is still unmerged). - The test that asserts the default silences itself.
test_settlementActivationDelayDefaultsToSeventyTwoHoursis guarded byvm.skip(bytes(vm.envOr(name, string(""))).length != 0), so it does not run whenever the variable is exported. §9 says "We check that they actually ran", and it is running — our own run reports 0 skipped across 753 tests. But that is a procedural check: nothing fails if the assertion stops running, and a CI runner or developer shell that exports the variable turns the guarantee off silently.
So the 72-hour protection is a convention held by a default, a comment, and a test that can opt out of itself. Nothing in the contracts or the deploy path enforces it, and it has never been exercised at that value on any network.
Why it matters
Normally this would be a documentation-grade observation. It is Low rather than Informational because of what #37 did to the surrounding controls: with Special exempted from the holder's refusal, this delay is the only contract-level protection for the highest-value class, and your §10(j) names it as the compensating control. A compensating control whose value is set by an environment variable, floored at 60 seconds, and asserted by a self-skipping test is weaker than the sentence in §10(j) implies.
Recommendation
You already have everything needed, and it is a better fix than raising the constant alone:
Gate the floor by network. script/network.sh already maps NETWORK_KEY to one chain id and already refuses a mainnet key on a testnet RPC. Extend that: a production NETWORK_KEY requires SETTLEMENT_ACTIVATION_DELAY >= 72 hours (and the admin transfer delay >= 7 days); testnet keys may go short, as they do now. Then the divergence in §9 becomes enforced rather than conventional, and the shell harnesses that set 60 seconds keep working unchanged.
Secondary: make the skip visible rather than silent — assert on the default path in a harness that controls its own environment (vm.setEnv to clear, or a parameterised read) so the assertion cannot be turned off by the ambient shell. Note vm.setEnv is process-global and races under parallel tests, so prefer the parameterised read.
Related: #3 (the 60-second floor, and the unmerged 1-hour commit), #37 (why this delay is now load-bearing).
Remediation
Fixed, in both halves.
The contract half is the construction floor described under M-12, so no deployment on any network can build a badge below it whatever its environment says.
The script half. The production gate is keyed to a production network key only, accepts plain decimal seconds exclusively, strips leading zeros — the padded-value bypass the client's own adversarial round found — treats values above eighteen significant digits as above any minimum, judges the process environment before the .env file, and runs before any RPC read. The minima are 259,200 seconds of activation delay and 604,800 seconds of administrator transfer delay. Before promotion, the wrappers additionally read both delays back from the chain and refuse to promote on a failed probe rather than reasoning from one — the correct direction for a control of this kind. One shared reader now serves each delay across the deployment and upgrade scripts, each with a parameterised parse, so the default is asserted without mutating process environment.
The self-silencing assertion this finding was named for is gone: the defaults test no longer skips itself, and the suite reported zero skipped tests. While fixing it the client found and closed a further latent defect — both upgrade scripts still defaulted the administrator transfer delay to two days after the base deployment moved to seven — which the shared reader closes.
Two things to state plainly. The 72-hour production value has still never been exercised at that value on any network; under our agreement of 13 October the rehearsal is post-review evidence and not a new precondition. And a narrower instance of the same self-skipping pattern survives in AdminRequired.t.sol, where the assertion that the production variable names are the ones read silences itself if a developer exports one of them locally. The substantive assertions for that finding do not skip, so this does not disturb the verified fix here; the same parameterised reader already built is the remedy.
Verification provenance
Final disposition: Fixed. Verified at generation-d, commit bcf18bb2926589bb8538fa8c322eabc871ff2801. This finding was assessed in the generation-d mitigation review. The report-wide scope and evidence limitations remain applicable; see the final report overview.