Add drop authorization ADR#20
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ADR 0001 documenting EIP-712 drop authorization (typed drop intent, EOA/ERC-1271 support, replay/revocation, signer epochs) and updates ADR README, ROADMAP links, and AUTONOMOUS_RUN operational tracking to record acceptance and follow-up work. ChangesDrop Authorization ADR
Sequence DiagramsequenceDiagram
participant Caller
participant StreamDrops
participant ReplayState
Caller->>StreamDrops: Submit signed DropIntent (dropId, deadline, signerEpoch, payload)
StreamDrops->>StreamDrops: Verify EIP-712 signature (EOA or ERC-1271)
StreamDrops->>StreamDrops: Validate domain, deadline, signerEpoch, and malleability rules
StreamDrops->>ReplayState: Check dropId not consumed or cancelled
StreamDrops->>ReplayState: Mark dropId consumed before external calls
StreamDrops-->>Caller: Execute drop with signer-authorized parameters
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 drop authorization ADR before implementation starts. Please focus on whether the ADR gives a complete and safe design for EIP-712 schema/domain separation, |
punk6529
force-pushed
the
codex/drop-authorization-adr
branch
from
7ccc758 to
0c8d54a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0001-drop-authorization.md`:
- Around line 73-85: Clarify the relationship and purpose of the signed fields
`nonce` and `dropId` in the ADR: either state that `nonce` is used to derive
`dropId` (describe the derivation and include the algorithm/concatenation and
hashing step), or state that `nonce` is a per-signerEpoch sequence number that
provides ordering/uniqueness within an epoch (explain how validation uses
signerEpoch + nonce and how replay protection uses
`consumedDropIds`/`cancelledDropIds` vs per-epoch nonces), or remove/rename
`nonce` if it is redundant and leave only `dropId` as the globally unique
replay-protected identifier; reference the existing symbols `dropId`, `nonce`,
`signerEpoch`, `consumedDropIds`, and `cancelledDropIds` so readers can update
the replay/validation and signer compromise sections consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53db0ee2-6ff1-451a-ae9f-e05a88423324
📒 Files selected for processing (4)
docs/adr/0001-drop-authorization.mddocs/adr/README.mdops/AUTONOMOUS_RUN.mdops/ROADMAP.md
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ops/AUTONOMOUS_RUN.md (1)
36-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the "Last updated" timestamp to reflect the latest decision log entry.
The timestamp shows
2026-06-10 02:04 UTC, but the Decision Log at line 473 records an activity at2026-06-10 02:18 UTC. The "Last updated" field should be at least 02:18 UTC or later to accurately reflect when this operational state was last modified.🕐 Proposed fix
-| Last updated | `2026-06-10 02:04 UTC` | +| Last updated | `2026-06-10 02:18 UTC` |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ops/AUTONOMOUS_RUN.md` at line 36, Update the "Last updated" timestamp value `2026-06-10 02:04 UTC` in AUTONOMOUS_RUN.md to reflect the most recent Decision Log entry (at `2026-06-10 02:18 UTC`); replace the old timestamp string with `2026-06-10 02:18 UTC` or a later UTC timestamp so the "Last updated" field accurately matches the Decision Log activity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ops/AUTONOMOUS_RUN.md`:
- Line 36: Update the "Last updated" timestamp value `2026-06-10 02:04 UTC` in
AUTONOMOUS_RUN.md to reflect the most recent Decision Log entry (at `2026-06-10
02:18 UTC`); replace the old timestamp string with `2026-06-10 02:18 UTC` or a
later UTC timestamp so the "Last updated" field accurately matches the Decision
Log activity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 501d88a4-97ef-47de-872c-af8318db0d25
📒 Files selected for processing (2)
docs/adr/0001-drop-authorization.mdops/AUTONOMOUS_RUN.md
✅ Files skipped from review due to trivial changes (1)
- docs/adr/0001-drop-authorization.md
|
Addressed Claude's three ADR review threads in Resolution:
Validation after the changes:
|
|
Addressed the CodeRabbit timestamp follow-up in What changed:
Validation:
|
|
Addressed the Claude Resolution:
Validation:
|
|
@claude please re-review latest head |
1 participant
Summary
docs/adr/0001-drop-authorization.mdas the accepted design gate before P0 auth rewrites.Follow-up implementation issues
P0-AUTH-001: removetx.originfrom drop execution).P0-AUTH-002: replace drop authorization with replay-safe EIP-712 typed data).P0-AUTH-003: implement ERC-1271 contract signer support).Notes
Acceptedso the implementation PRs can proceed against a concrete design instead of re-deciding the authorization model mid-refactor.Validation
rg -n "^#|^##|^###" docs\adr\0001-drop-authorization.md docs\adr\README.md ops\ROADMAP.md ops\AUTONOMOUS_RUN.mdrg -n "P0-AUTH-ADR|P0-AUTH-001|P0-AUTH-002|P0-AUTH-003|0001-drop-authorization" docs\adr\0001-drop-authorization.md docs\adr\README.md ops\ROADMAP.md ops\AUTONOMOUS_RUN.mdgit diff --checkmake check(17 tests passed; known compiler warnings remain)powershell -ExecutionPolicy Bypass -File scripts\check.ps1(17 tests passed; known compiler warnings remain)Closes #17
Summary by CodeRabbit