What would be the impact of delegate on the account manager approve system?

[MerlinEgalite]

OpenZeppelin/openzeppelin-contracts#2852

After reflecting on this. I think that if Morpho is deploying 2 instances (one general and the other for a specific e-mode), with both versions pointing to the same implementation (scenario with a high likelihood tbh). It would be possible to re-use the manager's approval of one version to the other, can you confirm?

[Rubilmax]

An e-mode specific version of Morpho is implementation-specific, because we use immutables. Why would we point both Morpho versions to the same implementation?

[MerlinEgalite]

True! Is there another way we could have issues though? I'm not 100% sure yet that we could not have any issue

[pakim249CAL]

Yes, we need to add a salt to the calculation of the domain separator. Each implementation contract should have its own salt as a constructor argument. We can have the salt be the emode category ID. The salt will fulfill the requirement of needing different signatures for different morpho instances.

[QGarchery]

Do we really need to add a salt ? I mean the address of the contract is already in the domain separator, so it seems like we already require different signatures for different Morpho instances

[MerlinEgalite]

An attack could be someone deploying a Morpho instance pointing to the same implementation contract, with a fake frontend collecting signatures that could be replayed on the real Morpho

[pakim249CAL]

Do we really need to add a salt ? I mean the address of the contract is already in the domain separator, so it seems like we already require different signatures for different Morpho instances

Actually I am investigating and you are right. But I am now actually concerned for a different reason. The domain separator should not be dependent on the address of the implementation contract. This creates a situation that any contract upgrade would invalidate our previous domain separator. The signatures should be based on the proxy contract, meaning that the domain separator should probably be either initialized in storage, or calculated on runtime if we don't want to reserve a storage var for this.

[MerlinEgalite]

That was the point of my previous message haha and I agree we should change it, thx for explaining it better