Refactor Fees

[stevieraykatz]

Ticket(s):
We currently store and track fees in three different places using three different mechanisms. This PR will unify the way we track and store fees.

Explanation of the solution

We can implement an ENUM in the LocalRegistrar that acts as a key set for all Fees. In the storage of the LocalRegistrar, we can employ a new mapping which stores the fee rate per enumerated Fee.

Some limitations with the way that I have it written now:

• All of the fee names must be enumerated at the LocalRegistrar level; even ones that won't be used on a cross chain implementation
• This currently doesn't employ the same theoretical flexibility that our existing EndowmentFee struct provides:

    struct EndowmentFee {
        address payoutAddress;
        uint256 feePercentage;
        bool active;
    }

However, looking through our code, it doesn't look like we take advantage of the struct's feature set anywhere so maybe we don't need that granular level of control?

• Using what I currently have sketched, we won't have a way of tracking fees on a per endowment basis. It seems like we're protecting for the concept of "fees by endowment by situation" with our current structure/config, but are we really going to change the fees for individuals? I think that we can add protections here wherein the Enum covers all our basis assuming we don't need control on an individual basis.

Instructions on making this work

• run yarn or yarn install to install npm dependencies
• run yarn test to verify all tests still pass

[SovereignAndrey]

@stevieraykatz

This currently doesn't employ the same theoretical flexibility that our existing EndowmentFee struct provides

I think it's OK that the Registrar Fees setup doesn't have the same flexibility. At the end of the day:

• AP Protocol-level Fees will flow to a singular address (ex. Collector contract, AP Team MultiSig, or some random wallet) that we set and manage at the Registrar level
• Endowment-level fees will need the flexibility to go to separate payout addresses

While we're thinking about Fees and refactoring, wdyt about removing EndowmentFee.active field and use EndowmentFee.percentage as 0 for anything that wasn't active? If yes, I'll work in those changes in the cosmos branch as part of the final prep to go into merge-registrars after this branch is merged in first.

UPDATES: Made the changes to EndowmentFee in merge-registrars.

• No more more active field,
• renamed feePercentage > percentage
• added check that new percentages passed are less than 100% when updating EndowmentFees

    struct EndowmentFee {
        address payoutAddress;
        uint256 percentage;
    }

[stevieraykatz]

I like the idea of removing the active param and using 0 as a default/off switch.

I think we can actually extend this one step further and use the same EndowmentFee struct in the Registrar's mapping:

mapping(Fees => EndowmentFee) FeeRateByFees;

I'm also a fan of renaming some things for clarity. Perhaps:

struct EndowmentFee -> struct FeeSetting
EndowmentFee.percentage -> FeeSetting.feeRate
enum Fees -> enum FeeTypes
mapping FeeRateByFees -> mapping FeeSettingsByFeeType

[SovereignAndrey]

@stevieraykatz I agree with all your suggested changes! That'll unify everything nicely and clarifies what things are and how they can be used. 💯

Those suggestions have been captured in 921f83b.

[stevieraykatz]

@SovereignAndrey should be ready for review and merge