Raw revert for custom errors #3136
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3136 +/- ##
==========================================
- Coverage 88.49% 88.21% -0.28%
==========================================
Files 95 95
Lines 10760 10777 +17
Branches 2266 2268 +2
==========================================
- Hits 9522 9507 -15
- Misses 796 829 +33
+ Partials 442 441 -1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| def build_revert_bytestring(errorDecl, *data): | ||
| # revert bytes should be 4-byte selector (from keccak of error definition) | ||
| # followed by abi-encoded data | ||
| error_selector = keccak256(bytes(errorDecl, "utf-8"))[:4] | ||
| match = re.search(r"\((.*?)\)", errorDecl) # .group(1) | ||
| if match and match.group(1) != "": | ||
| arg_types = match.group(1) | ||
| encoded_data = encode(arg_types.split(","), data) | ||
| return b"".join([error_selector, encoded_data]) | ||
| else: | ||
| return error_selector |
There was a problem hiding this comment.
you probably want abi.encode_single, not abi.encode -- the latter is going to encode them individually, instead of together as a tuple (the encodings are different)
There was a problem hiding this comment.
That is now deprecated. So use encode wherever possible instead
There was a problem hiding this comment.
will add a test case encoding some more complex data and see how it handles it
There was a problem hiding this comment.
honestly, i don't even think it needs to be that complicated, since we don't really need to test arbitrary/generic revert data generation. when i originally said to construct the revert data using utility functions, i was thinking more just like method_id("NoFives()") + (5).to_bytes(32, "big"). (the point is for it to be clear, not necessarily generic)
There was a problem hiding this comment.
got it, I had to implement my own method_id function but otherwise followed what you said and made it simpler
use simpler encoding of expected error instead
| def method_id(method_str: str) -> bytes: | ||
| return keccak256(bytes(method_str, "utf-8"))[:4] |
There was a problem hiding this comment.
this feels like it should go in vyper/utils.py. i'm ok with it as it is as long as there is a note like # TODO should move to utils.py
There was a problem hiding this comment.
moving it there makes sense imo, but there's already a function in there called abi_method_id
the difference between that one and this one is that abi_method_id returns an int instead of bytes. this probably works in other places in the compiler, but it breaks something in the usage here.
when method_id returns bytes, we can concatenate them to other bytes like we do in the test:
revert_bytes = method_id("NoFives(uint256)") + (5).to_bytes(32, "big")
with a returned integer, this doesn't work
I think a function called method_id/abi_method_id should return bytes, and we should rename abi_method_id to something like method_id_int. happy to take suggestions on naming convention here, I'm not the best at coming up with names
There was a problem hiding this comment.
for now:
method_idreturns bytes. I think this is the best name (instead ofabi_method_id) mainly because it matches the name and the behavior of the builtin function usable from within Vyper, keeping things clear and consistentmethod_id_intreturns an int. The only difference in behavior betweenmethod_idandmethod_id_intis the return type
I also added a type annotation to method_id_int to reflect that it returns an int, even though the name is already clear, for editor integration purposes
| raw_revert(data) | ||
| """ | ||
|
|
||
| revert_bytes = method_id("NoFives(uint256)") + (5).to_bytes(32, "big") |
There was a problem hiding this comment.
maybe for clarity, use abi.encode_single("(uint256)", 5)
What I did
Added
raw_revertas a builtin function, working similarly toraw_logandraw_callHow I did it
Created a new
RawRevertclass. This represents a built-in function that stores any data passed to it to memory, and then passes the rightoffsetandsizetorevertin IR.How to verify it
Run
./quicktest.sh tests/parser/features/test_reverting.pyI also tested this in Foundry using VyperDeployer and verified that custom errors coming from solidity and the equivalent vyper
raw_revertarguments created the same error dataCommit message
Implement
raw_revertbuilt-in functionDescription for the changelog
Add
raw_revertCute Animal Picture