Skip to content

refactor: make ERC1271Forwarder.isValidSignature overridable - #61

Merged
mfw78 merged 1 commit into
developfrom
refactor/forwarder-overridable
Aug 2, 2026
Merged

refactor: make ERC1271Forwarder.isValidSignature overridable#61
mfw78 merged 1 commit into
developfrom
refactor/forwarder-overridable

Conversation

@mfw78

@mfw78 mfw78 commented Aug 2, 2026

Copy link
Copy Markdown

Based on develop. #59 is stacked on this, so this merges first.

Two ABI-neutral token changes to ERC1271Forwarder.isValidSignature: bytes memory to bytes calldata, and virtual. Same selector, same external ABI, body untouched.

-function isValidSignature(bytes32 _hash, bytes memory signature) public view override returns (bytes4)
+function isValidSignature(bytes32 _hash, bytes calldata signature) public view virtual override returns (bytes4)

Why

A contract co-inheriting this forwarder with another ERC-1271 implementation cannot compile otherwise. Solc forbids co-inheriting public functions whose data locations differ, and the function was not overridable.

That is not hypothetical. An owner that wants to answer both the ComposableCow order payload and a direct signature of its own has to implement isValidSignature twice over and dispatch between them, which requires overriding this one.

Nothing here assumes a Safe, and it never did: isValidSafeSignature takes one only as a typed address, _auth reads the registry's own roots and singleOrders, test/ComposableCow.base.t.sol already has TestNonSafeWallet, and _buildSignature already has a catch branch commented "Assume a non-Safe wallet". This makes that path usable by an owner that also has signature logic of its own.

#60 carried more alongside this and is closed.

Verification

172 tests pass, forge build and forge fmt --check clean, descriptors current. No behaviour changes, so no test changes.

@mfw78
mfw78 changed the base branch from feat/admin-commitments to develop August 2, 2026 12:10
Two ABI-neutral token changes: `bytes memory` to `bytes calldata`, and
`virtual`. Same selector, same external ABI, body untouched.

A contract co-inheriting this forwarder with another ERC-1271 implementation
cannot compile otherwise: solc forbids co-inheriting public functions whose data
locations differ, and the function was not overridable. That combination is what
an EIP-7702 account needs, since it has to answer both the ComposableCow order
payload and a direct owner signature.

The account itself lives in nxm-rs/nexum-account and depends on this repository
for the forwarder, so nothing else moves here.
@mfw78
mfw78 force-pushed the refactor/forwarder-overridable branch from a716ee9 to 5a39e21 Compare August 2, 2026 12:12
@mfw78
mfw78 merged commit be4c311 into develop Aug 2, 2026
1 check passed
@mfw78
mfw78 deleted the refactor/forwarder-overridable branch August 2, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant