Smart Account contracts: audit fixes #396
Conversation
| // \____/ \__| \__|\__|\__| \_______| \_____\____/ \_______|\_______/ | ||
|
|
||
| contract AccountExtension is ContractMetadata, PermissionsEnumerable, ERC721Holder, ERC1155Holder { | ||
| contract AccountExtension is ContractMetadata, AccountPermissions, ERC721Holder, ERC1155Holder { |
There was a problem hiding this comment.
how come both AccountCore and AccountExtension both inherit AccountPermissions ? would have expected it only in the core
There was a problem hiding this comment.
Both require the functionality of AccountPermissions since both contracts have logic that works with permission checks.
The AccountPermissions contract uses the library storage layout, so even though the two seperate contracts inherit it, the same storage slots are read/written to in a dynamic contract using both AccountCore + AccountExtension.
| require(data.approvedTargets[role].contains(targets[i]), "Account: target not approved."); | ||
| require(restrictions.maxValuePerTransaction >= values[i], "Account: value too high."); | ||
| } | ||
| } |
There was a problem hiding this comment.
shouldn't we revert if the function called is neither execute/executeBatch here?
| mstore(ptr, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73) | ||
| mstore(add(ptr, 0x58), salt) | ||
| mstore(add(ptr, 0x78), keccak256(add(ptr, 0x0c), 0x37)) | ||
| predicted := keccak256(add(ptr, 0x43), 0x55) |
There was a problem hiding this comment.
need to test this accross the board make sure there's no unexpected behavior on the SDK
There was a problem hiding this comment.
This comes from the latest OpenZeppelin 4.8.0 repo: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/proxy/Clones.sol
2 participants
No description provided.