-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature: Implements ERC1404. #15
Conversation
cb47fb7
to
47cea1d
Compare
238ae77
to
eebc62d
Compare
import "./interfaces/IToken.sol"; | ||
import "./lib/AccountLib.sol"; | ||
import "./Registry.sol"; | ||
import "./Transact.sol"; | ||
|
||
|
||
contract Token is Initializable, IToken, IERC20 { | ||
contract Token is Initializable, IToken, IERC20, IERC1404 { |
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.
@afmfe-iul This is the main change from a functional perspective. Our Token contract now has to follow that IERC1404
interface and implement the methods it imposes.
@@ -26,7 +31,21 @@ contract Token is Initializable, IToken, IERC20 { | |||
mapping (address => AccountLib.Data) accounts; | |||
mapping (address => mapping (address => uint256)) allowances; | |||
|
|||
// Public functions. | |||
/// --- Constants. |
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.
@afmfe-iul The way ERC1404
works is that before every transfer, you call a function to "detect" (detectTransferRestriction
) whether you'd be authorised to perform it. These tests should not cover balance etc, only business logic that imposes restrictions (Being an actor etc).
The detection function returns errors in the form of uint8
numbers, and exposes another function to allow a code to be mapped into a human-readable string message (messageForTransferRestriction
).
These are essentially constants for each error in our transfer restriction logic, and their message counterpart.
address owner = msg.sender; | ||
require(owner != recipient, "Recipient cannot be the same as owner"); |
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.
@afmfe-iul We now have a modifier (ownerAndRecipientDifferent
) that does this, so we can use it here too.
// ------------------------ Public but app functions. | ||
/// --- ERC1404 functions. | ||
|
||
function detectTransferRestriction (address owner, address recipient, uint256) public view returns (uint8) { |
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.
@afmfe-iul This is the transfer restriction detection method. Based on its input, it can return an error code or zero if everything is good.
Note that this function shall be called from the front-end wallet in the next iteration, so the results are consistent with the ERC1404 implementation.
Basically when a user would push the "Transfer" button in the front-end, we would run the detection function, and if the return code isn't zero we would display a message explaining that a restriction prevents the transfer to be initiated. Since the function is a view
(not a transaction), it can be called for free.
} | ||
} | ||
|
||
function messageForTransferRestriction (uint8 errCode) public view returns (string memory) { |
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.
@afmfe-iul This function converts restriction detection error codes into human-readable messages. From a front-end perspective they will be useful if the detection function triggers to retrieve an error message that we can display as an explanation.
@@ -228,10 +283,26 @@ contract Token is Initializable, IToken, IERC20 { | |||
_; | |||
} | |||
|
|||
modifier isActor(address c) { | |||
modifier isOwnerActor(address c) { |
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.
@afmfe-iul I decided to remove the plain isActor
modifier in favour of more verbose ones (so you can know which one of your parameter is actually not an actor).
@@ -0,0 +1,26 @@ | |||
pragma solidity ^0.5.9; |
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.
@afmfe-iul This is the official ERC1404 interface as described in the EIP that can be found here. It is not closed but I can't forsee major changes to it so it's safe to assume that it will remain identical for the main things.
eebc62d
to
9770872
Compare
9770872
to
e582918
Compare
Codecov Report
@@ Coverage Diff @@
## master #15 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 17 17
Lines 296 315 +19
Branches 32 38 +6
=====================================
+ Hits 296 315 +19
|
This PR integrates the ERC1404 (soon to be) standard in our token.
Also, a suggestion I made to the EIP: ethereum/EIPs#1404