blockful-io / swaplace-contracts

Swaplace is an open-source, ownerless and feeless token swap protocol
https://app.swaplace.xyz/
MIT License
33 stars 33 forks source link

refactor: change handle Errors without interface, only as smart contract #159

Closed luislucena16 closed 8 months ago

luislucena16 commented 8 months ago

I suggest changing the way Errors are called, removing the use of Interface and being called from a contract.

Example change this:

IErrors.sol
interface IErrors {
  error InvalidAddress(address caller);

to this:

SwaplaceErrors.sol
contract SwaplaceErrors {
  // Owner
    error InvalidAddress(address caller);

@0xneves what do you think? if you see fit, assign it to me too!

0xneves commented 8 months ago

Hey @luislucena16

The errors are hardcoded on deployment. Thus, having them in interfaces is a better approach for legibility. Both will have the same gas consumption. The same applies for events or structs. Specially when they are used in multiple contracts.

luislucena16 commented 8 months ago

hey @0xneves I don't find it very suitable to manage it from the interface, both are readable and quite clear, for me it is better to manage it in a separate contract as I mentioned! If we think about the growth of the project, doing it in this modular way would be much easier for contributions as well, an example of this:

Swaplace.sol => SwaplaceErrors.sol. Controller.sol => ControllerErrors.sol.

This way when making contributions you can touch different files with similar approaches and avoid conflicts between them, because suppose there are two users trying to modify IErrors at the same time, that would be a problem, because there would be all the errors in the project.

I suggest reopening this issue!