Pre-auditThree days of senior audit time for $1,500, credited in full toward your Zealynx audit.3 audit days, credited toward your auditSee how →
L-11·audit-finding

Nothing enforces the 72-hour activation delay at a production deploy: an env default, a 60s floor, and a self-skipping assertion

Fixednfterc-1155marketplace
TL;DR

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.

Severity
LOW
Method
MManual review
CAT.

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.

02Section · What we checked

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:

  1. 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 to 60 is accepted on any network, including a production NETWORK_KEY.
  2. The contract floor is 60 seconds, so the code refuses nothing in that range (see #3 — the 1-hour commit 7d74f21 is still unmerged).
  3. The test that asserts the default silences itself. test_settlementActivationDelayDefaultsToSeventyTwoHours is guarded by vm.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.

03Section · Why it matters

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.

04Section · Recommendation

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).

05Section · Remediation

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.

06Section · Verification provenance

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.

Status
Fixed
L-11