Conversation
Bumps the production-dependencies group with 1 update: [@fhevm/solidity](https://github.com/zama-ai/fhevm). Updates `@fhevm/solidity` from 0.10.0 to 0.11.0 - [Release notes](https://github.com/zama-ai/fhevm/releases) - [Commits](zama-ai/fhevm@v0.10.0...v0.11.0) --- updated-dependencies: - dependency-name: "@fhevm/solidity" dependency-version: 0.11.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: production-dependencies ... Signed-off-by: dependabot[bot] <support@github.com>
…ev/production-dependencies-53321bd464 chore(deps): bump @fhevm/solidity from 0.10.0 to 0.11.0 in the production-dependencies group
Bumps the production-dependencies group with 1 update: [@fhevm/solidity](https://github.com/zama-ai/fhevm). Updates `@fhevm/solidity` from 0.11.0 to 0.11.1 - [Release notes](https://github.com/zama-ai/fhevm/releases) - [Commits](https://github.com/zama-ai/fhevm/commits) --- updated-dependencies: - dependency-name: "@fhevm/solidity" dependency-version: 0.11.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: production-dependencies ... Signed-off-by: dependabot[bot] <support@github.com>
…ev/production-dependencies-1917e71b4e chore(deps): bump @fhevm/solidity from 0.11.0 to 0.11.1 in the production-dependencies group
📝 WalkthroughWalkthroughThe pull request updates the package version to 2.0.3 and bumps the "@fhevm/solidity" dependency to ^0.11.1. Additionally, the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/LiquidityOrchestrator.sol`:
- Around line 410-413: Add a unit test for
LiquidityOrchestrator.transferRedemptionFunds that covers the zero-amount no-op
path: from an authorized vault address call transferRedemptionFunds(user, 0) and
assert the call does not revert, the underlyingAsset ERC20 balance of the user
and the orchestrator remain unchanged, and no Transfer (or related) event is
emitted; use the existing authorized vault setup and the contract's
transferRedemptionFunds function name to locate where to call and assert
behavior.
In `@package.json`:
- Line 80: The package.json declares `@fhevm/solidity` at ^0.11.1 which violates
the peer dependency required by `@fhevm/hardhat-plugin`@0.1.0 (which expects
`@fhevm/solidity`@^0.8.0); fix by either downgrading `@fhevm/solidity` to a ^0.8.x
version in package.json or upgrade `@fhevm/hardhat-plugin` to a release that
officially supports ^0.11.1, then update the lockfile (run npm/yarn/pnpm
install) and verify no peer dependency warnings remain for `@fhevm/solidity` and
`@fhevm/hardhat-plugin`.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
contracts/LiquidityOrchestrator.solpackage.json
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAllows zero-amount redemption fund transfers without reverting, and updates package metadata and fhevm dependency versions. Sequence diagram for redemption fund transfer with zero-amount handlingsequenceDiagram
actor Vault
participant LiquidityOrchestrator
participant ERC20Token
Vault->>LiquidityOrchestrator: transferRedemptionFunds(user, amount)
LiquidityOrchestrator->>LiquidityOrchestrator: config.isOrionVault(msg.sender)
alt Caller is not registered vault
LiquidityOrchestrator-->>Vault: revert ErrorsLib.NotAuthorized
else Caller is registered vault
alt amount > 0
LiquidityOrchestrator->>ERC20Token: safeTransfer(user, amount)
ERC20Token-->>LiquidityOrchestrator: transfer result
LiquidityOrchestrator-->>Vault: success
else amount == 0
LiquidityOrchestrator-->>Vault: success (no transfer)
end
end
Class diagram for LiquidityOrchestrator transferRedemptionFunds logicclassDiagram
class LiquidityOrchestrator {
address underlyingAsset
Config config
+transferRedemptionFunds(address user, uint256 amount) external
}
class Config {
+isOrionVault(address caller) bool
}
class ErrorsLib {
+NotAuthorized() error
}
class IERC20 {
+safeTransfer(address to, uint256 amount)
}
LiquidityOrchestrator --> Config : uses
LiquidityOrchestrator --> ErrorsLib : reverts_with
LiquidityOrchestrator --> IERC20 : transfers_underlyingAsset
note for LiquidityOrchestrator "transferRedemptionFunds now allows amount == 0 without reverting and only calls IERC20.safeTransfer when amount > 0"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Now that
transferRedemptionFundsallowsamount == 0, consider documenting this behavioral change at the call sites or in the interface to make it clear that zero-amount calls are treated as no-ops rather than input validation errors. - With the removal of the
AmountMustBeGreaterThanZerorevert intransferRedemptionFunds, check whether this custom error is still used elsewhere and remove it if it has become dead code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that `transferRedemptionFunds` allows `amount == 0`, consider documenting this behavioral change at the call sites or in the interface to make it clear that zero-amount calls are treated as no-ops rather than input validation errors.
- With the removal of the `AmountMustBeGreaterThanZero` revert in `transferRedemptionFunds`, check whether this custom error is still used elsewhere and remove it if it has become dead code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Relax redemption transfer validation and update package metadata and dependencies.
Bug Fixes:
Build:
Summary by CodeRabbit
Bug Fixes
Chores