Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors and extends the upgradeable proxy system for the Blueprint/Agent contracts while adding comprehensive tests, deployment, and upgrade scripts. Key changes include:
- Upgrading Blueprint implementation to V7 and introducing a new RouterV1 using delegatecall fallback to better support meta-transactions.
- Refactoring of EIP712 digest generation and signature verification methods across contracts.
- Updates to test suites and deployment scripts to support the new proxy “diamond pattern” and improve overall contract maintenance.
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MockERC20.sol | Added custom error types and commented-out legacy balance-check code. |
| test/BlueprintV6.t.sol | Updated tests for deployment config update with BlueprintV6 changes. |
| test/BlueprintV7.t.sol | Added tests to verify BlueprintV7 functionality and its initialize logic. |
| src/RouterV1.sol | Introduced a new router contract with improved fallback delegation. |
| src/EIP712.sol | Added new digest functions for copy agent fee and copy agent request. |
| script/DeployRouter.s.sol | Deployment script to deploy and configure RouterV1 along with its selectors. |
| script/UpgradeRouter.s.sol | Upgrade script for switching the proxy to a new RouterV1 implementation. |
| Others | Various other changes across Blueprints, Agent, Payment, and Storage for upgrade compatibility. |
Comments suppressed due to low confidence (1)
test/BlueprintV6.t.sol:63
- [nitpick] Clarify the comment regarding setting cost to zero as 'less than 0' is misleading since 0 is not less than 0; consider rephrasing the comment for improved clarity.
// verify user balance after top up
| // uint256 bal = _balances[msg.sender]; | ||
| // if (bal < amount) { | ||
| // revert InsufficientBalance(msg.sender, bal, amount); | ||
| // } |
There was a problem hiding this comment.
[nitpick] Consider removing or clarifying the commented-out balance check code to improve readability and maintainability in tests.
| // uint256 bal = _balances[msg.sender]; | |
| // if (bal < amount) { | |
| // revert InsufficientBalance(msg.sender, bal, amount); | |
| // } |
Sherlock AI FindingsThe automated tool identified the following potential security issues in the codebase. Please review the details for each issue in the linked dashboard.
Next Steps: Review the linked issues in the dashboard and address high-severity bugs first. Contact the team if you need assistance. Full report available at: https://ai.sherlock.xyz/runs/04fef9d2-275e-4303-8120-9a827cb30dc4 |
Type of Changes
What kind of change does this PR introduce?
Notion Task
Feature/Bugfix Details
Existing Behavior
New Behavior
Breaking Changes
Requirements
Please check if the PR fulfills these requirements
Other information