feat(wrapper-registry): adding wrapper-registry

[DRC9702]

Changes:

• Adding IWrapperRegistry and WrapperRegistry

Testing:

• Wrote unit tests with 100% coverage

Reviewers:

@marktoda
@aalavandhan
@Melvillian
@Fiddlekins
@SocksNFlops
@MichalStachnik

[aalavandhan]

Nice and clean interface definition 👍

[aalavandhan]

You could just invoke IWrapper(wrapper).underlying()? (which is one external storage read, rather than n internal reads)

[DRC9702]

I guess this function also had the purpose of validating that the underlying<->wrapper entry was present in the registry. Since IWrapper already has that functionality, it might actually be redundant to include this function at all. Thoughts?

[marktoda]

I'd elect to remove. You could determine if the wrapper was already in the registry with a staticcall to addWrapper

[aalavandhan]

You can make the _underlyingToWrapperMapping an EnumerableMap instead of the regular mapping and get rid of the _underlyingTokens address set.

https://docs.openzeppelin.com/contracts/3.x/api/utils#EnumerableMap

That should still accomplish what you want ie):

• O(1) add, remove and existence checks for underlying tokens
• O(n) enumeration

[DRC9702]

@aalavandhan, I believe EnumerableMap currently only has support for UintToAddressMap. That's the reason why I use both EnumerableSet and mapping to achieve the same goal.

[aalavandhan]

Makes sense 👍

[marktoda]

Looks great, nice job

[marktoda]

I'd elect to remove. You could determine if the wrapper was already in the registry with a staticcall to addWrapper

[aalavandhan]

Is this check really required? The owner can ensure that they only add immutable wrappers so this will always be true.

[DRC9702]

@aalavandhan This is a non-permissioned view function that operates as the opposite of getWrapperFromUnderlying. The issue of not doing the check is that you can just take any wrapper and call IButtonWrapper(wrapperToken).underlying() without confirming if the wrapper is even in the registry or not. So if AMPL has one official wrapper and one unofficial bootleg wrapper, they'll both return AMPL even though only the first one is actually in the registry.

This isn't really a problem, but I think the point is that this function also operates as a check of "is this wrapper in the registry?". Otherwise people would just call .underlying() directly on the wrapper.