-
Notifications
You must be signed in to change notification settings - Fork 62
feture: refactored ERC6909 #284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
beebozy
wants to merge
4
commits into
Perfect-Abstractions:main
Choose a base branch
from
beebozy:feture-refactor-ERC6909
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity >=0.8.30; | ||
|
|
||
| /* Compose | ||
| * https://compose.diamonds | ||
| */ | ||
|
|
||
| contract ERC6909ApproveFacet { | ||
| /** | ||
| * @notice Thrown when the spender address is invalid. | ||
| */ | ||
| error ERC6909InvalidSpender(address _spender); | ||
|
|
||
| /** | ||
| * @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("erc6909"); | ||
|
|
||
| /** | ||
| * @custom:storage-location erc8042: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 | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @notice Approves an amount of an id to a spender. | ||
| * @param _spender The address of the spender. | ||
| * @param _id The id of the token. | ||
| * @param _amount The amount of the token. | ||
| * @return Whether the approval succeeded. | ||
| */ | ||
| function approve(address _spender, uint256 _id, uint256 _amount) external returns (bool) { | ||
| if (_spender == address(0)) { | ||
| revert ERC6909InvalidSpender(address(0)); | ||
| } | ||
|
|
||
| ERC6909Storage storage s = getStorage(); | ||
|
|
||
| s.allowance[msg.sender][_spender][_id] = _amount; | ||
|
|
||
| emit Approval(msg.sender, _spender, _id, _amount); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * @notice Exports the function selectors of the ERC6909ApproveFacet | ||
| * @dev This function is use as a selector discovery mechanism for diamonds | ||
| * @return selectors The exported function selectors of the ERC6909ApproveFacet | ||
| */ | ||
| function exportSelectors() external pure returns (bytes memory) { | ||
| return bytes.concat(this.approve.selector); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity >=0.8.30; | ||
|
|
||
| /* Compose | ||
| * https://compose.diamonds | ||
| */ | ||
|
|
||
| /** | ||
| * @notice Thrown when the spender address is invalid. | ||
| */ | ||
| error ERC6909InvalidSpender(address _spender); | ||
|
|
||
| /** | ||
| * @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("erc6909"); | ||
|
|
||
| /** | ||
| * @custom:storage-location erc8042: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() pure returns (ERC6909Storage storage s) { | ||
| bytes32 position = STORAGE_POSITION; | ||
| assembly { | ||
| s.slot := position | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @notice Approves an amount of an id to a spender. | ||
| * @param _spender The address of the spender. | ||
| * @param _id The id of the token. | ||
| * @param _amount The amount of the token. | ||
| * @return Whether the approval succeeded. | ||
| */ | ||
| function approve(address _spender, uint256 _id, uint256 _amount) returns (bool) { | ||
| if (_spender == address(0)) { | ||
| revert ERC6909InvalidSpender(address(0)); | ||
| } | ||
|
|
||
| ERC6909Storage storage s = getStorage(); | ||
|
|
||
| s.allowance[msg.sender][_spender][_id] = _amount; | ||
|
|
||
| emit Approval(msg.sender, _spender, _id, _amount); | ||
|
|
||
| return true; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity >=0.8.30; | ||
|
|
||
| /* Compose | ||
| * https://compose.diamonds | ||
| */ | ||
|
|
||
| contract ERC6909BurnFacet { | ||
| /** | ||
| * @notice Thrown when the sender has insufficient balance. | ||
| */ | ||
| error ERC6909InsufficientBalance(address _sender, uint256 _balance, uint256 _needed, uint256 _id); | ||
|
|
||
| /** | ||
| * @notice Thrown when the spender has insufficient allowance. | ||
| */ | ||
| error ERC6909InsufficientAllowance(address _spender, uint256 _allowance, uint256 _needed, uint256 _id); | ||
|
|
||
| /** | ||
| * @notice Thrown when the sender address is invalid. | ||
| */ | ||
| error ERC6909InvalidSender(address _sender); | ||
|
|
||
| /** | ||
| * @notice Emitted when a transfer occurs. | ||
| */ | ||
|
|
||
| event Transfer( | ||
| address _caller, address indexed _sender, address indexed _receiver, uint256 indexed _id, uint256 _amount | ||
| ); | ||
|
|
||
| /** | ||
| * @dev Storage position determined by the keccak256 hash of the diamond storage identifier. | ||
| */ | ||
| bytes32 constant STORAGE_POSITION = keccak256("erc6909"); | ||
|
|
||
| /** | ||
| * @custom:storage-location erc8042: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 | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @notice Burns (destroys) a specific amount of tokens from the caller's balance. | ||
| * @dev Emits a {Transfer} event to the zero address. | ||
| * @param _amount The amount of tokens to burn. | ||
| */ | ||
| function burn(uint256 _id, uint256 _amount) external { | ||
| ERC6909Storage storage s = getStorage(); | ||
|
|
||
| uint256 fromBalance = s.balanceOf[msg.sender][_id]; | ||
|
|
||
| if (fromBalance < _amount) { | ||
| revert ERC6909InsufficientBalance(msg.sender, fromBalance, _amount, _id); | ||
| } | ||
|
|
||
| unchecked { | ||
| s.balanceOf[msg.sender][_id] = fromBalance - _amount; | ||
| } | ||
|
|
||
| emit Transfer(msg.sender, msg.sender, address(0), _id, _amount); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Burns tokens from another account, deducting from the caller's allowance. | ||
| * @dev Emits a {Transfer} event to the zero address. | ||
| * @param _from The address whose tokens will be burned. | ||
| * @param _amount The amount of tokens to burn. | ||
| */ | ||
| function burnFrom(address _from, uint256 _id, uint256 _amount) external { | ||
| if (_from == address(0)) { | ||
| revert ERC6909InvalidSender(_from); | ||
| } | ||
|
|
||
| ERC6909Storage storage s = getStorage(); | ||
|
|
||
| uint256 currentAllowance = s.allowance[_from][msg.sender][_id]; | ||
|
|
||
| if (currentAllowance < type(uint256).max) { | ||
| if (currentAllowance < _amount) { | ||
| revert ERC6909InsufficientAllowance(msg.sender, currentAllowance, _amount, _id); | ||
| } | ||
| unchecked { | ||
| s.allowance[_from][msg.sender][_id] = currentAllowance - _amount; | ||
| } | ||
|
|
||
| uint256 fromBalance = s.balanceOf[_from][_id]; | ||
|
|
||
| unchecked { | ||
| s.balanceOf[_from][_id] = fromBalance - _amount; | ||
| } | ||
| } | ||
| emit Transfer(msg.sender, _from, address(0), _id, _amount); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Exports the function selectors of the ERC6909BurnFacet | ||
| * @dev This function is use as a selector discovery mechanism for diamonds | ||
| * @return selectors The exported function selectors of the ERC6909BurnFacet | ||
| */ | ||
| function exportSelectors() external pure returns (bytes memory) { | ||
| return bytes.concat(this.burn.selector, this.burnFrom.selector); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity >=0.8.30; | ||
|
|
||
| /* Compose | ||
| * https://compose.diamonds | ||
| */ | ||
| /** | ||
| * @notice Thrown when the sender has insufficient balance. | ||
| */ | ||
| error ERC6909InsufficientBalance(address _sender, uint256 _balance, uint256 _needed, uint256 _id); | ||
|
|
||
| /** | ||
| * @notice Thrown when the spender has insufficient allowance. | ||
| */ | ||
| error ERC6909InsufficientAllowance(address _spender, uint256 _allowance, uint256 _needed, uint256 _id); | ||
|
|
||
| /** | ||
| * @notice Emitted when a transfer occurs. | ||
| */ | ||
| event Transfer( | ||
| address _caller, address indexed _sender, address indexed _receiver, uint256 indexed _id, uint256 _amount | ||
| ); | ||
|
|
||
| /** | ||
| * @dev Storage position determined by the keccak256 hash of the diamond storage identifier. | ||
| */ | ||
| bytes32 constant STORAGE_POSITION = keccak256("erc6909"); | ||
|
|
||
| /** | ||
| * @custom:storage-location erc8042: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() pure returns (ERC6909Storage storage s) { | ||
| bytes32 position = STORAGE_POSITION; | ||
| assembly { | ||
| s.slot := position | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @notice Burns (destroys) a specific amount of tokens from the caller's balance. | ||
| * @dev Emits a {Transfer} event to the zero address. | ||
| * @param _amount The amount of tokens to burn. | ||
| */ | ||
| function burn(uint256 _id, uint256 _amount) external { | ||
| ERC6909Storage storage s = getStorage(); | ||
|
|
||
| uint256 fromBalance = s.balanceOf[msg.sender][_id]; | ||
|
|
||
| if (fromBalance < _amount) { | ||
| revert ERC6909InsufficientBalance(msg.sender, fromBalance, _amount, _id); | ||
| } | ||
|
|
||
| unchecked { | ||
| s.balanceOf[msg.sender][_id] = fromBalance - _amount; | ||
| } | ||
|
|
||
| emit Transfer(msg.sender, msg.sender, address(0), _id, _amount); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Burns tokens from another account, deducting from the caller's allowance. | ||
| * @dev Emits a {Transfer} event to the zero address. | ||
| * @param _from The address whose tokens will be burned. | ||
| * @param _amount The amount of tokens to burn. | ||
| */ | ||
| function burnFrom(address _from, uint256 _id, uint256 _amount) external { | ||
| if (_from == address(0)) { | ||
| revert ERC6909InvalidSender(_from); | ||
| } | ||
|
|
||
| ERC6909Storage storage s = getStorage(); | ||
|
|
||
| uint256 currentAllowance = s.allowance[_from][msg.sender][_id]; | ||
|
|
||
| if (currentAllowance < type(uint256).max) { | ||
| if (currentAllowance < _amount) { | ||
| revert ERC6909InsufficientAllowance(msg.sender, currentAllowance, _amount, _id); | ||
| } | ||
| unchecked { | ||
| s.allowance[_from][msg.sender][_id] = currentAllowance - _amount; | ||
| } | ||
|
|
||
| uint256 fromBalance = s.balanceOf[_from][_id]; | ||
|
|
||
| unchecked { | ||
| s.balanceOf[_from][_id] = fromBalance - _amount; | ||
| } | ||
| } | ||
| emit Transfer(msg.sender, _from, address(0), _id, _amount); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currentAllowanceif (line 93)similar to the transferFrom of the
ERC6909TransferFacetUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@beebozy Please review this too. What do you think of making the balance check and update after the allowance check and update?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reviewing a bit more, i would do something like this:
Current issues in the code:
Question that need reflexion: do we allow an operator to burn tokens? (in the code proposed above, i just allowed the msg.sender to the the _from)