Skip to content
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

uses arb test mint token as example for updated contracts #4

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stuartwk
Copy link
Contributor

No description provided.

bytes memory data
) external returns (bool success);

event Transfer(address indexed from, address indexed to, uint256 value, bytes data);
Copy link
Member

@Akira-Taniguchi Akira-Taniguchi Oct 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is duplicated in the Transfer event of IERC20Upgradeable, is that OK?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the interface is declared duplicate, I think you need to remove one or the other.

contract aeERC20 is ERC20PermitUpgradeable, TransferAndCallToken, ReentrancyGuardUpgradeable {
using AddressUpgradeable for address;

constructor() public initializer {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can delete this constructor.

}
}

contract ArbDEVTokenL1 is aeERC20, ICustomToken {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need not to create this contract in upgradable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this contract should be upgradeable. Because Arbitrum is still unstable, and the requirements may change.

Comment on lines +50 to +52
function mint() external {
_mint(msg.sender, 50000000);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this function is needed. Maybe for testing or something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If such an interface is required, we can upgrade this implementation contract later. Therefore, we can remove functions that we think are unnecessary.

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.

3 participants