Add generalized meta tx contract

[MerlinEgalite]

Fixes #80

To do/answer:

• [ ] Verify that safety checks are not missing
• [x] Should we make MetaTransactionUpgradeable not to inherit from ContextMixin?
• [ ] Should MetaTransactionUpgradeable have gaps at the end?
• [ ] Add tests

[MerlinEgalite]

Ready for review to collect feedbacks (I'll add tests and comments)

[MerlinEgalite]

I'm not sure to understand why the forwarder is inheriting the context mixin, and am a bit confused

If I'm not mistaken, the context mixin's purpose is to be integrated in a meta-tx-ready protocol (such as Morpho), so that the protocol can query which sender the tx is from, and in a safe manner in the case msg.sender is a relayer?

But then why does the context mixin check if msg.sender == address(this) because it's not supposed to be used within the context of the relayer, and rather check whether the sender is a trusted relayer or not

Btw, do we have plans to trust specific relayers? Or deploy our own?

This is one of the question. We can separate both tbh.

The sender must be the address(this) as the relayer should have called executeMetaTransaction before and then the called is sent from Morpho itself. If msg.sender != address(this) then the original sender did not trigger executeMetaTransaction. I don't think we'll need trusted relayer as in EIP2771 since in our case we want something more general anyone can sign a message to allow another account to do things on its behalf. It's up to the user to do it or not. Is it clearer?

[Rubilmax]

If we want to develop an efficient forwarder + context according to EIP2771, then we should separate it to follow the EIP

If we don't stick to the workflow drawn in the EIP, then it's a more specific use-case that we should name otherwise, without referencing EIP2771 as it can be confusing (because the call stack would not be the same)

Aside from this, I don't understand the usecase you designed. Is it possible to have a schema of the desired call flow? Including the Morpho protocol, the user, the forwarder (if any) and the type of interactions involved between stakeholders (calls/signatures)?

[MerlinEgalite]

If we want to develop an efficient relayer + context according to EIP2771, then we should separate it to follow the EIP

At the start this is not the purpose. The purpose is to implement the account manager on Morpho-AaveV3 as proposed by @pakim249CAL and not the EIP2771. Do you mean that the naming is misleading?

Aside from this, I don't understand the usecase you designed. Is it possible to have a schema of the desired call flow? Including the Morpho protocol, the user, the relayer (if any) and the type of interactions involved between stakeholders (calls/signatures)?

The flow would be:

• Alice signs a message allowing Bob to supply on her behalf.
• Bob call executeMetaTransaction passing the correct arguments AND the message signed.
• The call is checked thanks to the logic in this PR.

Perhaps I'm missing something though, in this case @pakim249CAL might help.

[Rubilmax]

Ok so first, I think we are mis-understood because I replaced forwarder with relayer in my previous comments. This is now fixed.

Do you mean that the naming is misleading?

I'd instead think it's just that I saw this PR and looked after what work was done on the meta tx subject so I acknowledged we were sticking to standards (which in this case is the EIP) So in the end it's a lack of communication / I didn't check the correct resource to review this PR

---

The flow you presented looks like the flow designed in the EIP, where Alice is the sender and Bob is the forwarder However, the EIP is suggesting:

1. to separate the context verifier & the forwarder
2. to only trust a specific set of forwarders

Here's my analysis:

1. separating allows to save bytecode size ; I don't see what advantage merging provides
2. I don't get why the EIP states that the signer address could be manipulated, because it's verified using the signature (I guess it has something to do with ecrecover vulnerabilities?)

What I understand from our discussion is that you'd like to merge the forwarder & the context verifier. What advantage does it provide? The EIP does allow anyone to be a relay and submit an off-chain signed tx on behalf of someone else, through a forwarder (which in the case of the EIP, should be trusted - but if we want to introduce a trustless forwarder system then I'd say we're jumping into vulnerabilities)

[MerlinEgalite]

1. separating allows to save bytecode size ; I don't see what advantage merging provides

I've separated them.

2. I don't get why the EIP states that the signer address could be manipulated, because it's verified using the signature (I guess it has something to do with ecrecover vulnerabilities?)

I think so but I need to double-check.

What I understand from our discussion is that you'd like to merge the forwarder & the context verifier. What advantage does it provide? The EIP does allow anyone to be a relay and submit an off-chain signed tx on behalf of someone else, through a forwarder (which in the case of the EIP, should be trusted - but if we want to introduce a trustless forwarder system then I'd say we're jumping into vulnerabilities)

In our case we'll need to check in the Morpho contracts that the overridden value of msg.sender (returned by _msgSender) has power over the from address as part of the allowances check.

[Rubilmax]

I've separated them.

I disagree: currently, MetaTransactionUpgradeable (the forwarder) inherits from ContextMixin (the context verifier), just as if we were expecting the integrator to use the context mixin within the forwarder, which is not what the EIP suggests (even beyond the EIP: why would the forwarder need to access the tx signer with the same logic used to access the tx signer at the protocol level?)

To make it clearer:

• Morpho should inherit ContextMixin (as you stated it), and check for the trusted forwarder (for security purposes, but we need to dig this)
• the forwarder should be completely independent from the protocol, because it has nothing to do with the protocol (it's just a forwarder), and thus should not inherit verification logic (namely: the check for the trusted forwarder) ; so I would remove the inheritance because the forwarder should never access the signer the way the protocol (the application) does via ContextMixin

[MerlinEgalite]

updated bad network

[Rubilmax]

Here's a summary so far:

• we want to implement a all-in-one context verifier & forwarder

It allows to only have a single address to interact with (Morpho's), enables trust less forwarding (the protocol itself being the forwarder) as well as saving a CALL (compared to the EIP)

However, it includes the forwarding logic into the protocol

I'm not sure if I'm against or in favor of it What I'm sure of is that if we go for this solution, then I agree MetaTransactionUpgradeable should inherit ContextMixin

[MerlinEgalite]

Here's a summary so far:

• we want to implement a all-in-one context verifier & forwarder

It allows to only have a single address to interact with (Morpho's), enables trust less forwarding (the protocol itself being the forwarder) as well as saving a CALL (compared to the EIP)

However, it includes the forwarding logic into the protocol

I'm not sure if I'm against or in favor of it What I'm sure of is that if we go for this solution, then I agree MetaTransactionUpgradeable should inherit ContextMixin

Ok if I get it right, we must use the EIP and set trustedForwarder as address(this) in our case. And then the Mixin would check whether msg.sender is equal to the trustedForwarder or not. Is that correct?

[pakim249CAL]

I mostly agree with @Rubilmax but I think this solution is also workable. If this is the solution chosen, then the logic in the Morpho contract needs to filter based on msg.sender being the forwarder. But the Morpho contract can also function as its own forwarder with an additional external entry point which I would prefer.

[MerlinEgalite]

I mostly agree with @Rubilmax but I think this solution is also workable. If this is the solution chosen, then the logic in the Morpho contract needs to filter based on msg.sender being the forwarder. But the Morpho contract can also function as its own forwarder with an additional external entry point which I would prefer.

There are 3 solutions (in the context of meta tx):

1. Use the EIP completely and the trusted forwarder is external to Morpho.
2. Use the EIP. Morpho inherits it and the ContextMixin (additional call) and the forwarder is set to the Morpho address.
3. Use this current version that Morpho inherits (as well as the ContextMixin -- to merge? --) to remove this extra call.

Wdyt?

[pakim249CAL]

I'm in favor of solution 3, as it would be redundant in our case to store a forwarder when we know it will be address(this).

[MerlinEgalite]

I'm in favor of solution 3, as it would be redundant in our case to store a forwarder when we know it will be address(this).

Ok and about merging with the ContextMixin? Should we keep them separate? Or should one contract inherits the other?

[pakim249CAL]

Since Morpho is the contract that needs to modify msg.sender, it should be the Morpho contract inheriting ContextMixin. The forwarder contract should only be responsible for appending the right msg sender to the calldata.

[Rubilmax]

Since Morpho is the contract that needs to modify msg.sender, it should be the Morpho contract inheriting ContextMixin. The forwarder contract should only be responsible for appending the right msg sender to the calldata.

Disagreed because the ContextMixin in this case is correct only because it's supposed ti be inherited by the forwarder (otherwise the check msg.sender == address(this) wouldn't mean anything if the ContextMixin is inherited by anything else than the forwarder) So these 2 should be merged to me, without distinction

Or renamed to ForwarderContextMixin but what's the point of separating them if they are supposed to be inherited by the same contract?

[pakim249CAL]

Since Morpho is the contract that needs to modify msg.sender, it should be the Morpho contract inheriting ContextMixin. The forwarder contract should only be responsible for appending the right msg sender to the calldata.

Disagreed because the ContextMixin in this case is correct only because it's supposed ti be inherited by the forwarder (otherwise the check msg.sender == address(this) wouldn't mean anything if the ContextMixin is inherited by anything else than the forwarder) So these 2 should be merged to me, without distinction

Or renamed to ForwarderContextMixin but what's the point of separating them if they are supposed to be inherited by the same contract?

I'm not really sure what you mean. I'm pretty sure the ContextMixin does not need to be used by a forwarder at all. This is how I understand the flow of generalized metatransactions to be:

Let's suppose we have addresses: A: The Morpho contract B: The forwarder contract C: The caller of the contract

The job of the forwarder is to accept a typed struct that has all the parameters needed to call a function, and verify an accompanying signature of C. After signature verification, the forwarder calls the function on the designated target but with address(C) appended to the calldata.

This is just a regular call, so msg.sender when the function is called in contract A is now address(B). At this point, contract A must do additional processing to extract msg.sender from the appended calldata if msg.sender is address(B).

This is the only place where ContextMixin comes into play. Contract A is the only contract that needs to extract msg.sender from the calldata. The forwarder contract only has the job of appending the correct msg.sender to the calldata.

The reason I suggest doing it all in the Morpho contract is because there is actually no reason that A and B have to be distinct. Instead of keeping track of address(B) in contract A, contract A can simply use address(this) if it is it's own forwarder. In this case, the forwarder would have access to ContextMixin simply by virtue of being in the Morpho contract, but it should not be used in the forwarder at all.

[MerlinEgalite]

This last message seems to me to be the solution 1, am I missing something?

Btw there are some limitations that are worrying for our purposes:

• a contract cannot sign a message, so a contract cannot delegate its management to another account
• a signed message is available only once

Given that I'm not sure anymore that meta tx is the correct way to do it

[Rubilmax]

Agreed @pakim249CAL

My point was: if Context is referring to address(this) instead of a stored forwarder address, it only works because the context is used within the forwarder! So we should not separate the context from the forwarder in this case, because the context is not supposed to be inherited by anything else than the forwarder in this case (and may not work so because it's checking against address(this) which isn't necessarily the forwarder)

[MerlinEgalite]

Re-opening it because I think it can still be cool add it to the repo. I'll clean things if necessary

[Rubilmax]

I believe it's a bad design to couple application & meta tx execution. I do believe the segregation of roles (between the application - the recipient contract - & the forwarder) is specifically intended in EIP-2771, to avoid coupling the 2 (e.g. an immutable forwarder contract could be deployed and trusted by all meta-tx-compliant applications on mainnet, without having to integrate a specific forwarder in each application)