-
Notifications
You must be signed in to change notification settings - Fork 621
CustomRevertDecoder #470
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
saucepoint
wants to merge
12
commits into
main
Choose a base branch
from
sauce/custom-revert-decoder
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
CustomRevertDecoder #470
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
98842d3
draft / initial working example
saucepoint 472b5f4
test: update decode function to return revert reason params (#471)
clemlak 7b58a7c
resolve conflicts, adjust tests
saucepoint 207baac
variable consistency
saucepoint a23fdd7
another variable
saucepoint 89f09d6
additional testing
saucepoint 74a0ed8
organize to proper file
saucepoint 36c1f6e
remove console.log
saucepoint cd1bd8e
resync dependency
saucepoint e44bef8
Merge branch 'main' into sauce/custom-revert-decoder
saucepoint 435edde
natspec
saucepoint 717905e
support arbitrary bytes in additionalContext
saucepoint 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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| //SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.0; | ||
|
|
||
| /// @title CurrencyLibrary | ||
| /// @notice Decodes arbitrary revert strings from the CustomRevert library | ||
| library CustomRevertDecoder { | ||
| /// @notice Decodes a revert string from the CustomRevert library | ||
| /// @param err the revert string returned by CustomRevert.bubbleUpAndRevertWith | ||
| /// @return wrappedErrorSelector the selector of the wrapped error, typically CustomRevert.WrappedError.selector | ||
| /// @return revertingContract the address of the contract that reverted | ||
| /// @return revertingFunctionSelector the selector of the function that reverted | ||
| /// @return revertReasonSelector the selector of the revert reason | ||
| /// @return revertReason the revert reason, typically abi.encodeWithSelector(revertReasonSelector, reasonData) | ||
| /// @return additionalContextSelector the selector of the additional contextual revert | ||
| function decode(bytes memory err) | ||
| internal | ||
| pure | ||
| returns ( | ||
| bytes4 wrappedErrorSelector, | ||
| address revertingContract, | ||
| bytes4 revertingFunctionSelector, | ||
| bytes4 revertReasonSelector, | ||
| bytes memory revertReason, | ||
| bytes4 additionalContextSelector, | ||
| bytes memory additionalContextReason | ||
| ) | ||
| { | ||
| assembly { | ||
| wrappedErrorSelector := mload(add(err, 0x20)) | ||
| revertingContract := mload(add(err, 0x24)) | ||
| revertingFunctionSelector := mload(add(err, 0x44)) | ||
|
|
||
| let offsetRevertReason := mload(add(err, 0x64)) | ||
| let offsetAdditionalContext := mload(add(err, 0x84)) | ||
| let sizeRevertReason := mload(add(err, add(offsetRevertReason, 0x24))) | ||
| let sizeAdditionalContext := mload(add(err, add(offsetAdditionalContext, 0x24))) | ||
|
|
||
| revertReasonSelector := mload(add(err, add(offsetRevertReason, 0x44))) | ||
| additionalContextSelector := mload(add(err, add(offsetAdditionalContext, 0x44))) | ||
|
|
||
| // Decode the revert reason | ||
| let ptr := mload(0x40) | ||
| revertReason := ptr | ||
| mstore(revertReason, sizeRevertReason) | ||
|
|
||
| let w := not(0x1f) | ||
|
|
||
| for { let s := and(add(sizeRevertReason, 0x20), w) } 1 {} { | ||
| mstore(add(revertReason, s), mload(add(err, add(offsetRevertReason, add(0x24, s))))) | ||
| s := add(s, w) | ||
| if iszero(s) { break } | ||
| } | ||
|
|
||
| mstore(0x40, add(ptr, add(0x20, sizeRevertReason))) | ||
| // ------ | ||
|
|
||
| // Decode the additional context | ||
| ptr := mload(0x40) | ||
| additionalContextReason := ptr | ||
| mstore(additionalContextReason, sizeAdditionalContext) | ||
|
|
||
| w := not(0x1f) | ||
|
|
||
| for { let s := and(add(sizeAdditionalContext, 0x20), w) } 1 {} { | ||
| mstore(add(additionalContextReason, s), mload(add(err, add(offsetAdditionalContext, add(0x24, s))))) | ||
| s := add(s, w) | ||
| if iszero(s) { break } | ||
| } | ||
|
|
||
| mstore(0x40, add(ptr, add(0x20, sizeAdditionalContext))) | ||
| } | ||
| } | ||
| } | ||
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,206 @@ | ||
| //SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.24; | ||
|
|
||
| import {Test} from "forge-std/Test.sol"; | ||
| import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol"; | ||
| import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol"; | ||
| import {LPFeeLibrary} from "@uniswap/v4-core/src/libraries/LPFeeLibrary.sol"; | ||
| import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol"; | ||
| import {CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol"; | ||
| import {CustomRevert} from "@uniswap/v4-core/src/libraries/CustomRevert.sol"; | ||
|
|
||
| import {CustomRevertDecoder} from "../../src/utils/CustomRevertDecoder.sol"; | ||
|
|
||
| contract CustomRevertDecoderTest is Test { | ||
| function setUp() public {} | ||
|
|
||
| function test_fuzz_decode_customRevert( | ||
| bytes4 wrappedErrorSelector, | ||
| address revertingContract, | ||
| bytes4 revertingFunctionSelector, | ||
| bytes4 revertReasonSelector, | ||
| bytes memory reasonData, | ||
| bytes4 additionalContextSelector, | ||
| bytes memory additionalContextData | ||
| ) public pure { | ||
| bytes4 _decodedWrapSelector; | ||
| address _decodedRevertingContract; | ||
| bytes4 _decodedRevertingFunctionSelector; | ||
| bytes4 _decodedRevertReasonSelector; | ||
| bytes memory _decodedReason; | ||
| bytes4 _decodedAdditionalContextSelector; | ||
| bytes memory _decodedAdditionalContextReason; | ||
|
|
||
| // scoped to avoid stack too deep error | ||
| { | ||
| bytes memory data = abi.encodeWithSelector( | ||
| wrappedErrorSelector, | ||
| revertingContract, | ||
| revertingFunctionSelector, | ||
| abi.encodeWithSelector(revertReasonSelector, reasonData), | ||
| abi.encodeWithSelector(additionalContextSelector, additionalContextData) | ||
| ); | ||
|
|
||
| ( | ||
| _decodedWrapSelector, | ||
| _decodedRevertingContract, | ||
| _decodedRevertingFunctionSelector, | ||
| _decodedRevertReasonSelector, | ||
| _decodedReason, | ||
| _decodedAdditionalContextSelector, | ||
| _decodedAdditionalContextReason | ||
| ) = CustomRevertDecoder.decode(data); | ||
| } | ||
|
|
||
| assertEq(_decodedWrapSelector, wrappedErrorSelector); | ||
| assertEq(_decodedRevertingContract, revertingContract); | ||
| assertEq(_decodedRevertingFunctionSelector, revertingFunctionSelector); | ||
| assertEq(_decodedRevertReasonSelector, revertReasonSelector); | ||
| assertEq(_decodedReason, abi.encodeWithSelector(revertReasonSelector, reasonData)); | ||
| assertEq(_decodedAdditionalContextSelector, additionalContextSelector); | ||
| assertEq( | ||
| _decodedAdditionalContextReason, abi.encodeWithSelector(additionalContextSelector, additionalContextData) | ||
| ); | ||
| } | ||
|
|
||
| function test_decode_empty() public pure { | ||
| bytes4 wrappedErrorSelector = CustomRevert.WrappedError.selector; | ||
| address revertingContract = address(0x1111); | ||
| bytes4 revertingFunctionSelector = bytes4(0); | ||
| bytes4 revertReasonSelector = bytes4(0); | ||
| bytes4 additionalContextSelector = CurrencyLibrary.NativeTransferFailed.selector; | ||
|
|
||
| bytes memory data = abi.encodeWithSelector( | ||
| wrappedErrorSelector, | ||
| revertingContract, | ||
| revertingFunctionSelector, | ||
| abi.encodeWithSelector(revertReasonSelector), | ||
| abi.encodeWithSelector(additionalContextSelector) | ||
| ); | ||
|
|
||
| ( | ||
| bytes4 _decodedWrapSelector, | ||
| address _decodedRevertingContract, | ||
| bytes4 _decodedRevertingFunctionSelector, | ||
| bytes4 _decodedRevertReasonSelector, | ||
| bytes memory _decodedReason, | ||
| bytes4 _decodedAdditionalContextSelector, | ||
| bytes memory _decodedAdditionalContextReason | ||
| ) = CustomRevertDecoder.decode(data); | ||
|
|
||
| // assert original values against decoded values | ||
| assertEq(_decodedWrapSelector, wrappedErrorSelector); | ||
| assertEq(_decodedRevertingContract, revertingContract); | ||
| assertEq(_decodedRevertingFunctionSelector, revertingFunctionSelector); | ||
| assertEq(_decodedRevertReasonSelector, revertReasonSelector); | ||
| assertEq(_decodedReason, abi.encodeWithSelector(revertReasonSelector)); | ||
| assertEq(_decodedAdditionalContextSelector, additionalContextSelector); | ||
| assertEq(_decodedAdditionalContextReason, abi.encodeWithSelector(additionalContextSelector)); | ||
| } | ||
|
|
||
| function test_decode_singleParameter() public pure { | ||
| bytes4 wrappedErrorSelector = CustomRevert.WrappedError.selector; | ||
| address revertingContract = address(0x1111); | ||
| bytes4 revertingFunctionSelector = IHooks.afterInitialize.selector; | ||
| bytes4 revertReasonSelector = LPFeeLibrary.LPFeeTooLarge.selector; | ||
| uint24 reasonData = uint24(10000); | ||
| bytes4 additionalContextSelector = Hooks.HookCallFailed.selector; | ||
|
|
||
| bytes memory data = abi.encodeWithSelector( | ||
| wrappedErrorSelector, | ||
| revertingContract, | ||
| revertingFunctionSelector, | ||
| abi.encodeWithSelector(revertReasonSelector, reasonData), | ||
| abi.encodeWithSelector(additionalContextSelector) | ||
| ); | ||
|
|
||
| ( | ||
| bytes4 _decodedWrapSelector, | ||
| address _decodedRevertingContract, | ||
| bytes4 _decodedRevertingFunctionSelector, | ||
| bytes4 _decodedRevertReasonSelector, | ||
| bytes memory _decodedRevertReason, | ||
| bytes4 _decodedAdditionalContextSelector, | ||
| bytes memory _decodedAdditionalContextReason | ||
| ) = CustomRevertDecoder.decode(data); | ||
|
|
||
| // assert original values against decoded values | ||
| assertEq(_decodedWrapSelector, wrappedErrorSelector); | ||
| assertEq(_decodedRevertingContract, revertingContract); | ||
| assertEq(_decodedRevertingFunctionSelector, revertingFunctionSelector); | ||
| assertEq(_decodedRevertReasonSelector, revertReasonSelector); | ||
| assertEq(_decodedRevertReason, abi.encodeWithSelector(revertReasonSelector, reasonData)); | ||
| assertEq(_decodedAdditionalContextSelector, additionalContextSelector); | ||
| assertEq(_decodedAdditionalContextReason, abi.encodeWithSelector(additionalContextSelector)); | ||
| } | ||
|
|
||
| function test_decode_norevertReasonSelector() public pure { | ||
| bytes4 wrappedErrorSelector = CustomRevert.WrappedError.selector; | ||
| address revertingContract = address(0x1111); | ||
| bytes4 revertingFunctionSelector = IHooks.afterInitialize.selector; | ||
| bytes32 reason = bytes32(0); | ||
| bytes4 additionalContextSelector = CurrencyLibrary.ERC20TransferFailed.selector; | ||
|
|
||
| bytes memory data = abi.encodeWithSelector( | ||
| wrappedErrorSelector, | ||
| revertingContract, | ||
| revertingFunctionSelector, | ||
| abi.encode(reason), | ||
| abi.encodeWithSelector(additionalContextSelector) | ||
| ); | ||
|
|
||
| ( | ||
| bytes4 _decodedWrapSelector, | ||
| address _decodedRevertingContract, | ||
| bytes4 _decodedRevertingFunctionSelector, | ||
| bytes4 _decodedRevertReasonSelector, | ||
| bytes memory _decodedReason, | ||
| bytes4 _decodedAdditionalContextSelector, | ||
| bytes memory _decodedAdditionalContextReason | ||
| ) = CustomRevertDecoder.decode(data); | ||
|
|
||
| // assert original values against decoded values | ||
| assertEq(_decodedWrapSelector, wrappedErrorSelector); | ||
| assertEq(_decodedRevertingContract, revertingContract); | ||
| assertEq(_decodedRevertingFunctionSelector, revertingFunctionSelector); | ||
| assertEq(_decodedRevertReasonSelector, bytes4(0)); | ||
| assertEq(_decodedReason, abi.encode(reason)); | ||
| assertEq(_decodedAdditionalContextSelector, additionalContextSelector); | ||
| assertEq(_decodedAdditionalContextReason, abi.encodeWithSelector(additionalContextSelector)); | ||
| } | ||
|
|
||
| function test_decode_noReason() public pure { | ||
| bytes4 wrappedErrorSelector = CustomRevert.WrappedError.selector; | ||
| address revertingContract = address(0x1111); | ||
| bytes4 revertingFunctionSelector = IHooks.afterInitialize.selector; | ||
| bytes4 revertReasonSelector = IPoolManager.UnauthorizedDynamicLPFeeUpdate.selector; | ||
| bytes4 additionalContextSelector = Hooks.HookCallFailed.selector; | ||
|
|
||
| bytes memory data = abi.encodeWithSelector( | ||
| wrappedErrorSelector, | ||
| revertingContract, | ||
| revertingFunctionSelector, | ||
| abi.encodeWithSelector(revertReasonSelector), | ||
| abi.encodeWithSelector(additionalContextSelector) | ||
| ); | ||
|
|
||
| ( | ||
| bytes4 _decodedWrapSelector, | ||
| address _decodedRevertingContract, | ||
| bytes4 _decodedRevertingFunctionSelector, | ||
| bytes4 _decodedRevertReasonSelector, | ||
| bytes memory _decodedReason, | ||
| bytes4 _decodedAdditionalContextSelector, | ||
| bytes memory _decodedAdditionalContextReason | ||
| ) = CustomRevertDecoder.decode(data); | ||
|
|
||
| // assert original values against decoded values | ||
| assertEq(_decodedWrapSelector, wrappedErrorSelector); | ||
| assertEq(_decodedRevertingContract, revertingContract); | ||
| assertEq(_decodedRevertingFunctionSelector, revertingFunctionSelector); | ||
| assertEq(_decodedRevertReasonSelector, revertReasonSelector); | ||
| assertEq(_decodedReason, abi.encodeWithSelector(revertReasonSelector)); | ||
| assertEq(_decodedAdditionalContextSelector, additionalContextSelector); | ||
| assertEq(_decodedAdditionalContextReason, abi.encodeWithSelector(additionalContextSelector)); | ||
| } | ||
| } |
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.
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.
@clemlak I added support for arbitrary bytes in
additionalContextfor reverts of the following format:I'm reusing your size / looping logic -- is re-using
mload(0x40)safe? what about the0x24magic-offset?Uh oh!
There was an error while loading. Please reload this page.
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.
when you encode the additional context data you have to make sure it's encoded properly. Because you can't just use
abi.encodeWithSelector(selector,abi.encode(uint256,bytes))for example, as the encoded result ofabi.encodeWithSelector(selector,uint256,bytes)would be different.