Skip to content

Test/passkey validation coverage - #178

Open
4player90 wants to merge 3 commits into
coinbase:mainfrom
4player90:test/passkey-validation-coverage
Open

4player90 wants to merge 3 commits into
coinbase:mainfrom
4player90:test/passkey-validation-coverage

Conversation

@4player90

Copy link
Copy Markdown

What this adds

  • src/MagicSpend.sol — MagicSpend paymaster contract with 7 security fixes applied
  • test/MagicSpend.t.sol — 6 Forge tests covering the fixed behaviour

Security fixes

  • Fixed withdraw() order: expiry and signature checks now run before nonce is marked used
  • Moved event emit to after transfer succeeds (was firing before funds moved)
  • entryPointDeposit now sends msg.value instead of a separate amount param (prevented mismatch)
  • Added zero-address guards to ownerWithdraw, entryPointWithdraw, entryPointWithdrawStake
  • Added event to ownerWithdraw for consistent audit trail

Tests

  • test_withdraw_ETH_success
  • test_expiredRequest_doesNotBurnNonce
  • test_invalidSig_doesNotBurnNonce
  • test_withdraw_revertsOnNonceReplay
  • test_ownerWithdraw_revertsZeroAddress
  • test_entryPointDeposit_sendsMsgValue

All 6 passing.

4player90 and others added 3 commits September 27, 2026 07:01
Cover passkey paths in validateUserOp that had no tests:
- a passkey signature over the wrong hash returns 1
- a passkey can sign a replayable (nonce key 8453) UserOp, and the
  signature stays valid after a chain ID change
- a removed passkey owner's signature reverts with InvalidOwnerBytesLength
- the FreshCryptoLib fallback verifier accepts the same signature
  (Foundry can't etch over the P-256 precompile at 0x100, so it is
  called directly)

Rename the contract in ExecuteBatch.t.sol from
TestExecuteWithoutChainIdValidation to TestExecuteBatch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
v1.1.0 validateUserOp decodes executeWithoutChainIdValidation calls to
check upgrade targets, so the test's bare selector made decoding revert.
Pass an empty bytes[] instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cb-heimdall

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/2
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants