Skip to content

feat: emit msg.sender in cert revocation events (I-02)#47

Open
leanthebean wants to merge 1 commit into
base:mainfrom
leanthebean:chore/cert-revocation-event-sender
Open

feat: emit msg.sender in cert revocation events (I-02)#47
leanthebean wants to merge 1 commit into
base:mainfrom
leanthebean:chore/cert-revocation-event-sender

Conversation

@leanthebean

Copy link
Copy Markdown
Contributor

Doc finding — I-02 (Informational): Missing msg.sender in Cert Revoked/Unrevoked events

From the BLOCKSEC-5249: Nitro Validator post-Fusaka review audit (finding I-02):

The CertRevoked and CertUnrevoked events only provide the certificate id and not the msg.sender.

Recommendation: Add the msg.sender as an indexed address in the event to make it easier to monitor.

The fix

Add the acting account as a second indexed topic on both revocation events:

event CertRevoked(bytes32 indexed certHash, address indexed account);
event CertUnrevoked(bytes32 indexed certHash, address indexed account);

and record msg.sender at every emit site:

  • unrevokeCertemit CertUnrevoked(certId, msg.sender)
  • _revokeCertemit CertRevoked(certId, msg.sender) (reached via the external, access-gated revokeCert / revokeCerts, so msg.sender is the original caller — the owner for root halts or the delegated revoker for non-root certs)

CertVerified is intentionally left unchanged — the finding scopes to the revocation events only. No interface change is required; these events are declared in CertManager, not in ICertManager.

Tests

New CertRevocationEventTest:

  • test_RevokeCertEmitsSender / test_RevokeCertsEmitsSender / test_UnrevokeCertEmitsSender — assert the event carries the caller via vm.expectEmit.
  • test_RevokeCertRecordsActualCaller — sets a distinct delegated revoker, pranks as it, and asserts that address (not the contract) lands in the topic.

forge fmt + full forge test: 152 passed, 0 failed, 1 skipped.

Address BLOCKSEC-5249 finding I-02.

Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
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