Skip to content

fix: emit ecrecover authorizer signer#966

Open
prd-carapulse[bot] wants to merge 5 commits into
mainfrom
hermes/ecrecover-authorizer-signer-event-1780397877
Open

fix: emit ecrecover authorizer signer#966
prd-carapulse[bot] wants to merge 5 commits into
mainfrom
hermes/ecrecover-authorizer-signer-event-1780397877

Conversation

@prd-carapulse

@prd-carapulse prd-carapulse Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds the recovered signer address to EcrecoverAuthorizer.SetIsAuthorized.
  • Keeps existing indexed topics (caller, authorizer, authorized) unchanged and appends signer as event data.
  • Adds regression coverage for delegated signer flow where signer != authorizer.

Request Context

  • Initiator: Adrien H (Slack U036QFAHCTF) / GitHub: @adhusson
  • Initial Slack thread: channel C0B787HAFQX, thread_ts 1780397877.321229
  • Additional context: "In midnight the event in ECRecoverAuthorizer doesn’t allow to recover who was the signer, make a pr that fixes this"
  • Rationale: Offchain consumers need the emitted event to identify the account that produced the accepted EIP-712 signature, especially delegated signatures.

Test Plan

  • forge test --force --match-path test/SetIsAuthorizedWithSigTest.sol --match-test testEcrecoverAuthorizerEventEmitsSigner -vvv
  • forge test --force --match-path test/SetIsAuthorizedWithSigTest.sol
  • forge build --force

Include the recovered signer in the EcrecoverAuthorizer SetIsAuthorized event so offchain consumers can reconstruct who signed delegated authorizations.
address indexed authorized,
bool isAuthorized,
uint256 nonce,
address signer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure where to put it, no clear consensus (github search), also don't think it matters much

Comment thread test/SetIsAuthorizedWithSigTest.sol Outdated
prd-carapulse Bot and others added 4 commits June 2, 2026 11:34
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