-
Notifications
You must be signed in to change notification settings - Fork 66
feat: add common address execution mode support to ERC7821 #341
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
base: main
Are you sure you want to change the base?
Conversation
to execution mode support to ERC7821
to execution mode support to ERC7821 to execution mode support to ERC7821
to execution mode support to ERC7821 | } | ||
| bool isMultichain = nonce >> 240 == MULTICHAIN_NONCE_PREFIX; | ||
| bytes32 structHash = EfficientHashLib.hash( | ||
| uint256(EXECUTE_TYPEHASH), LibBit.toUint(isMultichain), uint256(a.hash()), nonce |
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.
isMultichain is redundant since we already get it from domain
| } | ||
|
|
||
| bytes32[] memory a = EfficientHashLib.malloc(calls.length); | ||
| for (uint256 i; i < calls.length; ++i) { |
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.
This means that someone can take a valid CallSansTo execution bundle and submit with the Call mode
Would prefer if we hash this a little differently to prevent that, its good design to only allow 1 possible way things can be executed with the same sig
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.
Yeah good point, but does it matter if they are used interchangeably?
the nonce already does replay protection.
But yes maybe we should hash it separately, just to prevent footguns in the future
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.
nothing obvious, but yep best to not have hanging things like this, there are attacks that could happen when you combine a bunch of hanging things
|
|
||
| d.d.execute(_ERC7821_BATCH_SANS_TO_EXECUTION_MODE, t.executionData); | ||
|
|
||
| assertEq(targetFunctionPayloads.length, t.n); |
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.
redundant test
|
its not clear to me how this is flowing thru guarded executor, i might be missing something but either way lets add a spend limit test |
|
Wait, let me marinate first. Vectorized/solady#1485 (comment) Imo, a native calldata compression like that in |
Add CallSansTo batch execution support
Implements a new batch execution mode for calls that share a common target address,
optimizing gas usage and simplifying transaction construction.
Changes
Core Implementation:
CallSansTostruct withvalueanddatafields (no individualtoaddress)
0x0100000000007821000300000000000000000000000000000000000000000000_executeBatchCommonTo()function in ERC7821Ithaca that extracts sharedtoaddress
computeDigest()overload for CallSansTo arrays with shared targettoaddress passed to the function is set to address(0), it is replaced with address(this) onchain.The new mode allows batching multiple calls to the same contract more efficiently
by specifying the target address once rather than per-call, reducing calldata size
and gas costs while maintaining full security guarantees.