Skip to content

CustomRevertDecoder#470

Open
saucepoint wants to merge 12 commits into
mainfrom
sauce/custom-revert-decoder
Open

CustomRevertDecoder#470
saucepoint wants to merge 12 commits into
mainfrom
sauce/custom-revert-decoder

Conversation

@saucepoint

@saucepoint saucepoint commented May 12, 2025

Copy link
Copy Markdown
Contributor

Related Issue

Developers are reporting that CustomRevert wrapping can be frustrating to use, given that it produces an arbitrary concatenated byte string

Description of changes

Introduce CustomRevertDecoder which slices out the data into descriptive variables

@saucepoint saucepoint marked this pull request as ready for review May 15, 2025 15:41

@gretzke gretzke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to not only be able to decode bytes4 for the additional context but also bytes for selectors with arguments. We don't use them in v4-core but I think it would be helpful to make it general purpose

Comment on lines +57 to +70
// 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)))

@saucepoint saucepoint May 15, 2025

Copy link
Copy Markdown
Contributor Author

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 additionalContext for reverts of the following format:

bytes memory data = abi.encodeWithSelector(
    wrappedErrorSelector,
    revertingContract,
    revertingFunctionSelector,
    abi.encodeWithSelector(revertReasonSelector, reasonData),
    abi.encodeWithSelector(additionalContextSelector, additionalContextData) // <-- additionalContextData
);

I'm reusing your size / looping logic -- is re-using mload(0x40) safe? what about the 0x24 magic-offset?

@gretzke gretzke May 16, 2025

Copy link
Copy Markdown
Contributor

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 of abi.encodeWithSelector(selector,uint256,bytes) would be different.

@saucepoint saucepoint removed the request for review from hensha256 May 15, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants