-
Notifications
You must be signed in to change notification settings - Fork 0
Adds a Basic PredicateClient implementation #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Created BasicPredicateClient for simple policies (no function/value validation) - Created AdvancedPredicateClient for complex policies (full function/value support) - Added BasicVault example demonstrating simplified integration - Added AdvancedVault example showing function-specific and value-based rules - Updated README with clear decision guide for choosing implementations - Maintained existing PredicateClient unchanged (audited code preserved) - All existing tests pass without modification
Adds Basic and Advanced PredicateClient implementations
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
- Reduced both vault examples to minimal code needed to demonstrate concepts - BasicVault: Shows simple WHO-based authorization with no encoding - AdvancedVault: Shows function-specific and value-based authorization - Added clear documentation about required setPolicyID/setRegistry implementations - Noted that business logic contracts must implement access control for setters - Clarified that getPolicyID/getRegistry are inherited and exposed
Added clear guidance: 'Do I need to enforce different rules based on WHAT users are doing, or just WHO is doing it?' This helps developers quickly determine which PredicateClient implementation to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| balances[msg.sender] += msg.value; | ||
| emit Deposit(msg.sender, msg.value); | ||
| // Lock logic would go here | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing _executeLock call after authorization in depositAndLock
Low Severity
The depositAndLock function encodes and authorizes a call to _executeLock(address,uint256,uint256) but never actually calls _executeLock. After authorization, it only performs a regular deposit by updating balances and emitting a Deposit event. This breaks the pattern established by other functions like withdraw, withdrawTo, and transfer, which all call their corresponding internal execution functions after authorization. The _lockPeriod parameter is completely ignored in the actual execution, making the function misleading as example code for developers implementing lock functionality.
- Added BasicPredicateClient for WHO-based authorization only - Added BasicVault example in inheritance folder (simplified authorization) - Added AdvancedVault example in inheritance folder (uses existing PredicateClient) - Updated README with decision guide: 'Do I need different rules based on WHAT users are doing, or just WHO?' - Removed duplicate AdvancedPredicateClient (use existing PredicateClient instead) - All examples now in src/examples/inheritance/ folder
|
Wondering if we should rename |
Problem
Most teams using Predicate only need simple WHO-based access control (KYC, allowlists, time restrictions), but today they're forced to encode function signatures and handle parameters they don't actually use.
Solution
Split into two implementations:
Decision guide: "Do I need different rules based on WHAT users are doing, or just WHO is doing it?"
Note: to simplify the entire flow, we will need to modify the attestation API to ignore the data and msg_value fields
Note
Introduces a simplified client for common WHO-only policies and updates examples and docs to clarify integration choices.
BasicPredicateClientmixin implementing WHO-based authorization only: ERC-7201 storage,PredicateRegistryUpdated/PredicatePolicyIDUpdatedevents, and_authorizeTransactionusing canonical zeromsgValueand emptyencodedSigAndArgsBasicVault(no encoding; sender-only validation) andAdvancedVault(function/value/parameter-aware validation using encoded data and optional msg.value)BasicPredicateClientandPredicateClient, concise examples, and decision matrix; links tosrc/examples/inheritance/Written by Cursor Bugbot for commit 2e71ceb. This will update automatically on new commits. Configure here.