L-30: Make AccountERC7579 isModuleInstalled return false for the address zero module

[gonzaotc]

Fixes #????

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 719d42d1-722d-4c83-ad71-37935f8f9ea3

📥 Commits

Reviewing files that changed from the base of the PR and between fa7cb1b and b65f4c2.

📒 Files selected for processing (4)

• .changeset/fifty-owls-boil.md
• contracts/account/extensions/draft-AccountERC7579.sol
• contracts/account/extensions/draft-AccountERC7579Hooked.sol
• test/account/extensions/AccountERC7579.behavior.js

---

Walkthrough

This PR adds a patch release fix for the openzeppelin-solidity package addressing a bug in AccountERC7579. The changes ensure that isModuleInstalled returns false when the module address is zero for fallback and hook module types. The fix involves: adding an explicit non-zero address check in the fallback module validation logic, a similar check in the hooked variant, and new test coverage validating the zero-address behavior across all supported module types.

Possibly related PRs

• #5974: Modifies fallback-handling logic in draft-AccountERC7579.sol with length checks and decode error handling.
• #5961: Also updates isModuleInstalled function in the same contract file with related validation changes.

Suggested labels

bug, Account Abstraction

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: making isModuleInstalled return false for the zero address module in AccountERC7579.
Description check	✅ Passed	The description is related to the changeset, mentioning the issue reference placeholder and the PR checklist, though it lacks detailed explanation of the specific changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/L30-Account7579-isModuleInstalled-address0

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[changeset-bot]

🦋 Changeset detected

Latest commit: 5f17106

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
openzeppelin-solidity	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Amxx]

FV run: https://prover.certora.com/output/857050/9cd90fbfeb194dcb9a22e5c8ec763d11?anonymousKey=35adcc19bc4e9f4f944888878bfaeb2bf57d91fe

additonal coments

[Amxx]

I realise that this change could negativelly impact the observability of the account.

Right now it is possible check "are there any module installed for fallback selector X" or "is there a hook module installed". You can do so by asking "is module(0) installed". If this query returns true, you know nothing in installed, and installing something will not overwrite any existing config. If the query returns false, you know there is something installed, and that installing something else could break the config.

This could be usefull for apps/platforms that manage ERC-7579Accounts.

Without this behavior (return true on isModuleInstalled(moduleType, address(0), ...), then the only way to have this knowledge is by observing past events, which is significantly more complexe/expensive than doing a lookup.

There is a hook() getter that would work for the hook module, but it is not standard to ERC7579 so apps/platform might not look for it. There are no getters for the _fallbacks mapping.

For this reason, I think the current behavior should be preserved.