BT-950💥 added Account Abstraction for Identity contract #131
Conversation
| function getNonce() public view virtual returns (uint256) { | ||
| return entryPoint().getNonce(address(this), 0); | ||
| } | ||
|
|
There was a problem hiding this comment.
you’re only using nonce key 0 everywhere ERC4337 supports multiple nonce “keys” so users can send few txs in parallel
right now they can send only a tx at once
There was a problem hiding this comment.
need to read the key from upper bits of the nonce instead of hardcoding 0
otherwise only one userOp at a time will work
| * @param userOpHash The hash of the UserOperation | ||
| * @return validationData Packed validation data (0 for success, 1 for signature/permission failure) | ||
| */ | ||
| function _validateSignature( |
There was a problem hiding this comment.
your signature validation just returns 0 ro 1
in 4337 you can also return validAfter and validUntil (time window for sigs)
useful for expiring or delayed ops
not a blocker but nice to have if you plan to support smart scheduling
| if (msg.sender == address(entryPoint())) { | ||
| // For entry point calls, use direct execution | ||
| _executeDirect(_to, _value, _data); | ||
| return 0; // Return 0 for entry point calls | ||
| } | ||
|
|
||
| // For regular calls, use KeyManager's execution logic | ||
| KeyStorage storage ks = _getKeyStorage(); | ||
| executionId = ks.executionNonce; | ||
| ks.executions[executionId].to = _to; | ||
| ks.executions[executionId].value = _value; | ||
| ks.executions[executionId].data = _data; | ||
| ks.executionNonce++; | ||
|
|
||
| emit ExecutionRequested(executionId, _to, _value, _data); |
There was a problem hiding this comment.
execute() does two different things
if called by EntryPoint runs directly adn if called by anyone else it goes through 734 flow
I think is kinda messy can be confusing maybe separate functions or force EntryPoint to use executeBatch
| function validateUserOp( | ||
| PackedUserOperation calldata userOp, | ||
| bytes32 userOpHash, | ||
| uint256 missingAccountFunds | ||
| ) | ||
| external | ||
| virtual | ||
| override(IAccount) | ||
| onlyEntryPoint | ||
| returns (uint256 validationData) | ||
| { | ||
| // 1. Validate signature and permissions in one step | ||
| validationData = _validateSignature(userOp, userOpHash); | ||
| if (validationData != SIG_VALIDATION_SUCCESS) { | ||
| return validationData; | ||
| } | ||
|
|
||
| // 2. Validate nonce | ||
| _validateNonce(userOp.nonce); | ||
|
|
||
| // 3. Handle missing funds | ||
| _payPrefund(missingAccountFunds); | ||
|
|
||
| return SIG_VALIDATION_SUCCESS; | ||
| } |
There was a problem hiding this comment.
validateUserOp ignores paymasterAndData
means literally any paymaster can “sponsor” a tx
that’s risky untrusted paymaster could spam failed ops or other issues
| /** | ||
| * @dev Hardcoded Entry Point addresses per network | ||
| * Official ERC-4337 Entry Point v0.6: 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789 | ||
| */ | ||
| IEntryPoint internal constant _DEFAULT_ENTRY_POINT = | ||
| IEntryPoint(0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789); |
| function validateUserOp( | ||
| PackedUserOperation calldata userOp, | ||
| bytes32 userOpHash, | ||
| uint256 missingAccountFunds | ||
| ) | ||
| external | ||
| virtual | ||
| override(IAccount) | ||
| onlyEntryPoint | ||
| returns (uint256 validationData) | ||
| { | ||
| // 1. Validate signature and permissions in one step | ||
| validationData = _validateSignature(userOp, userOpHash); | ||
| if (validationData != SIG_VALIDATION_SUCCESS) { | ||
| return validationData; | ||
| } | ||
|
|
||
| // 2. Validate nonce | ||
| _validateNonce(userOp.nonce); | ||
|
|
||
| // 3. Handle missing funds | ||
| _payPrefund(missingAccountFunds); | ||
|
|
||
| return SIG_VALIDATION_SUCCESS; | ||
| } |
There was a problem hiding this comment.
no sanity checks on userOp gas fields
too low could reverts, too high waste or DOS risk
add basic checks like “verificationGas > 100k” before approving the op
There was a problem hiding this comment.
all ops are validated individually 4337 also allows aggregated sigs for cheaper batching
optional, but worth noting if you plan multiop bundling later (more optizmied)
| function _payPrefund(uint256 missingAccountFunds) internal virtual { | ||
| if (missingAccountFunds > 0) { | ||
| entryPoint().depositTo{ value: missingAccountFunds }(address(this)); | ||
| } | ||
| } |
There was a problem hiding this comment.
if the account doesnt have enough ETH it just reverts inside EntryPoint which is hard to debug (believe me)
add a quick balance check before depositing and maybe skip if deposit already covers thsi makes flow safer and errors clearer
No description provided.