Added adapters mappings and helpers

[martinvol]

Description

Added primitives to easily answer this questions:

• What are all the underlying assets?
• What is the underlying assets of a gas currency addresses (token or adaptor)?
• How can I get all the list of underlying gas currencies (token or adaptor) and it's corresponding underlying asset in one RPC request?

• Add natspecs

Other changes

Describe any minor or "drive-by" changes here.

Tested

An explanation of how the changes were tested or an explanation as to why they don't need to be.

Related issues

• Fixes #[issue number here]

Backwards compatibility

Brief explanation of why these changes are/are not backwards compatible.

Documentation

The set of community facing docs that have been added/modified because of this change

[aaronmgdr]

in this case the if i call getAdaptor with an random address the random address will be returned.

from what we said last night we wanted to restrict to valid ones right?

[martinvol]

It's actually hard to check now if something is or not a whitelisted gas fee address as it's an array and not a mapping. You'll get that but it doesn't mean anything.

[aaronmgdr]

fair enough.

In that case Id argue it should return address(0) when nothing is found. The method is getAdapter() so it returning null when either address is wrong or when address does not need work is intuitive.

[pahor167]

We could make this AddressSet which would allow to add underlyingTokens directly in setAdaptor rather than have special method for it

[pahor167]

This will not work if we want to have multiple adapters for one token ..

[martinvol]

I'd rather not update this to 0.8 and leave as is

[pahor167]

The problem with current approach is that it is rather manual.

You can setAdapter and you can forget to setUnderlyingToken which will result in inconsistent behaviour. If we will introduce AddressSet instead of array, then we can remove setUnderlyingToken and instead set the underlying token in setAdapter

[martinvol]

I can make setAdapter also set the underlying token, so it's one call and there's no chance for error.

[martinvol]

And regarding this will not work for more than a token, yes, that's correct. But in reality I don't see how we could have more than one adaptor for a token,

[pahor167]

We should also be able to remove Adapter

[martinvol]

you can set it to zero

[pahor167]

it is not the same though ... it will not remove it from the storage

[mcortesi]

Does this whole PR makes sense??? To me, before this PR, FeeCurrencyAdapter was not a concern of the protocol, nor the protocol needed to know ANYTHING about it. But now we're coupling both.

I'm not sure this is a good idea, IMO. What's the benefit?

You can always get the FeeCurrency and check if it implements some method liks "isAdapter()" or "getUnderlyingAsset()"...

[mcortesi]

Expanding on before:

To me the problem is that this PR or the design behind it breaks what it was a nice decoupled interface.

Before No part of the system was "aware" of FeeCurrencyAdapter, nor that some fee currencies required an adapter. From the system's perspective, every configured fee currency needs to implement a feeCurrency Interface and that's it. No questions asked.

With the change on this PR, we break that. Now the FeeCurrencyWhitelist "knows" about the existing of adapters and has a mapping of them. In that sense we break this decoupling (or separation of concerns) that was previously very explicit.

To make a analogy, in some OO programing or strict type programming language, you would have a List<FeeCurrency> where FeeCurrency is an interface, and you wouldn't need to know anything about the implementation of those FeeCurrency objects, just that they follow the interface. Now, you have another mapping with knowledge about the implementation of those FeeCurrencies, and you've spread that to the system.

In the future, if you wanted to remove the notion of Adapter, without this PR it was just a matter of deregistering feeCurrencies by governance. No code, and nor traces would have remained. With this PR, you actually have the notion inside the FeeCurrencyWhitelist and you'd need to remove that too, or leave it as tech debt.

The way this is typically solve on programming language with strong types is to "ask" wether an interface implements some other type. Basically an instanceOf check. This you could easily do now, without any changes. You would fetch al the feeCurrency from the whitelist contract, then for each you could "try" to call getFeeAdapter() method that is present on the FeeAdapter contract and by that learn that (a) is a fee currency adapter and (b) to whom. This might be similar to 'duck typing' .. "If it walks like a duck and it quacks like a duck, then it must be a duck"

Actually, this check would be stronger than what the current PR does, since the PR is "saving" the mapping as defined by governance. So governance is the one telling "this is the fee adaper for X"... but that's not a trustless guarantee.. there's no code checking that.

@martinvol @arthurgousset @pahor167

[martinvol]

I should update interface as well.

[martinvol]

this should be called when creating the whitelist, not on-by-one

[aaronmgdr]

some thoughts in case they are helpful

I was thinking about how I might go about building a tool that autoselects the best fee currency to use for a transaction.
likely one flow would be

1. fetch the list of feeCurrency Transactions fro the FeeCurrencyWhitelist.
2. check the balance of each for the address id be sending from.
3. select either the highest balance OR
4. if i know i'm interacting with a contract for a fee currency already use that one if it has a balance.

alternatively I might be building an app like minipay and choose only to support 1 feeCurrency. In which case id

• Since the Adapter already has a balanceOf function that is easy i don't need to care if address is for adapter or erc20
• The time id care about knowing underlying token vs adapter is for stage 4.

[aaronmgdr]

The way this is typically solve on programming language with strong types is to "ask" wether an interface implements some other type. Basically an instanceOf check. This you could easily do now, without any changes. You would fetch al the feeCurrency from the whitelist contract, then for each you could "try" to call getFeeAdapter() method that is present on the FeeAdapter contract and by that learn that (a) is a fee currency adapter and (b) to whom. This might be similar to 'duck typing' .. "If it walks like a duck and it quacks like a duck, then it must be a duck"

One question on this. Ive always had to supply the ABI for the contract i'm interacting with. IE I need to know if the contract has a getFeeAdapter method. i cant ask the contract. Is there some technique i'm not aware of here? (for other programing languages what you are describing is definitely something id do.

[mcortesi]

some thoughts in case they are helpful

I was thinking about how I might go about building a tool that autoselects the best fee currency to use for a transaction. likely one flow would be

1. fetch the list of feeCurrency Transactions fro the FeeCurrencyWhitelist.
2. check the balance of each for the address id be sending from.
3. select either the highest balance OR
4. if i know i'm interacting with a contract for a fee currency already use that one if it has a balance.

alternatively I might be building an app like minipay and choose only to support 1 feeCurrency. In which case id

• Since the Adapter already has a balanceOf function that is easy i don't need to care if address is for adapter or erc20
• The time id care about knowing underlying token vs adapter is for stage 4.

yes, that's the way i though about that.. and for that you don't need any of this..

only for (4), since you need to know if they are 'the same contract' and you have 2 diff addresses. But for that you can do what i suggested

[mcortesi]

The way this is typically solve on programming language with strong types is to "ask" wether an interface implements some other type. Basically an instanceOf check. This you could easily do now, without any changes. You would fetch al the feeCurrency from the whitelist contract, then for each you could "try" to call getFeeAdapter() method that is present on the FeeAdapter contract and by that learn that (a) is a fee currency adapter and (b) to whom. This might be similar to 'duck typing' .. "If it walks like a duck and it quacks like a duck, then it must be a duck"

One question on this. Ive always had to supply the ABI for the contract i'm interacting with. IE I need to know if the contract has a getFeeAdapter method. i cant ask the contract. Is there some technique i'm not aware of here? (for other programing languages what you are describing is definitely something id do.

Think something like smalltalk or python... what happens if you call a method that it's not defined? It fails with some exception.

Solidity behaves the same way.

Any "call" to a smart contract, it's actually sending a "message" to the smart contract address, which always executes the same bytecode. That bytecode parses the messages and "routes" the message to the correct method on the bytecode, but if it does not find that it fails.

You could check how the Proxy pattern works on solidity.