Fix hooking the `fallback()`

[filmakarov]

Fix the audit issue #58 where fallback was using assembly return so the hook.postCheck was never used.

Proposed solution as per recommendations is the following:

• Do not use withHook modifier, instead do preCheck and postCheck manually in the fallback() function
• Do not use assembly returns.
• The calldata for call is appended with msg.sender address so fallback module has access to the original sender, this is done with lib function

The logic in the fallback now is the following: 1) do preCheck, store result in memory 2) if there's a fallback handler, call it. if not successful, revert. 3) else check if some of onERCXXXReceived methods were called. if this is neither the case, revert 4) do postCheck 5) return result

[github-actions[bot]]

Changes to gas cost

Generated at commit: 199d84e7dd82a7913558806e955c6068f2ba0573, compared to commit: c81e30ca56de428364382ab71f16c086fc0fa701

🧾 Summary (5% most significant diffs)

Contract	Method	Avg (+/-)	%
Nexus	execute executeFromExecutor	-127 ✅ -169 ✅	-0.27% -0.83%

---

Full diff report 👇

| Contract | Deployment Cost (+/-) | Method | Min (+/-) | % | Avg (+/-) | % | Median (+/-) | % | Max (+/-) | % | # Calls (+/-) | |:-|-:|:-|-:|-:|-:|-:|-:|-:|-:|-:|-:| | **Nexus** | 4,780,735 (+106,245) | _execute_
_executeFromExecutor_
_installModule_
_uninstallModule_ | 6,308 (-118)
14,764 (-115)
32,512 (0)
5,868 (0) | **-1.84%**
**-0.77%**
**0.00%**
**0.00%** | 46,436 (-127)
20,310 (-169)
38,223 (-14)
10,614 (+28) | **-0.27%**
**-0.83%**
**-0.04%**
**+0.26%** | 38,008 (-118)
20,238 (-169)
39,873 (0)
12,465 (+42) | **-0.31%**
**-0.83%**
**0.00%**
**+0.34%** | 143,086 (-118)
26,001 (-224)
44,191 (0)
12,466 (+42) | **-0.08%**
**-0.85%**
**0.00%**
**+0.34%** | 73 (0)
4 (0)
23 (0)
6 (0) | | **Bootstrap** | 2,024,500 (+94,083) | | | | | | | | | | |

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 82.85714% with 6 lines in your changes missing coverage. Please review.

Project coverage is 86.40%. Comparing base (c81e30c) to head (27b8627). Report is 8 commits behind head on dev.

Files with missing lines	Patch %	Lines
contracts/base/ModuleManager.sol	82.85%	6 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## dev #183 +/- ## ========================================== - Coverage 88.70% 86.40% -2.30% ========================================== Files 14 14 Lines 903 905 +2 Branches 263 266 +3 ========================================== - Hits 801 782 -19 - Misses 102 109 +7 - Partials 0 14 +14 ``` | [Files with missing lines](https://app.codecov.io/gh/bcnmy/nexus/pull/183?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcnmy) | Coverage Δ | | |---|---|---| | [contracts/base/ModuleManager.sol](https://app.codecov.io/gh/bcnmy/nexus/pull/183?src=pr&el=tree&filepath=contracts%2Fbase%2FModuleManager.sol&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcnmy#diff-Y29udHJhY3RzL2Jhc2UvTW9kdWxlTWFuYWdlci5zb2w=) | `86.75% <82.85%> (-2.05%)` | :arrow_down: | ... and [5 files with indirect coverage changes](https://app.codecov.io/gh/bcnmy/nexus/pull/183/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcnmy) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/bcnmy/nexus/pull/183?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcnmy). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcnmy) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/bcnmy/nexus/pull/183?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcnmy). Last update [c81e30c...27b8627](https://app.codecov.io/gh/bcnmy/nexus/pull/183?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcnmy). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcnmy).

[github-actions[bot]]

:robot: Slither Analysis Report :mag_right:

Slither report

# Slither report **THIS CHECKLIST IS NOT COMPLETE**. Use `--show-ignored-findings` to show all the results. Summary :yellow_circle: - [locked-ether](#locked-ether) (1 results) (Medium) :yellow_circle: - [unused-return](#unused-return) (1 results) (Medium) - [constable-states](#constable-states) (2 results) (Optimization) ## locked-ether :yellow_circle: Impact: Medium :red_circle: Confidence: High - [ ] ID-0 Contract locking ether found: Contract [Bootstrap]([utils/RegistryBootstrap.sol#L33-L165](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/utils/RegistryBootstrap.sol#L33-L165)) has payable functions: - [ModuleManager.receive()]([base/ModuleManager.sol#L65](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/base/ModuleManager.sol#L65)) - [ModuleManager.fallback(bytes)]([base/ModuleManager.sol#L68-L70](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/base/ModuleManager.sol#L68-L70)) But does not have a function to withdraw the ether [utils/RegistryBootstrap.sol#L33-L165](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/utils/RegistryBootstrap.sol#L33-L165) ## unused-return :yellow_circle: Impact: Medium :yellow_circle: Confidence: Medium - [ ] ID-1 [ERC7739Validator._hashTypedDataForAccount(address,bytes32)]([base/ERC7739Validator.sol#L279-L309](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/base/ERC7739Validator.sol#L279-L309)) ignores return value by [(name,version,chainId,verifyingContract) = EIP712(account).eip712Domain()]([base/ERC7739Validator.sol#L280-L288](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/base/ERC7739Validator.sol#L280-L288)) [base/ERC7739Validator.sol#L279-L309](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/base/ERC7739Validator.sol#L279-L309) ## constable-states Impact: Optimization :red_circle: Confidence: High - [ ] ID-2 [RegistryAdapter.registry]([base/RegistryAdapter.sol#L10](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/base/RegistryAdapter.sol#L10)) should be constant [base/RegistryAdapter.sol#L10](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/base/RegistryAdapter.sol#L10) - [ ] ID-3 [RegistryFactory.threshold]([factory/RegistryFactory.sol#L39](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/factory/RegistryFactory.sol#L39)) should be constant [factory/RegistryFactory.sol#L39](https://github.com/bcnmy/nexus/blob/199d84e7dd82a7913558806e955c6068f2ba0573/contracts/contracts/factory/RegistryFactory.sol#L39)

This comment was automatically generated by the GitHub Actions workflow.