Refactor to ERC20Ubiquity

[eugenioclrc]

There are a few things to solve in ERC20Ubiquity

• Import of ERC20Burnable is not used
• OZ already have an ERC20Permit implementation, ERC20Ubiquity could use it to have a simpler contract
• _tokenName and _symbol is already set in the ERC20 it should be redefined
• name and symbol should not change, also if the admin change it, it will create conflicts with the DOMAIN_SEPARATOR see ERC20Ubiquity.sol#L64

I would also recommend to implement solmate ERC20 implementation, is more gas efficient and it has ERC20Permit by default but you will have to do a deeper refactor.

[0x4007]

Could you provide a rough time estimate for implementation? I'm assuming it can be done within a day.

---

name and symbol should not change, also if the admin change it, it will create conflicts with the DOMAIN_SEPARATOR see ERC20Ubiquity.sol#L64

We wanted to preserve the ability to rename. Is there a way to prioritize this while not breaking the functionality of DOMAIN_SEPARATOR? I don't even know what that is used for to be honest.

---

@zgorizzo69 what do you think about this proposal?

[eugenioclrc]

All these tasks can indeed be accomplished within a day. However, i would recommend a separate task for the refactoring to use solmate. This should be addressed after initial refactor, and it could require additional time.

[eugenioclrc]

@pavlovcik The DOMAIN_SEPARATOR is part of the EIP712 and in this case is use for the ERC20Permit see OZ Implementation

[eugenioclrc]

this issue should be resolved before this refactor;
#643

[0x4007]

However, i would recommend a separate task for the refactoring to use solmate. This should be addressed after initial refactor, and it could require additional time.

Please feel free to create the bounty when you're ready.

[zgorizzo69]

Yep valid points thanks @eugenioclrc

[eugenioclrc]

/start

[eugenioclrc]

/start

[ubiquibot]

Skipping /start since the issue is closed

[eugenioclrc]

@pavlovcik @zgorizzo69 im having some issues here, with this refactor i have remove multiple lines, that means that coverage metrics will get lower (the ERC20Permit lib is excluded from coverage), thats why its not passing the coverage task in the github actions.
Got any suggestions?

[eugenioclrc]

@pavlovcik @zgorizzo69 im having some issues here, with this refactor i have remove multiple lines, that means that coverage metrics will get lower (the ERC20Permit lib is excluded from coverage), thats why its not passing the coverage task in the github actions. Got any suggestions?

nevermind, i solved, it seems that somehow the coverage thought that the method unpaused wasnt test it... i solved adding a extra test condition;
71cf980

[ubiquibot]

Do you have any updates @eugenioclrc? If you would like to release the bounty back to the DevPool, please comment /unassign

[eugenioclrc]

Do you have any updates @eugenioclrc? If you would like to release the bounty back to the DevPool, please comment /unassign

@pavlovcik @zgorizzo69 the work is ready, why the bot is asking if I have finished?

[eugenioclrc]

#649

[0x4007]

My apologies. Unfortunately @zgorizzo69 hasn't had the time to review yet and their review is required to merge pull requests on this repository. I sent them a message to check soon. Thanks for your patience!

In the future the bot will check for the review status to not follow up with the bounty hunter unless needed.

[ubiquibot]

[ CLAIM 200 DAI ]

0xD75943B...5282d2AD