Sign All Gas Parameters in ERC-4337 0.7 PackedUserOperation
#250
Assignees
Comments
nlordell
added a commit
that referenced
this issue
Feb 13, 2024
Fixes #250 This PR changes the `SafeOperation` type to include ALL gas parameters for signing, instead of packing them. This ensures the maximum amount of data is available to the user when signing user operations, instead of trusting gas parameters from a Dapp. Note a subtle change in the order of the "fee per gas" fields, this was done to match the order in the `PackedUserOperation` struct, as well as the order in EIP-1559 transactions. I figured, since we are changing the signing format anyway (notice the different integer types), this wouldn't be a big deal. Additionally, we have a new test that verifies that `safeOp` hashes are computed off-chain in the same way they are computed on-chain. This is to catch issues with field orders being slightly different in the `EncodedSafeOp` struct that we would otherwise miss. Notably, the "fee per gas" parameters are always set to the same value, and our existing test suite was not able to catch swapped order in `EncodedSafeOp` fields.
nlordell
added a commit
that referenced
this issue
Feb 23, 2024
Fixes #250 This PR changes the `SafeOperation` type to include ALL gas parameters for signing, instead of packing them. This ensures the maximum amount of data is available to the user when signing user operations, instead of trusting gas parameters from a Dapp. Note a subtle change in the order of the "fee per gas" fields, this was done to match the order in the `PackedUserOperation` struct, as well as the order in EIP-1559 transactions. I figured, since we are changing the signing format anyway (notice the different integer types), this wouldn't be a big deal. Additionally, we have a new test that verifies that `safeOp` hashes are computed off-chain in the same way they are computed on-chain. This is to catch issues with field orders being slightly different in the `EncodedSafeOp` struct that we would otherwise miss. Notably, the "fee per gas" parameters are always set to the same value, and our existing test suite was not able to catch swapped order in `EncodedSafeOp` fields.
nlordell
added a commit
that referenced
this issue
Mar 5, 2024
Fixes #250 This PR changes the `SafeOperation` type to include ALL gas parameters for signing, instead of packing them. This ensures the maximum amount of data is available to the user when signing user operations, instead of trusting gas parameters from a Dapp. Note a subtle change in the order of the "fee per gas" fields, this was done to match the order in the `PackedUserOperation` struct, as well as the order in EIP-1559 transactions. I figured, since we are changing the signing format anyway (notice the different integer types), this wouldn't be a big deal. Additionally, we have a new test that verifies that `safeOp` hashes are computed off-chain in the same way they are computed on-chain. This is to catch issues with field orders being slightly different in the `EncodedSafeOp` struct that we would otherwise miss. Notably, the "fee per gas" parameters are always set to the same value, and our existing test suite was not able to catch swapped order in `EncodedSafeOp` fields.
nlordell
added a commit
that referenced
this issue
Mar 5, 2024
Fixes #250 This PR changes the `SafeOperation` type to include ALL gas parameters for signing, instead of packing them. This ensures the maximum amount of data is available to the user when signing user operations, instead of trusting gas parameters from a Dapp. Note a subtle change in the order of the "fee per gas" fields, this was done to match the order in the `PackedUserOperation` struct, as well as the order in EIP-1559 transactions. I figured, since we are changing the signing format anyway (notice the different integer types), this wouldn't be a big deal. Additionally, we have a new test that verifies that `safeOp` hashes are computed off-chain in the same way they are computed on-chain. This is to catch issues with field orders being slightly different in the `EncodedSafeOp` struct that we would otherwise miss. Notably, the "fee per gas" parameters are always set to the same value, and our existing test suite was not able to catch swapped order in `EncodedSafeOp` fields.
nlordell
added a commit
that referenced
this issue
Mar 5, 2024
Fixes #250 This PR changes the `SafeOperation` type to include ALL gas parameters for signing, instead of packing them. This ensures the maximum amount of data is available to the user when signing user operations, instead of trusting gas parameters from a Dapp. Note a subtle change in the order of the "fee per gas" fields, this was done to match the order in the `PackedUserOperation` struct, as well as the order in EIP-1559 transactions. I figured, since we are changing the signing format anyway (notice the different integer types), this wouldn't be a big deal. Additionally, we have a new test that verifies that `safeOp` hashes are computed off-chain in the same way they are computed on-chain. This is to catch issues with field orders being slightly different in the `EncodedSafeOp` struct that we would otherwise miss. Notably, the "fee per gas" parameters are always set to the same value, and our existing test suite was not able to catch swapped order in `EncodedSafeOp` fields.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Context / issue
The new ERC-4337 EntryPoint v0.7.0 has a new
PackedUserOperationformat. Initial support for this user operation was added in #243, however we sign the fully packed gas parameters instead of each individual gas parameter seperately, making it less clear to the use exactly what is being signed.Proposed solution
Unpack the gas parameter data and include that in the EIP-712
SafeUserOperationstruct that gets signed.Alternatives
Save on gas and keep signing the packed gas parameters, at the cost of clearly understanding what is being signed. There may also be Solidity "stack-too-deep" errors which may be hard to work around (although it should not be an issue with how we compute the hashes in the latest code).
The text was updated successfully, but these errors were encountered: