-
Notifications
You must be signed in to change notification settings - Fork 80
Output validator [OutputValidator v1.0.0] #1327
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
Open
0xDEnYO
wants to merge
35
commits into
main
Choose a base branch
from
output-validator-lf-15125
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
1d4a5f1
create new contract OutputValidator + tests + docs
0xDEnYO 98a74d4
deploy to OPT and ARB staging
0xDEnYO 2a7abcc
some smaller adjustments
0xDEnYO 8a7c1a1
add constructor parameter validation
0xDEnYO 2a9abaf
Update test/solidity/Periphery/OutputValidator.t.sol
0xDEnYO 74ddaf5
Merge branch 'main' of github.com:lifinance/contracts into output-val…
0xDEnYO 0c67927
Logic in OutputValidator updated
0xDEnYO dbd39aa
update conventions
0xDEnYO 8717c9b
update deploy requirements
0xDEnYO 48d9599
update deploy scripts
0xDEnYO f1c3e04
unit tests updated (100%)
0xDEnYO 7b2b1a0
add withdrawablePeriphery as parent contract
0xDEnYO a5dd76a
update deploy requirements
0xDEnYO 62e13ed
update deploy scripts
0xDEnYO 5223a5e
update docs
0xDEnYO 5e2770d
increase unit test coverage to 100%
0xDEnYO 30f2c31
some smaller changes
0xDEnYO 3a1d8fd
re-added complex test cases
0xDEnYO ca79be2
remove unnecessary content from docs file
0xDEnYO 34f69f9
Update docs/OutputValidator.md
0xDEnYO 9b41cef
Update src/Periphery/OutputValidator.sol
0xDEnYO 27c23fb
Update docs/OutputValidator.md
0xDEnYO e15e372
Merge branch 'main' of github.com:lifinance/contracts into output-val…
0xDEnYO 60ecd6a
Merge branch 'output-validator-lf-15125' of github.com:lifinance/cont…
0xDEnYO a558756
update comments
0xDEnYO 97b1893
harmonize tests
0xDEnYO c78d983
update logic in OutputValidator
0xDEnYO 4b50606
update docs
0xDEnYO 514f66c
deployed to OPT/ARB staging
0xDEnYO 9427846
updated unit test
0xDEnYO ab084c4
verify staging deployments
0xDEnYO 38e0ade
update unit tests
0xDEnYO 9cb5553
add outputValidator signatures to sigs.json
0xDEnYO 17231bf
Merge branch 'main' of github.com:lifinance/contracts into output-val…
0xDEnYO 3a1012a
remove duplicate entries from diamond logs
0xDEnYO File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| # OutputValidator | ||
|
|
||
| ## Overview | ||
|
|
||
| The `OutputValidator` contract is a periphery contract that validates swap output amounts and handles positive slippage by transferring excess tokens to a designated validation wallet. It is designed to be called by the Diamond contract after a swap operation to ensure that any excess output tokens are properly managed. | ||
|
|
||
| ## Key Features | ||
|
|
||
| - **Output Validation**: Assumes actual output amount exceeds expected amount for gas efficiency | ||
| - **Excess Token Management**: Automatically transfers excess tokens to a validation wallet | ||
| - **Dual Token Support**: Handles both native (ETH) and ERC20 tokens | ||
| - **Gas Optimized**: Eliminates conditional checks and assumes positive slippage scenarios (~200-300 gas saved per call) | ||
| - **Comprehensive Test Coverage**: 100% line coverage with 19 test cases | ||
|
|
||
| ## Contract Logic | ||
|
|
||
| ### Native Token Flow | ||
|
|
||
| 1. The calling contract (Diamond) sends native tokens as `msg.value` to the OutputValidator | ||
| 2. The contract always returns the expected amount to the calling contract using `LibAsset.transferAsset` | ||
| 3. **Assumes positive slippage**: Always transfers excess to validation wallet (handles zero excess by transferring 0 tokens) | ||
|
|
||
| ### ERC20 Token Flow | ||
|
|
||
| 1. The calling contract (Diamond) must have sufficient ERC20 token balance | ||
| 2. The OutputValidator checks the Diamond's ERC20 balance using `ERC20(tokenAddress).balanceOf(msg.sender)` | ||
| 3. **Assumes positive slippage**: Always transfers excess to validation wallet (handles zero excess by transferring 0 tokens) | ||
| 4. The Diamond retains the expected amount | ||
|
|
||
| > **Gas Optimization**: The contract assumes `actualAmount > expectedAmount` and eliminates conditional checks. If this assumption is violated, the transaction reverts immediately on arithmetic underflow. Zero excess cases are handled gracefully by transferring 0 tokens. | ||
|
|
||
| **Note**: The contract successfully handles edge cases where `actualAmount == expectedAmount` by transferring 0 excess tokens, rather than reverting. | ||
|
|
||
| ## Functions | ||
|
|
||
| ### `validateOutput` | ||
|
|
||
| ```solidity | ||
| function validateOutput( | ||
| address tokenAddress, | ||
| uint256 expectedAmount, | ||
| address validationWalletAddress | ||
| ) external payable | ||
| ``` | ||
|
|
||
| **Parameters:** | ||
|
|
||
| - `tokenAddress`: The address of the token to validate (use `LibAsset.NULL_ADDRESS` for native tokens) | ||
| - `expectedAmount`: The expected amount of tokens | ||
| - `validationWalletAddress`: The address to send excess tokens to | ||
|
|
||
| **Behavior:** | ||
|
|
||
| - For native tokens: Receives tokens as `msg.value`, returns expected amount to caller, transfers excess to validation wallet | ||
| - For ERC20 tokens: Checks caller's balance, transfers excess to validation wallet using `transferFrom` | ||
|
|
||
| ## Errors | ||
|
|
||
| The contract does not define custom errors. Error handling is delegated to the underlying libraries: | ||
|
|
||
| - **Native token errors**: Handled by `LibAsset.transferAsset()` | ||
| - **ERC20 token errors**: Handled by `SafeTransferLib.safeTransferFrom()` | ||
| - **Input validation**: Handled by `LibAsset` library for null address checks | ||
|
|
||
| ## Integration | ||
|
|
||
| ### Example Usage | ||
|
|
||
| ```solidity | ||
| // For native tokens | ||
| outputValidator.validateOutput{value: actualAmount}( | ||
| LibAsset.NULL_ADDRESS, | ||
| expectedAmount, | ||
| validationWallet | ||
| ); | ||
|
|
||
| // For ERC20 tokens | ||
| // First approve the OutputValidator to spend tokens | ||
| token.approve(address(outputValidator), actualAmount); | ||
| outputValidator.validateOutput( | ||
| address(token), | ||
| expectedAmount, | ||
| validationWallet | ||
| ); | ||
| ``` | ||
|
|
||
| ## Security Considerations | ||
|
|
||
| - The contract inherits from `TransferrableOwnership` for secure ownership management | ||
| - Uses `SafeTransferLib` for safe ERC20 operations | ||
| - Custom errors provide gas-efficient error handling | ||
| - Input validation leverages `LibAsset.transferAsset` for null address checks | ||
|
|
||
| ## Test Coverage | ||
|
|
||
| The contract includes comprehensive test coverage with **100% line coverage** including: | ||
|
|
||
| ### **Core Functionality Tests** | ||
|
|
||
| - Native token validation with excess (positive slippage scenarios) | ||
| - ERC20 token validation with excess (positive slippage scenarios) | ||
| - Edge cases (zero expected amount, insufficient allowance) | ||
|
|
||
| ### **Integration Tests** | ||
|
|
||
| - Complete DEX swap + OutputValidator + Bridge flows | ||
| - ERC20 → ERC20 swap with positive slippage | ||
| - ERC20 → Native swap with positive slippage | ||
| - Native → ERC20 swap with positive slippage | ||
|
|
||
| ### **Negative Test Cases** | ||
|
|
||
| - Insufficient allowance scenarios | ||
| - Native transfer failures to invalid addresses | ||
| - Zero value with non-zero expected amount | ||
| - **No excess scenarios** (contract handles gracefully by transferring 0 tokens) | ||
|
|
||
| ### **Test Statistics** | ||
|
|
||
| - **19 test cases** covering all code paths | ||
| - **All branches covered** including edge cases | ||
| - **Realistic scenarios** using MockDEX and Diamond integration | ||
|
|
||
| > **Note**: Coverage tools may mark comment lines as uncovered, but all executable code lines achieve 100% coverage. | ||
|
|
||
| ## Deployment | ||
|
|
||
| The contract is deployed using the standard deployment script pattern and extracts the owner address from the global configuration file. The contract is automatically included in periphery contract deployments and is configured in `script/deploy/resources/deployRequirements.json`. | ||
|
|
||
| ### **Deployment Scripts** | ||
|
|
||
| - **Standard**: `script/deploy/Periphery/DeployOutputValidator.s.sol` | ||
| - **zkSync**: `script/deploy/zksync/DeployOutputValidator.zksync.s.sol` | ||
|
|
||
| Both scripts follow the established deployment patterns and integrate with the CREATE3Factory for predictable contract addresses. | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // SPDX-License-Identifier: UNLICENSED | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| pragma solidity ^0.8.17; | ||
|
|
||
| import { DeployScriptBase } from "./utils/DeployScriptBase.sol"; | ||
| import { OutputValidator } from "lifi/Periphery/OutputValidator.sol"; | ||
| import { stdJson } from "forge-std/Script.sol"; | ||
|
|
||
| contract DeployScript is DeployScriptBase { | ||
| using stdJson for string; | ||
|
|
||
| constructor() DeployScriptBase("OutputValidator") {} | ||
|
|
||
| function run() | ||
| public | ||
| returns (OutputValidator deployed, bytes memory constructorArgs) | ||
| { | ||
| constructorArgs = getConstructorArgs(); | ||
|
|
||
| deployed = OutputValidator(deploy(type(OutputValidator).creationCode)); | ||
| } | ||
|
|
||
| function getConstructorArgs() internal override returns (bytes memory) { | ||
| // get path of global config file | ||
| string memory globalConfigPath = string.concat( | ||
| root, | ||
| "/config/global.json" | ||
| ); | ||
|
|
||
| // read file into json variable | ||
| string memory globalConfigJson = vm.readFile(globalConfigPath); | ||
|
|
||
| // extract outputValidatorOwner address | ||
| address refundWalletAddress = globalConfigJson.readAddress( | ||
| ".refundWallet" | ||
| ); | ||
|
|
||
| return abi.encode(refundWalletAddress); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // SPDX-License-Identifier: UNLICENSED | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| pragma solidity ^0.8.17; | ||
|
|
||
| import { DeployScriptBase } from "./utils/DeployScriptBase.sol"; | ||
| import { OutputValidator } from "lifi/Periphery/OutputValidator.sol"; | ||
| import { stdJson } from "forge-std/Script.sol"; | ||
|
|
||
| contract DeployScript is DeployScriptBase { | ||
| using stdJson for string; | ||
|
|
||
| constructor() DeployScriptBase("OutputValidator") {} | ||
|
|
||
| function run() | ||
| public | ||
| returns (OutputValidator deployed, bytes memory constructorArgs) | ||
| { | ||
| constructorArgs = getConstructorArgs(); | ||
|
|
||
| deployed = OutputValidator(deploy(type(OutputValidator).creationCode)); | ||
| } | ||
|
|
||
| function getConstructorArgs() internal override returns (bytes memory) { | ||
| // get path of global config file | ||
| string memory globalConfigPath = string.concat( | ||
| root, | ||
| "/config/global.json" | ||
| ); | ||
|
|
||
| // read file into json variable | ||
| string memory globalConfigJson = vm.readFile(globalConfigPath); | ||
|
|
||
| // extract outputValidatorOwner address | ||
| address refundWalletAddress = globalConfigJson.readAddress( | ||
| ".refundWallet" | ||
| ); | ||
|
|
||
| return abi.encode(refundWalletAddress); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.