Feat/sma 809 potential account factories

[livingrockrises]

couple of things may be reverted from this change. first, here is the list of changes below and then I would dive into discussion and link the PR for further experimental changes.

1. Stakeable

Added common/Stakeable contract (which is Ownable) that can be inherited by the factory [ Or any entity which needs a stake on the entry point for that matter ]

2. AccountFactory minor changes

base StakeManager was wrong.
now Factory inherits from Stakeable which makes it also Owned. Using this owner we could further add access controlled actions like whitelisting one or more modules, Or setting ERC7484 registry which checks on module being installed

also it felt right to rename getCounterFactualAddress -> computeAccountAddress :))

3.small) Took a snapshot of current AccountFactory in case we start editing this one in current branch
that would mean altering createAccount method from
function createAccount(address validationModule, bytes calldata moduleInstallData, uint256 index)
to something
which can
i. accept generic initData to be directly posted on the account (account.call(initData)) / initData to call specific method on the account INexus.initializeAccount(initData)

ii. accepts array of modules and setup datas, or packed data and factory knows how to decode those and check agaisnt some whitelist.

4. BiconomyMetaFactory. inspired from kernel's factory staker
Stakeable Ownable Factory, idea is to use this as meta factory and it's address will be part of the initcode and under the hood you can use any approved (only approved ones) factory using deployUsingFactory() method.
clear benefit is you can gatekeep factories
if the approved/whitelisted factory has generic call data then issue reported by holterhaus still remains. perhaps these factories can have initial validator/few modules enshrined or whitelisted
in the future you can just add a new factory under the hood and start using it.

con might be added gas but that comes with any kind of whitelisting anywhere or when addressing security concerns.

5. Bootstrap.sol
singleton bootstrap contract inspired from ERC7579 reference implementation
this contract has util methods to prepare the module installation calldata

it would work like this in Nexus initialisation

function initializeAccount(bytes calldata initData) external payable virtual {
        // checks if already initialized and reverts before setting the state to initialized
        _initModuleManager();

        // this is just implemented for demonstration purposes. You can use any other initialization
        // logic here.
        (address bootstrap, bytes memory bootstrapCall) = abi.decode(initData, (address, bytes));
        (bool success,) = bootstrap.delegatecall(bootstrapCall);
        if (!success) revert();
 }

using Bootstrap util contract one can make bootstrap config using array of modules to be installed upon.
implementation is not altered in this branch to use this bootstrap contract

but experimental changes are here with evolved Bootstrap contract
https://github.com/bcnmy/nexus/blob/test/showcase-alterations-using-bootstrap/contracts/utils/Bootstrap.sol#L82

P.S - This does not help resolve Holterhaus reported issue but only to allow installing multiple modules. If we go with this approach I haven't thought about how to decode bootstrap config and make checks for individual module addresses or payloads.

[linear]

SMA-809 Nexus Account Factory (finalise and update)

[openzeppelin-code]

Feat/sma 809 potential account factory (Draft)

Generated at commit: b1232a378882c7592e144a7f5f9b53cb4109232c

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	0 1 0 9 29 39
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

---

For more details view the full report in OpenZeppelin Code Inspector

[livingrockrises]

also linking this here
#74

[Aboudjem]

typo

[Aboudjem]

initilazed

[Aboudjem]

typo

[livingrockrises]

which typo?

[Aboudjem]

valdiators

[Aboudjem]

.

[filmakarov]

I agree to all the proposed changes.
Ownable can help us managing stuff that will allow to mitigate HolterHus issue
having generic bytes data can also be useful. enabling several modules would be really useful

Question re MetaFactory

it's address will be part of the initcode
can you elaborate on this please?

[Aboudjem]

Got some thoughts on the recent PR. I think we can simplify things and make it more efficient

1. Move the init logic into the NexusAccount directly this way we can get rid of Bootstrap.sol and BootstrapUtil.sol
2. enshrine whitelist of modules in the factory itself or maybe in the Meta (I'm not sure where will it be used here yet as it's used nowhere for the moment)
3. simplifying the initData structure by encoding everything within the NexusAccount reduces the need for decoding and number of external calls

this will reduce gas with less external calls, simplify initData structure and reduce the number of contracts (files)

User
 |
 | Initiates account creation
 v
+----------------------+
| NexusMetaFactory     |
| - Verify Factory     |
+----------------------+
           |
           | deployWithFactory(factory, factoryData)
           v
+----------------------+
| Specific Factory     |
| - Verify Modules     |
| - Initialize Account |
+----------------------+
           |
           | createAccount(initData, salt)
           v
+----------------------+
| NexusAccount         |
| - Initialize Modules |
| - Configure Account  |
+----------------------+
           |
           | Initialization Complete
           v
         Account Created

Lmk what your thoughts

[livingrockrises]

There are some review comments I am keeping and leaving a note here.

NexusAccountFactory
// Review may not need stakeable here

ModuleWhitelistFactory
// Review if we should verify calldata[0:4] against the function selector of initNexus

K1ValidatorFactory
// Review: if salt should include K1 Validator address as well

[github-actions]

🤖 Slither Analysis Report 🔎

Slither report

# Slither report

THIS CHECKLIST IS NOT COMPLETE. Use --show-ignored-findings to show all the results.
Summary
🔴 - controlled-delegatecall (1 results) (High)

controlled-delegatecall

🔴 Impact: High
🟡 Confidence: Medium

• ID-0
  Nexus.initializeAccount(bytes) uses delegatecall to a input-controlled function id
  • (success) = bootstrap.delegatecall(bootstrapCall)

Nexus.sol#L213-L219

_This comment was automatically generated by the GitHub Actions workflow._