-
Notifications
You must be signed in to change notification settings - Fork 80
Conversation
Instead of using an intent struct, we use a efficient custom encoding instead. This reduces gas by ~12k for single intents and ~10k for intents in a batch
Gas savings come from:
- Smaller intent size (by ~5-10 words). This results in less memory usage, less memory expansion, and less intrinsic calldata cost paid
- Removing custom offset checks
LibBytes.checkInCalldata(~2-3k gas alone) - Not requiring just-in-time checks on struct offsets when accessing any struct field (solc adds this under the hood)
- Optimizing the args for
_payto only take in what we currently use today instead of the full intent - Removing fields from the EIP-712 digest - 2 less
keccakoperations
The new format is abi.encodePacked, but with some nuances:
- All static fields go before all dynamic fields, so all static fields have known offsets
- The static fields that go into the EIP712 digest are ordered continuously
- The static fields that are in the EIP712 digest are padded to 32 bytes, then with the previous property this allows us to do a single
calldatacopyfor digest calculations
(The relayer has the freedom to use dirty bytes on these fields, but then the digest would compute to something else and this would fail verification, so there's no incentive to do so) - Dynamic fields are prefixed with
uint256 field.lengthto allow us to use them as abytes calldatadirectly - We order the dynamic fields in the order that they will be used in the orchestrator flow
We rely on the helper function _getNextBytes(ptr) for efficient traversal through the dynamic fields. The helper function assumes that the pointer already points to the start of a dynamic byte range. It parses that out, then updates the CalldataPointer in memory to point to the start of the next dynamic byte range
b891f19 to
6945da1
Compare
|
Bytecode changes detected! EIP-712 domain versions have been automatically updated for: IthacaAccount,Orchestrator,SimpleFunder,Simulator |
bbb01eb to
5a155c3
Compare
|
Bytecode changes detected! EIP-712 domain versions have been automatically updated for: IthacaAccount,Orchestrator,SimpleFunder,Simulator |
a3e28e6 to
db35ed5
Compare
|
Bytecode changes detected! EIP-712 domain versions have been automatically updated for: IthacaAccount,Orchestrator,SimpleFunder,Simulator |
a69256f to
bde24a4
Compare
|
Bytecode changes detected! EIP-712 domain versions have been automatically updated for: IthacaAccount,Orchestrator,SimpleFunder,Simulator |
| function simulateExecute(bytes calldata encodedIntent) external payable returns (uint256) { | ||
| bool isStateOverride; | ||
| uint256 combinedGasOverride; | ||
| assembly ("memory-safe") { |
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.
note to reviewer - we need to do this b/c adding another arg changes the calldata layout, making encodedIntent have an offset that is not 0x40
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.
Think this can be solved by just seeding all functions of the IntentHelpers lib with the starting offset of the encodedIntent?
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.
Yup, we'd have to bubble that into the internal function. I think this is the best approach for us to to support batch executions without a separate call frame though, great idea
b215dee to
135ef20
Compare
| i.paymentRecipient | ||
| ); | ||
|
|
||
| { |
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.
a little ugly, but we're running into stack too deep if we don't do this
| u.signature = _sig(alice, u); | ||
|
|
||
| assertEq(oc.execute(abi.encode(u)), bytes4(keccak256("PaymentError()"))); | ||
| assertEq(oc.execute(encodeIntent(u)), bytes4(keccak256("Unauthorized()"))); |
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.
found it funny that a testUnauthorized was erroring with PaymentError
|
Bytecode changes detected! EIP-712 domain versions have been automatically updated for: IthacaAccount,Simulator |
|
Bytecode changes detected! EIP-712 domain versions have been automatically updated for: IthacaAccount,SimpleFunder,Simulator |
|
Bytecode changes detected! EIP-712 domain versions have been automatically updated for: IthacaAccount,Orchestrator,SimpleFunder,Simulator |
| // This generates an unnecessary check for `i < encodedIntents.length`, but helps | ||
| // generate all the implicit calldata bound checks on `encodedIntents[i]`. | ||
| (, errs[i]) = _execute(encodedIntents[i], 0, _NORMAL_MODE_FLAG); | ||
| errs[i] = this.execute(encodedIntents[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.
Why add self calls to batching?
Can't we update the IntentHelpers lib, to start at a particular calldata offset instead?
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.
yup, its possible, this approach is less efficient
right now its 10k cheaper in a batch and 12k in a single intent, i think that 2k difference is coming from this
let's include that in a separate PR, it might be a medium-sized effort if we're going to go through with some of the other planned changes
| function simulateExecute(bytes calldata encodedIntent) external payable returns (uint256) { | ||
| bool isStateOverride; | ||
| uint256 combinedGasOverride; | ||
| assembly ("memory-safe") { |
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.
Think this can be solved by just seeding all functions of the IntentHelpers lib with the starting offset of the encodedIntent?
78b2398 to
bac1c67
Compare
2684f1d to
b5432dc
Compare
* chore: readd merkle verification prefix
* chore: undo blank line addns
* chore: lint
* .
* ~50 failing tests down to 5
* down to 1 failing test
* fixed failing test
* chore: remove console logs and bench
* .
* Update test/Base.t.sol
* Update src/Orchestrator.sol
* Update test/utils/mocks/MockPayerWithSignatureOptimized.sol
* chore: final cleanup, rebench
* Update src/Orchestrator.sol
* chore: bump contract versions due to bytecode changes - Contracts updated: IthacaAccount,Orchestrator,SimpleFunder,Simulator
* chore: cleanup
* rebase
* fix
---------
Co-authored-by: GitHub Action
* chore: readd merkle verification prefix
* chore: undo blank line addns
* chore: lint
* .
* ~50 failing tests down to 5
* down to 1 failing test
* fixed failing test
* chore: remove console logs and bench
* .
* Update test/Base.t.sol
* Update src/Orchestrator.sol
* Update test/utils/mocks/MockPayerWithSignatureOptimized.sol
* chore: final cleanup, rebench
* Update src/Orchestrator.sol
* chore: bump contract versions due to bytecode changes - Contracts updated: IthacaAccount,Orchestrator,SimpleFunder,Simulator
* chore: cleanup
* rebase
* fix
---------
Co-authored-by: GitHub Action
* feat: simplify multichain nonce design
* chore: readd merkle verification prefix
* chore: undo blank line addns
* chore: lint
* .
* ~50 failing tests down to 5
* down to 1 failing test
* fixed failing test
* chore: remove console logs and bench
* .
* Update test/Base.t.sol
* Update src/Orchestrator.sol
* Update test/utils/mocks/MockPayerWithSignatureOptimized.sol
* chore: final cleanup, rebench
* Update src/Orchestrator.sol
* chore: bump contract versions due to bytecode changes - Contracts updated: IthacaAccount,Orchestrator,SimpleFunder,Simulator
* chore: cleanup
* rebase
* fix
---------
Co-authored-by: GitHub Action
* chore: update benchmarks
* chore: update to correct app sponsor cost
* chore: rename benchmarks for clarity
* chore: readme
* cache
* revert
* .
* chore: lint
* chore: lint, use diff tokens for erc20 transfers
* .
* chore: add native transfer and uni v2 swap benchmarks, standardize to use different recipients/tokens for benchmarks
* chore: lint
* feat: add 7702 account benchmarks
* chore: update readme
---------
Co-authored-by: GitHub Action
* chore: readd merkle verification prefix
* chore: undo blank line addns
* chore: lint
* .
* ~50 failing tests down to 5
* down to 1 failing test
* fixed failing test
* chore: remove console logs and bench
* .
* Update test/Base.t.sol
* Update src/Orchestrator.sol
* Update test/utils/mocks/MockPayerWithSignatureOptimized.sol
* chore: final cleanup, rebench
* Update src/Orchestrator.sol
* chore: bump contract versions due to bytecode changes - Contracts updated: IthacaAccount,Orchestrator,SimpleFunder,Simulator
* chore: cleanup
* rebase
* fix
---------
Co-authored-by: GitHub Action