Add ERC-6909 implementations #167

lumoswiz · 2025-11-03T10:53:29Z

Summary

Add ERC6909Facet and LibERC6909 implementations adhering to ERC-6909.

A minimal styling favouring panic via arithmetic checks in lieu of custom errors was used here. Please see additional notes below for alternative styles.

Testing to be done over the next few days.

Changes Made

Added:

ERC6909Facet.sol
LibERC6909.sol

Checklist

Before submitting this PR, please ensure:

Code follows the Solidity feature ban - No inheritance, constructors, modifiers, public/private variables, external library functions, using for directives, or selfdestruct
Code follows Design Principles - Readable, uses diamond storage, favors composition over inheritance
Code matches the codebase style - Consistent formatting, documentation, and patterns (e.g. ERC20Facet.sol)
Code is formatted with forge fmt
Tests are included - All new functionality has comprehensive tests
All tests pass - Run forge test and ensure everything works
Documentation updated - If applicable, update relevant documentation

Make sure to follow the CONTRIBUTING.md guidelines.

Additional Notes

There are multiple ERC-6909 implementations available, with slightly different styles. See links below:

Uniswap V4, which was adapted from Solmate
Solady
OpenZeppelin

I have opted for the minimal Uniswap V4/Solmate implementation, however, I am happy to adapt to whatever styling you prefer. Please let me know.

github-actions · 2025-11-03T10:53:59Z

Coverage Report

Metric	Coverage	Details
Lines	47%	667/1431 lines
Functions	58%	143/246 functions
Branches	25%	63/253 branches

Last updated: Mon, 03 Nov 2025 10:53:47 GMT for commit 681c035

github-actions · 2025-11-03T10:54:23Z

Gas Report

No gas usage changes detected between main and feat/erc6909.

All functions maintain the same gas costs. ✅

Last updated: Mon, 03 Nov 2025 10:54:08 GMT for commit 681c035

mudgen · 2025-11-03T13:26:18Z

Great! Will look at this. I am glad to have an ERC-6909 implementation.

lumoswiz · 2025-11-05T08:36:26Z

Forgot to add, after I have finalised the styling and testing of the base ERC-6909, I'll add the extensions (token supply, metadata and content URI).

mudgen · 2025-11-08T22:53:58Z

@lumoswiz Just want you to know, I am happy this pull request was submitted, because I want Compose to provide this ERC functionality. I and/or another reviewer will get around to this pull request.

mudgen

Looking great!

mudgen · 2025-11-16T20:43:50Z

src/token/ERC6909/LibERC6909.sol

+
+        if (_by != address(0) && !s.isOperator[_from][_by]) {
+            uint256 allowed = s.allowance[_from][_by][_id];
+            if (allowed != type(uint256).max) s.allowance[_from][_by][_id] = allowed - _amount;


Please use braces here, for example:

if (allowed != type(uint256).max)) { s.allowance[_from][_by][_id] = allowed - _amount; }

mudgen · 2025-11-16T20:45:06Z

src/token/ERC6909/ERC6909Facet.sol

+        ERC6909Storage storage s = getStorage();
+        if (msg.sender != _sender && !s.isOperator[_sender][msg.sender]) {
+            uint256 allowed = s.allowance[_sender][msg.sender][_id];
+            if (allowed != type(uint256).max) s.allowance[_sender][msg.sender][_id] = allowed - _amount;


Use braces here please.

Copilot

Pull Request Overview

This PR adds an ERC-6909 multi-token standard implementation using the diamond storage pattern. The implementation follows a minimal style that favors arithmetic panics over custom errors, adapted from the Uniswap V4/Solmate implementation.

Key changes:

Added LibERC6909.sol providing internal functions (mint, burn, transfer, approve, setOperator) for ERC-6909 token operations
Added ERC6909Facet.sol exposing the public ERC-6909 interface (balanceOf, allowance, isOperator, transfer, transferFrom, approve, setOperator)

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/token/ERC6909/LibERC6909.sol	Library containing ERC-6909 internal functions and diamond storage layout for multi-token operations
src/token/ERC6909/ERC6909Facet.sol	Facet implementing the public ERC-6909 interface using diamond storage pattern

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-16T20:59:12Z

src/token/ERC6909/LibERC6909.sol

+
+        if (_by != address(0) && !s.isOperator[_from][_by]) {
+            uint256 allowed = s.allowance[_from][_by][_id];
+            if (allowed != type(uint256).max) s.allowance[_from][_by][_id] = allowed - _amount;


This one-line if statement violates the project's style guide (STYLE.md section 5), which requires all if statements to use brackets and a newline. The statement should be reformatted as:\nsolidity\nif (allowed != type(uint256).max) {\n s.allowance[_from][_by][_id] = allowed - _amount;\n}\n

Suggested change

if (allowed != type(uint256).max) s.allowance[_from][_by][_id] = allowed - _amount;

if (allowed != type(uint256).max) {

s.allowance[_from][_by][_id] = allowed - _amount;

}

Copilot · 2025-11-16T20:59:13Z

src/token/ERC6909/ERC6909Facet.sol

+        ERC6909Storage storage s = getStorage();
+        if (msg.sender != _sender && !s.isOperator[_sender][msg.sender]) {
+            uint256 allowed = s.allowance[_sender][msg.sender][_id];
+            if (allowed != type(uint256).max) s.allowance[_sender][msg.sender][_id] = allowed - _amount;


This one-line if statement violates the project's style guide (STYLE.md section 5), which requires all if statements to use brackets and a newline. The statement should be reformatted as:\nsolidity\nif (allowed != type(uint256).max) {\n s.allowance[_sender][msg.sender][_id] = allowed - _amount;\n}\n

Suggested change

if (allowed != type(uint256).max) s.allowance[_sender][msg.sender][_id] = allowed - _amount;

if (allowed != type(uint256).max) {

s.allowance[_sender][msg.sender][_id] = allowed - _amount;

}

Copilot · 2025-11-16T20:59:13Z

src/token/ERC6909/LibERC6909.sol

+
+/// @title LibERC6909 — ERC-6909 Library
+/// @notice Provides internal functions and storage layout for ERC-6909 minimal multi-token logic.
+/// @dev Uses ERC-8042 for storage location standardization and ERC-6093 for error conventions.


The comment claims to use 'ERC-6093 for error conventions', but the implementation does not define or use any custom errors (it relies on arithmetic panics instead). Either remove the ERC-6093 reference from the documentation or add appropriate custom errors as done in other token implementations (see LibERC20.sol for reference).

Suggested change

/// @dev Uses ERC-8042 for storage location standardization and ERC-6093 for error conventions.

/// @dev Uses ERC-8042 for storage location standardization.

Copilot · 2025-11-16T20:59:13Z

src/token/ERC6909/ERC6909Facet.sol

+    /// @notice Emitted when a transfer occurs.
+    event Transfer(
+        address _caller, address indexed _sender, address indexed _receiver, uint256 indexed _id, uint256 _amount
+    );
+
+    /// @notice Emitted when an operator is set.
+    event OperatorSet(address indexed _owner, address indexed _spender, bool _approved);
+
+    /// @notice Emitted when an approval occurs.
+    event Approval(address indexed _owner, address indexed _spender, uint256 indexed _id, uint256 _amount);
+
+    /// @dev Storage position determined by the keccak256 hash of the diamond storage identifier.
+    bytes32 constant STORAGE_POSITION = keccak256("compose.erc6909");
+
+    /// @custom:storage-location erc8042:compose.erc6909
+    struct ERC6909Storage {
+        mapping(address owner => mapping(uint256 id => uint256 amount)) balanceOf;
+        mapping(address owner => mapping(address spender => mapping(uint256 id => uint256 amount))) allowance;
+        mapping(address owner => mapping(address spender => bool)) isOperator;
+    }
+
+    /// @notice Returns a pointer to the ERC-6909 storage struct.
+    /// @dev Uses inline assembly to access the storage slot defined by STORAGE_POSITION.
+    /// @return s The ERC6909Storage struct in storage.
+    function getStorage() internal pure returns (ERC6909Storage storage s) {
+        bytes32 position = STORAGE_POSITION;
+        assembly {
+            s.slot := position
+        }
+    }


[nitpick] There is significant code duplication between ERC6909Facet.sol and LibERC6909.sol. Both files define the same events (Transfer, OperatorSet, Approval), storage struct (ERC6909Storage), storage position constant (STORAGE_POSITION), and getStorage() function. According to the 'No Imports in Facets' rule (STYLE.md section 1), this duplication is intentional for self-containment. However, consider whether LibERC6909 functions could be used by the facet to reduce this duplication, or document why both implementations need to be completely standalone.

@copilot I suggest you read the documentation for the project. Specifically the design of compose here: https://compose.diamonds/docs/design/

lumoswiz added 4 commits November 3, 2025 10:18

feat: add ERC-6909 facet and lib skeleton with NatSpec

5153017

feat: implement LibERC6909 functions

199c3e1

feat: implement ERC6909Facet functions

3be87cb

refactor: align style with ERC6909Facet, rely on checked arithmetic

681c035

mudgen approved these changes Nov 16, 2025

View reviewed changes

mudgen reviewed Nov 16, 2025

View reviewed changes

mudgen added the Needs Review label Nov 16, 2025

mudgen marked this pull request as ready for review November 16, 2025 20:55

Copilot AI review requested due to automatic review settings November 16, 2025 20:55

Copilot started reviewing on behalf of mudgen November 16, 2025 20:56 View session

Copilot finished reviewing on behalf of mudgen November 16, 2025 20:58

Copilot AI reviewed Nov 16, 2025

View reviewed changes

	/// @dev Uses ERC-8042 for storage location standardization and ERC-6093 for error conventions.
	/// @dev Uses ERC-8042 for storage location standardization.

Add ERC-6909 implementations #167

Are you sure you want to change the base?

Add ERC-6909 implementations #167

Conversation

lumoswiz commented Nov 3, 2025

Summary

Changes Made

Checklist

Additional Notes

Uh oh!

github-actions bot commented Nov 3, 2025

Coverage Report

Uh oh!

github-actions bot commented Nov 3, 2025

Gas Report

Uh oh!

mudgen commented Nov 3, 2025

Uh oh!

lumoswiz commented Nov 5, 2025

Uh oh!

mudgen commented Nov 8, 2025

Uh oh!

mudgen left a comment

Choose a reason for hiding this comment

Uh oh!

mudgen Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

mudgen Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

mudgen Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mudgen Nov 16, 2025 •

edited

Loading