Skip to content

Commit

Permalink
Merge 3a03ad7 into 61cced5
Browse files Browse the repository at this point in the history
  • Loading branch information
@Aboudjem
Aboudjem authored Jun 2, 2024
2 parents 61cced5 + 3a03ad7 commit ccfddb7
Show file tree
Hide file tree
Showing 12 changed files with 296 additions and 880 deletions.
2 changes: 1 addition & 1 deletion .github/gas_report.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,4 +549,4 @@
"RECEIVER_ACCESS": "N/A",
"GAS_USED": 184796
}
]
]
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: Unified CI Workflow

on: [push, pull_request]
on: pull_request

permissions: write-all

Expand Down Expand Up @@ -171,4 +171,4 @@ jobs:
console.log("Slither report is empty. No comment will be posted.");
return;
}
await script({ github, context, header, body })
await script({ github, context, header, body })
61 changes: 59 additions & 2 deletions GAS_REPORT.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,60 @@
# Gas Report Comparison
| **Protocol** | **Actions / Function** | **Account Type** | **Is Deployed** | **With Paymaster?** | **Receiver Access** | **Gas Used** | **Gas Difference** |
|:------------:|:---------------------:|:----------------:|:--------------:|:-------------------:|:-------------------:|:------------:|:------------------:|

| **Protocol** | **Actions / Function** | **Account Type** | **Is Deployed** | **With Paymaster?** | **Receiver Access** | **Gas Used** | **Gas Difference** |
| :--------------------------------: | :------------------------------: | :--------------: | :-------------: | :-----------------: | :-----------------: | :----------: | :----------------: |
| ERC20 | transfer | EOA | False | False | 🧊 ColdAccess | 49921 | 0 |
| ERC20 | transfer | EOA | False | False | 🔥 WarmAccess | 25221 | 0 |
| ERC20 | transfer | Smart Account | True | False | 🧊 ColdAccess | 94767 | 0 |
| ERC20 | transfer | Smart Account | True | False | 🔥 WarmAccess | 74867 | 0 |
| ERC20 | transfer | Smart Account | False | True | 🧊 ColdAccess | 335883 | 0 |
| ERC20 | transfer | Smart Account | False | True | 🔥 WarmAccess | 315984 | 0 |
| ERC20 | transfer | Smart Account | False | False | 🧊 ColdAccess | 319073 | 0 |
| ERC20 | transfer | Smart Account | False | False | 🔥 WarmAccess | 299174 | 0 |
| ERC20 | transfer | Smart Account | False | False | 🧊 ColdAccess | 367178 | 0 |
| ERC20 | transfer | Smart Account | False | False | 🔥 WarmAccess | 347278 | 0 |
| ERC20 | transfer | Smart Account | True | True | 🧊 ColdAccess | 111262 | 0 |
| ERC20 | transfer | Smart Account | True | True | 🔥 WarmAccess | 91363 | 0 |
| ERC721 | transferFrom | EOA | False | False | 🧊 ColdAccess | 48483 | 0 |
| ERC721 | transferFrom | EOA | False | False | 🔥 WarmAccess | 28583 | 0 |
| ERC721 | transferFrom | Smart Account | True | False | 🧊 ColdAccess | 98254 | 0 |
| ERC721 | transferFrom | Smart Account | True | False | 🔥 WarmAccess | 78354 | 0 |
| ERC721 | transferFrom | Smart Account | False | True | 🧊 ColdAccess | 334585 | 0 |
| ERC721 | transferFrom | Smart Account | False | True | 🔥 WarmAccess | 314685 | 0 |
| ERC721 | transferFrom | Smart Account | False | False | 🧊 ColdAccess | 317777 | 0 |
| ERC721 | transferFrom | Smart Account | False | False | 🔥 WarmAccess | 297877 | 0 |
| ERC721 | transferFrom | Smart Account | False | False | 🧊 ColdAccess | 365881 | 0 |
| ERC721 | transferFrom | Smart Account | False | False | 🔥 WarmAccess | 345981 | 0 |
| ERC721 | transferFrom | Smart Account | True | True | 🧊 ColdAccess | 114777 | 0 |
| ERC721 | transferFrom | Smart Account | True | True | 🔥 WarmAccess | 94877 | 0 |
| ETH | transfer | EOA | False | False | 🧊 ColdAccess | 53073 | 0 |
| ETH | transfer | EOA | False | False | 🔥 WarmAccess | 28073 | 0 |
| ETH | call | EOA | False | False | 🧊 ColdAccess | 53201 | 0 |
| ETH | call | EOA | False | False | 🔥 WarmAccess | 28201 | 0 |
| ETH | send | EOA | False | False | 🧊 ColdAccess | 53201 | 0 |
| ETH | send | EOA | False | False | 🔥 WarmAccess | 28201 | 0 |
| ETH | transfer | Smart Account | True | False | 🧊 ColdAccess | 102616 | 0 |
| ETH | transfer | Smart Account | True | False | 🔥 WarmAccess | 77616 | 0 |
| ETH | transfer | Smart Account | False | True | 🧊 ColdAccess | 338898 | 0 |
| ETH | transfer | Smart Account | False | True | 🔥 WarmAccess | 313898 | 0 |
| ETH | transfer | Smart Account | False | False | 🧊 ColdAccess | 322110 | 0 |
| ETH | transfer | Smart Account | False | False | 🔥 WarmAccess | 297110 | 0 |
| ETH | transfer | Smart Account | False | False | 🧊 ColdAccess | 370215 | 0 |
| ETH | transfer | Smart Account | False | False | 🔥 WarmAccess | 345215 | 0 |
| ETH | transfer | Smart Account | True | True | 🧊 ColdAccess | 119101 | 0 |
| ETH | transfer | Smart Account | True | True | 🔥 WarmAccess | 94101 | 0 |
| UniswapV2 | swapExactETHForTokens | EOA | False | False | N/A | 149263 | 0 |
| UniswapV2 | swapExactETHForTokens | Smart Account | True | False | N/A | 199242 | 0 |
| UniswapV2 | swapExactETHForTokens | Smart Account | False | True | N/A | 435648 | 0 |
| UniswapV2 | swapExactETHForTokens | Smart Account | False | False | N/A | 418767 | 0 |
| UniswapV2 | swapExactETHForTokens | Smart Account | False | False | N/A | 466872 | 0 |
| UniswapV2 | swapExactETHForTokens | Smart Account | True | True | N/A | 215805 | 0 |
| UniswapV2 | swapExactTokensForTokens | EOA | False | False | N/A | 118252 | 0 |
| UniswapV2 | swapExactTokensForTokens | Smart Account | True | False | N/A | 168221 | 0 |
| UniswapV2 | swapExactTokensForTokens | Smart Account | False | True | N/A | 404616 | 0 |
| UniswapV2 | swapExactTokensForTokens | Smart Account | False | False | N/A | 387734 | 0 |
| UniswapV2 | approve+swapExactTokensForTokens | Smart Account | True | False | N/A | 200217 | 0 |
| UniswapV2 | approve+swapExactTokensForTokens | Smart Account | False | True | N/A | 436814 | 0 |
| UniswapV2 | approve+swapExactTokensForTokens | Smart Account | False | False | N/A | 419743 | 0 |
| UniswapV2 | approve+swapExactTokensForTokens | Smart Account | False | False | N/A | 467849 | 0 |
| UniswapV2 | swapExactTokensForTokens | Smart Account | True | True | N/A | 184796 | 0 |
| No differences found in gas usage. |
28 changes: 27 additions & 1 deletion contracts/base/ModuleManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,37 @@ contract ModuleManager is Storage, Receiver, IModuleManagerEventsAndErrors {
/// @param handler The address of the fallback handler to install.
/// @param params The initialization parameters including the selector and call type.
function _installFallbackHandler(address handler, bytes calldata params) internal virtual {
// Extract the function selector from the provided parameters.
bytes4 selector = bytes4(params[0:4]);

// Extract the call type from the provided parameters.
CallType calltype = CallType.wrap(bytes1(params[4]));

// Extract the initialization data from the provided parameters.
bytes memory initData = params[5:];
if (_isFallbackHandlerInstalled(selector)) revert FallbackAlreadyInstalledForSelector(selector);

// Revert if the selector is either `onInstall(bytes)` (0x6d61fe70) or `onUninstall(bytes)` (0x8a91b0e3).
// These selectors are explicitly forbidden to prevent security vulnerabilities.
// Allowing these selectors would enable unauthorized users to uninstall and reinstall critical modules.
// If a validator module is uninstalled and reinstalled without proper authorization, it can compromise
// the account's security and integrity. By restricting these selectors, we ensure that the fallback handler
// cannot be manipulated to disrupt the expected behavior and security of the account.
if (selector == bytes4(0x6d61fe70) || selector == bytes4(0x8a91b0e3)) {
revert FallbackSelectorForbidden();
}

// Revert if a fallback handler is already installed for the given selector.
// This check ensures that we do not overwrite an existing fallback handler, which could lead to unexpected behavior.
if (_isFallbackHandlerInstalled(selector)) {
revert FallbackAlreadyInstalledForSelector(selector);
}

// Store the fallback handler and its call type in the account storage.
// This maps the function selector to the specified fallback handler and call type.
_getAccountStorage().fallbacks[selector] = FallbackHandler(handler, calltype);

// Invoke the `onInstall` function of the fallback handler with the provided initialization data.
// This step allows the fallback handler to perform any necessary setup or initialization.
IFallback(handler).onInstall(initData);
}

Expand Down
3 changes: 3 additions & 0 deletions contracts/interfaces/base/IModuleManagerEventsAndErrors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,7 @@ interface IModuleManagerEventsAndErrors {

/// @dev Thrown when no fallback handler is available for a given selector.
error MissingFallbackHandler(bytes4 selector);

/// @dev Thrown when there is an attempt to install a forbidden selector as a fallback handler.
error FallbackSelectorForbidden();
}
Loading

0 comments on commit ccfddb7

Please sign in to comment.