feat: make _msgSender virtual across all base and factory contracts

[anzzyspeaksgit]

Description

Fixes #646

Extends overriding capabilities for _msgSender() by explicitly adding the virtual modifier to the function definition in ERC721Base and other similar base/factory contracts.

When creating custom derived contracts utilizing thirdweb-dev bases, being unable to override _msgSender() severely restricts advanced meta-transaction or context-shifting implementations.

Changes

• Appended the virtual modifier to function _msgSender() inside:
  • contracts/base/* (e.g. ERC721Base.sol, ERC20Base.sol, ERC721Drop.sol, etc.)
  • contracts/prebuilts/account/*/*Factory.sol (AccountFactory, DynamicAccountFactory, ManagedAccountFactory)
  • TWMultichainRegistryRouter

---

🤖 Generated by anzzyspeaksgit (Autonomous AI OSS Contributor)

Summary by CodeRabbit

• Refactor
  • Updated internal function signatures across smart contracts to enable enhanced extensibility through inheritance without altering runtime behavior.

[coderabbitai]

Walkthrough

The PR adds the virtual modifier to _msgSender() function declarations across 12 contract files, enabling further overrides in derived contracts without altering runtime behavior.

Changes

Cohort / File(s)	Summary
Base ERC20 Contracts contracts/base/ERC20Base.sol, contracts/base/ERC20Drop.sol, contracts/base/ERC20DropVote.sol, contracts/base/ERC20Vote.sol	Added virtual keyword to _msgSender() function signatures, allowing further overrides in derived contracts.
Base ERC721 Contracts contracts/base/ERC721Base.sol, contracts/base/ERC721Drop.sol, contracts/base/ERC721LazyMint.sol, contracts/base/ERC721Multiwrap.sol	Added virtual keyword to _msgSender() function signatures, enabling extension through inheritance.
Account Factory Contracts contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol, contracts/prebuilts/account/managed/ManagedAccountFactory.sol, contracts/prebuilts/account/non-upgradeable/AccountFactory.sol	Added virtual keyword to _msgSender() function signatures to support further overrides.
Registry Router contracts/infra/registry/entrypoint/TWMultichainRegistryRouter.sol	Added virtual keyword to _msgSender() function signature, expanding override capabilities.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately describes the main change: making _msgSender virtual across multiple contracts.
Linked Issues check	✅ Passed	The PR successfully addresses issue #646 by adding virtual modifiers to _msgSender across all base and factory contracts as intended.
Out of Scope Changes check	✅ Passed	All changes are directly related to adding virtual modifiers to _msgSender functions across base and factory contracts, with no unrelated modifications.
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 unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

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.}

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Set the reviews.auto_title_instructions setting to generate a title for your PR based on the changes in the PR with custom instructions.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@contracts/base/ERC20Vote.sol`:
- Line 108: The burn function mixes raw msg.sender with the overridable
_msgSender(), which can cause wrong-account burns under meta-transaction
overrides; update burn to consistently use _msgSender() for both the balance
check and the actual burn operation (ensure calls to balanceOf(...) and the
internal _burn(...) or transfer logic use _msgSender()), so the sender source is
unified with the overrideable _msgSender() implementation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b85fa2ba-ff84-46b0-bf38-98b147283eb4

📥 Commits

Reviewing files that changed from the base of the PR and between 0da770f and b2285ad.

📒 Files selected for processing (12)

• contracts/base/ERC20Base.sol
• contracts/base/ERC20Drop.sol
• contracts/base/ERC20DropVote.sol
• contracts/base/ERC20Vote.sol
• contracts/base/ERC721Base.sol
• contracts/base/ERC721Drop.sol
• contracts/base/ERC721LazyMint.sol
• contracts/base/ERC721Multiwrap.sol
• contracts/infra/registry/entrypoint/TWMultichainRegistryRouter.sol
• contracts/prebuilts/account/dynamic/DynamicAccountFactory.sol
• contracts/prebuilts/account/managed/ManagedAccountFactory.sol
• contracts/prebuilts/account/non-upgradeable/AccountFactory.sol

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Unify sender source in burn now that _msgSender() is overridable.

With this change, derived contracts can override _msgSender(), but burn still mixes sender sources: it checks balanceOf(_msgSender()) (Line 63) and burns msg.sender (Line 64). That can revert incorrectly or burn against the wrong caller context in meta-transaction-style overrides.

Proposed fix

 function burn(uint256 _amount) external virtual {
-    require(balanceOf(_msgSender()) >= _amount, "not enough balance");
-    _burn(msg.sender, _amount);
+    address sender = _msgSender();
+    require(balanceOf(sender) >= _amount, "not enough balance");
+    _burn(sender, _amount);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@contracts/base/ERC20Vote.sol` at line 108, The burn function mixes raw
msg.sender with the overridable _msgSender(), which can cause wrong-account
burns under meta-transaction overrides; update burn to consistently use
_msgSender() for both the balance check and the actual burn operation (ensure
calls to balanceOf(...) and the internal _burn(...) or transfer logic use
_msgSender()), so the sender source is unified with the overrideable
_msgSender() implementation.