Implement ERC-1271 drop authorization#57
Conversation
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds ERC-1271 contract-signer validation to StreamDrops: mintDrop computes the EIP-712 digest and delegates signer resolution to _validateSigner, which routes to EOA ecrecover or ERC-1271 staticcall checks; includes comprehensive ERC-1271 tests and documentation/ops updates. ChangesERC-1271 Authorization Implementation
Sequence diagram: sequenceDiagram
participant Caller
participant mintDrop as StreamDrops.mintDrop
participant validate as _validateSigner
participant EOA as _recoverEOASigner
participant ERC1271 as IERC1271.isValidSignature
Caller->>mintDrop: call mintDrop(DropAuthorization, signature)
mintDrop->>validate: compute digest, pass signer & signature
validate->>validate: check signer.code.length
alt EOA (no code)
validate->>EOA: ecrecover(digest, signature)
EOA-->>validate: recovered address
else Contract (has code)
validate->>ERC1271: staticcall isValidSignature(digest, signature)
ERC1271-->>validate: bytes4 magic or revert/invalid data
validate->>validate: verify success && magic == ERC1271_MAGIC_VALUE
end
validate-->>mintDrop: validated signer or revert
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@claude please review this PR. Focus areas:
|
|
@coderabbitai review Please review the latest head |
|
✅ Action performedFull review finished. |
|
@coderabbitai the latest review comment reports no actionable comments and pre-merge checks passed for |
|
✅ Action performedFull review finished. |
|
Autonomous merge evidence for latest head
Proceeding with squash merge under the AFK autonomous-run instruction set. |
Summary
DropAuthorizationpath.isValidSignature(bytes32,bytes).0x1626ba7eand fails closed on reverted checks, empty returns, short returns, extra return data, invalid magic, wrong digest, and wrong signature bytes.StreamDropsERC1271.t.soltarget-state coverage for fixed-price and auction contract-signer flows, negative contract-signer paths, replay, expiry, and EOA regression.Closes #19.
Validation
forge test --match-contract StreamDropsERC1271Test -vvvpassed with 12 tests.forge test --match-contract StreamDropsEIP712Test -vvvpassed with 23 tests.make checkpassed with 59 tests and the known Solidity warning baseline.powershell -ExecutionPolicy Bypass -File scripts\check.ps1passed with 59 tests and the known Solidity warning baseline.forge fmt --check smart-contracts\StreamDrops.sol test\StreamDropsERC1271.t.sol test\StreamDropsEIP712.t.solpassed.git diff --checkandgit diff --cached --checkpassed.Notes
Summary by CodeRabbit
New Features
Documentation
Tests
Chores