fix(audit-2): remediate slashing/jobs/rewards/extensions findings (1 high + 4 med + low)

[drewstone]

Summary

Second audit batch (11 findings across slashing, jobs, services-lifecycle, rewards/inflation, extensions, beacon-L2). Each finding was independently validated against the cited code before any fix — two report-vs-analysis discrepancies were resolved by ground-truthing the exact files named (F3, F5). 9 issues fixed in code; F5 deferred with documentation; F6/F11 are latent/intentional and documented. Full affected suites (slashing, lifecycle, jobs, rewards, extensions, beacon, integration, upgrade) are green.

Validity verdicts

ID	Sev	Verdict	Action
F1	High	VALID	Snapshot per-asset commitments at propose; execute uses snapshot
F2	Med	VALID	Non-zero default disputeBond (0.02 ETH, matches prod config)
F3	Med	VALID	_ensureAggregationBypass now fails closed on hook failure
F4	Med	VALID	Failsafe non-zero default stakeLockDuration (blocks JIT capture)
F5	Med	VALID	Deferred — needs interface + staking-layer + accumulator change; documented
F6	Med	latent	Dead clamp; liability-undercount only on a pre-liability upgrade (none exists). Documented
F7	Low	VALID	Permissionless distributeEpoch no longer reallocates the staker slice
F8	Low	VALID	Prune assetOperators[] on zero stake (swap-and-pop)
F9	Low	VALID	Duplicate-asset commitment dedup made unconditional
F10	Low	VALID	Bound incoming slashBps <= 10000 at L2 receipt (defense-in-depth)
F11	Info	intentional	instantSlashEnabled read+enforced but deliberately unreachable. Documented

Fixes

• F1 (High) — executeSlash read commitments LIVE, so an operator could self-dispute → leave → rejoin with a 1-bps commitment and evade ~99.99% of the slash. Now snapshotted into a dedicated _slashCommitmentSnapshots mapping at propose time and consumed at execute. Storage appended at end, gap shrunk 29→28; SlashProposal (and its ITangleSlashing binding) untouched, so no Rust binding regen needed.
• F2 (Med) — default disputeBond was 0 → free self-dispute freezes delegator withdrawals for the resolution window. Ship a non-zero default matching deploy/config/*.json.
• F3 (Med) — _ensureAggregationBypass only reverted on ok && requiresAggregation, failing open when the manager hook reverts/gas-griefs → a single operator could finalize a quorum job. Now !ok || requiresAggregation reverts. Safe because the BSM base implements requiresAggregation (returns false), so !ok is a genuine failure.
• F4 (Med) — stakeLockDuration=0 default enabled same-block JIT instant-mode reward capture. Seed a non-zero failsafe default in the constructor.
• F7 (Low) — the unspent staker slice was reallocated to other categories on the permissionless empty-serviceIds path, letting a front-running operator/customer/dev redirect staker inflation. Now reallocated only when staker distribution was attempted; otherwise retained in the pool to roll forward.
• F8 (Low) — assetOperators[] was append-only and double-iterated in epoch distribution. Added _untrackOperator (swap-and-pop) + prune on zero stake; gap shrunk 50→49.
• F9 (Low) — duplicate-asset dedup only ran when the service had requirements; duplicates double-slashed/double-billed otherwise. Dedup is now unconditional.
• F10 (Low) — clamp incoming slashBps at receipt so a malformed cross-chain value can't inflate the deferred accumulator.

Deferred / documented (no code change)

• F5 (Med) — ServiceFeeDistributor.onDelegationChanged bakes lockMultiplierBps into the position score but gets no lock expiry, and the staking layer computes a blended multiplier across multiple locks, so a correct decay requires an onDelegationChanged interface change + staking-layer expiry threading + lazy decay in the O(1) reward-accumulator math (mirroring RewardVaults._decayExpiredLock). Deferred to a focused follow-up to avoid reward-accounting regressions; root cause + plan documented inline.
• F6 (Med) — the epoch-budget clamp is effectively dead (budget ≤ freeBalance ≤ poolBalance); the liability undercount only bites an upgrade from a pre-pendingRewardsLiability deployment, which does not exist here. Documented; any future migration must seed liability in a reinitializer.
• F11 (Info) — instantSlashEnabled is read & enforced in SlashingLib, but proposeSlash hardcodes instant=false, leaving it intentionally unreachable. Documented as a deliberate control (wiring instant slash needs a dispute-bypass review), not dead code.

Tests

• New repros: test/audit/batch2/AuditBatch2.t.sol (F1 leave/rejoin dilution, F2 bonded self-dispute, F9 duplicate-asset reject) and a BeaconL2 receiver test for F10.
• Regression guards repurposed for the intended default changes: F4 (default lock blocks JIT) in TokenizedExt/BlueprintExtensions, F7 (staker slice retained, not reallocated) in audit/medlow/InflationPool.
• Updated 10 slashing mechanism tests to post the new default dispute bond.
• Green: tangle/Slashing*, EndToEndSlashingTest, SvcLifecycle, JobsAgg, BLSAggregation, PerAssetExposureIntegration, DoubleExposureUnderSlashPoC, Rewards, InflationPool (+audit), RewardVaults, TokenizedExt, BlueprintExtensions, BeaconL2, Integration, MultiAssetDelegation, FullStackScenario, UpgradeFlow.

The full ~1855-test suite was not run in one shot — forge test with no path filter OOMs this box (full via-IR graph). Coverage was run per touched subsystem + integration/upgrade; changes are confined to the listed contracts. CI can run the full suite.

[tangletools]

✅ Auto-approved PR — 1d4ac030

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

_{tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-19T14:47:59Z}

[tangletools]

🟢 Value Audit — sound

Verdict	sound
Concerns	2 (1 low, 1 weak-concern)
Heuristic	0.0s
Duplication	0.0s
Interrogation	190.6s (2 bridge agents)
Total	190.6s

💰 Value — sound

Batch audit remediation (1 high + 4 med + low) across slashing, jobs, rewards, extensions, and beacon-L2 — every fix follows existing codebase patterns and closes real attack vectors without reinventing anything.

• What it does: Fixes 9 validated audit findings: F1 snapshots operator commitments at slash-propose time so execution cannot be evaded via leave/rejoin dilution; F2 sets a non-zero disputeBond default (0.02 ETH); F3 makes the aggregation-bypass guard fail-closed on hook failure; F4 seeds a non-zero stakeLockDuration default (1 day) in TokenizedBlueprintBase; F7 stops permissionless distributeEpoch from redirecti
• Goals it achieves: Close a high-severity slash-evasion vector (F1), harden three default configurations to be secure-by-default (F2, F4, F10), add defense-in-depth at job-result-verification and L2-receipt boundaries (F3, F10), prevent inflation-redirection and unbounded-list growth in rewards (F7, F8), and prevent duplicate-commitment weight inflation in billing/slashing paths (F9).
• Assessment: Every fix is minimal, targeted, and built in the codebase's existing grain — snapshotting follows SlashingManager._snapshotOperator (SlashingManager.sol:764) and ServicesLifecycle._snapshotJoinPrice (ServicesLifecycle.sol:789); fail-closed follows _proposerIsBlueprintControlled (Slashing.sol:286-289) and JobsAggregation's implicit fail-closed at line 94; swap-and-pop mirrors _untrackDelegatorOpera
• Better / existing approach: none — this is the right approach
• Model: opencode/deepseek/deepseek-v4-pro
• Bridge attempts: 1

🎯 Usefulness — sound

9 security fixes correctly wired into existing call paths and a documented deferral; no dead surface, no competing patterns, no awkward interfaces.

• Integration: Every changed function has a live, reachable caller. F1 snapshot is populated at propose (Slashing.sol:112-121), consumed at execute (Slashing.sol:513), deleted after (Slashing.sol:315,370) — 5 code references form a closed loop. F2 default feeds all unconfigured deployments via SlashingLib.initializeConfig:143. F3 fail-closed triggers from _processResultSubmission:97, the only plain-submission pa
• Fit with existing patterns: All changes follow the codebase's established patterns: propose-execute state machines with snapshots (F1 mirrors the _slashProposals pattern already in SlashingLib), swap-and-pop removal (F8 is structurally identical to _untrackDelegatorOperator at RewardVaults.sol:866-884), defense-in-depth guards (F6, F10 match the existing 'never trust cross-chain input' pattern), configurable defaults initial
• Real-world viability: Each fix handles failure modes beyond the happy path. F1: operator self-dispute → leave → rejoin with diluted commitment vector tested in AuditBatch2.t.sol:86-118. F3: hook revert/OOG/gas-grief correctly block submission (Base.sol:819-831 returns ok=false on any failure). F4: 1-day lock applies to every subsequent stake; existing stakes unaffected (correct). F7: front-runner with empty serviceIds
• Model: opencode/deepseek/deepseek-v4-pro
• Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added test/audit/batch2/AuditBatch2.t.sol

•    uint64 slashId = tangle.proposeSlash(serviceId, operator1, 1000, keccak256("evidence"));
  

💰 Value Audit

🟡 F9 inline dedup runs twice when service has requirements [duplication] ``

The new unconditional dedup check at ServicesLifecycle.sol:226-235 fires for every joinServiceWithCommitments call, and _validateSecurityCommitments at line 633-642 runs the identical O(n²) dedup again when requirements.length > 0 (line 238-240). Both pass, so no correctness bug — just redundant work. A cleaner refactor would remove dedup from validateSecurityCommitments and call it unconditionally, but the duplication is minor (commitment arrays are small, bounded by MAX_SECURITY_REQUIREMENTS

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260619T145258Z}