Skip to content

Pin the executed Fireblocks-signer revocation as an absence - #323

Merged
hardyjosh merged 1 commit into
mainfrom
2026-08-25-fireblocks-revocation-pin-flip
Aug 25, 2026
Merged

Pin the executed Fireblocks-signer revocation as an absence#323
hardyjosh merged 1 commit into
mainfrom
2026-08-25-fireblocks-revocation-pin-flip

Conversation

@hardyjosh

@hardyjosh hardyjosh commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Why

Main's test CI (and cron) has been red since 2026-08-22, and #314's inherits it: the 20260810-revoke-fireblocks-service-signer Safe bundles executed on all three chains (verified live — the retired signer 0x1C66…D1A9 holds no DEPOSIT/WITHDRAW/CERTIFY on Base, Ethereum, or HyperEVM), which flipped its forcing-function tests red by design: they demand this pin PR.

What (per the script's own retire instructions)

  • Canonical grant map: the retired signer's three rows leave expectedGrants (16 → 13 entries). The GRANTEE_SERVICE_1C66 constant stays as the audit-trail record.
  • The revocation is pinned as an absence: assertExpectedGrants gains assertRetiredSignerAbsent — any action role landing back on the retired signer red-lines cron with the new UnexpectedRetiredSignerGrant. Strictly stronger than the old presence rows.
  • Spent fixtures retire: the script, its run-script.yaml registry entry, the .prod.t.sol forcing tests, the unit tests, and both harnesses are deleted — the standing absence assertion supersedes their pre-flight per the registry's deletion rule.

Consumers of the retired rows (second round — surfaced by CI, masked locally by RPC rate limits)

  • 20260619-deploy-v4-authoriser-clone: MIRROR_COUNT 9 → 6 (the operational slice is now Safe + service signer) plus its replica test.
  • 20260729-migrate-governance-to-timelock: the governance-loop proof's sample no-op op re-granted DEPOSIT to the retired signer — post-revocation that's a real grant and trips the absence check. Now re-grants to the live signer (GRANTEE_SERVICE_3D0C, a genuine no-op again).
  • 20260723-provision-additional-service-signer drift test: drift is now created by revoking the Safe's DEPOSIT row instead of the departed retired-signer row.

Verification

Against live forks on all three chains: the full previously-red set passes — provisioning prod tests (AlreadyProvisioned refusals restored), timelock-migration surface, cross-chain parity, Base prod-state, Ethereum clone pin. slither . 0, fmt/reuse clean.

Merge order

This merges first; the orchestrator stack (#314#316#317) restacks on it and its inherited test failures disappear.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KbsbYN4C4YDa8pu9DdudoX

Summary by CodeRabbit

  • Updates

    • Retired service signer access has been removed from the expected authorization configuration.
    • Authorization grants now reflect six operational grants, with Safe grants listed before service grants.
    • Governance validation now uses the active service signer for DEPOSIT.
    • Added checks to prevent accidental regranting of roles to the retired signer.
  • Maintenance

    • Removed the retired signer revocation workflow and its associated validation tests.

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b5dc2c06-ad37-4c66-94aa-687e69c22c77

📥 Commits

Reviewing files that changed from the base of the PR and between 4f12618 and 2345558.

📒 Files selected for processing (12)
  • .github/workflows/run-script.yaml
  • script/20260619-deploy-v4-authoriser-clone.s.sol
  • script/20260729-migrate-governance-to-timelock.s.sol
  • script/20260810-revoke-fireblocks-service-signer.s.sol
  • src/lib/LibAuthoriserInvariants.sol
  • test/script/20260619-deploy-v4-authoriser-clone.t.sol
  • test/script/20260723-provision-additional-service-signer.t.sol
  • test/script/20260810-revoke-fireblocks-service-signer.prod.t.sol
  • test/script/20260810-revoke-fireblocks-service-signer.t.sol
  • test/script/RevokeFireblocksServiceSignerHarness.sol
  • test/script/RevokeFireblocksServiceSignerRemainderHarness.sol
  • test/src/lib/LibAuthoriserInvariants.t.sol
💤 Files with no reviewable changes (6)
  • .github/workflows/run-script.yaml
  • test/script/20260810-revoke-fireblocks-service-signer.prod.t.sol
  • test/script/RevokeFireblocksServiceSignerRemainderHarness.sol
  • test/script/20260810-revoke-fireblocks-service-signer.t.sol
  • test/script/RevokeFireblocksServiceSignerHarness.sol
  • script/20260810-revoke-fireblocks-service-signer.s.sol

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The PR removes the retired Fireblocks service signer from the canonical grant map, adds an explicit absence check, updates signer and grant-count references, and deletes the signer revocation script with its tests and harnesses.

Changes

Retired service signer cleanup

Layer / File(s) Summary
Grant invariant updates
src/lib/LibAuthoriserInvariants.sol, test/src/lib/LibAuthoriserInvariants.t.sol, test/script/20260723-provision-additional-service-signer.t.sol
The expected grant map drops the retired signer’s three roles. assertRetiredSignerAbsent rejects any re-grant. Tests cover the new invariant and updated drift fixture.
Operational grant alignment
script/20260619-deploy-v4-authoriser-clone.s.sol, script/20260729-migrate-governance-to-timelock.s.sol, test/script/20260619-deploy-v4-authoriser-clone.t.sol
The deployment script mirrors six grants, Safe grants precede service grants, and governance re-grants DEPOSIT to GRANTEE_SERVICE_3D0C.
Revocation tooling removal
.github/workflows/run-script.yaml, script/20260810-revoke-fireblocks-service-signer.s.sol, test/script/20260810-revoke-fireblocks-service-signer*.t.sol, test/script/RevokeFireblocksServiceSigner*.sol
The workflow option, revocation script, production tests, unit tests, and harnesses are deleted.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 23455

This change records the executed signer revocation as an enforced absence and updates dependent fixtures and tests; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: thedavidmeister

Poem

A rabbit checks the grant map twice
The old signer hops out of sight
Six grants mirror in tidy rows
The Safe leads where service flows
No stale role remains tonight

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: recording the executed Fireblocks signer revocation as a permanent absence in the canonical grant state.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (6 skipped: 6 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-08-25-fireblocks-revocation-pin-flip

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@hardyjosh
hardyjosh force-pushed the 2026-08-25-fireblocks-revocation-pin-flip branch from a5458af to 93d2905 Compare August 25, 2026 09:11
The 20260810-revoke-fireblocks-service-signer bundles executed on all
three chains (verified live: the retired signer 0x1C66...D1A9 holds no
DEPOSIT/WITHDRAW/CERTIFY anywhere), which turned its forcing-function
tests red on main since 2026-08-22 - by design, demanding this pin PR.

Per the script's own retire instructions: the retired signer's three
rows leave the canonical grant map (16 -> 13), assertExpectedGrants now
asserts the ABSENCE (new UnexpectedRetiredSignerGrant refusal, so a
re-grant red-lines cron), and the spent script, its artifact-workflow
registry entry, and its test fixtures retire. The GRANTEE_SERVICE_1C66
constant stays as the audit-trail record the absence is asserted
against.

Verified against live forks: the full previously-red set (provisioning
prod tests, timelock migration surface, cross-chain parity, Base
prod-state) passes on Base, Ethereum and HyperEVM.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbsbYN4C4YDa8pu9DdudoX
@hardyjosh
hardyjosh force-pushed the 2026-08-25-fireblocks-revocation-pin-flip branch from 93d2905 to 2345558 Compare August 25, 2026 10:57
@hardyjosh
hardyjosh marked this pull request as ready for review August 25, 2026 11:09
@hardyjosh
hardyjosh merged commit 405ed78 into main Aug 25, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant