For example the ERC2771ContextUpgradeable contract inherits initializable but does not have an initializer. Instead, it has a traditional constructor function defined. Is this just because the team is still working on porting all the contracts to the upgradeable version, or is there something special about ERC2771Context?
If we want a contract to inherit ERC2771Context and also be upgradeable, what would be the recommended way to do this?
All our upgradeable contracts inherit initializable regardless of whether or not they have an initializer. This is to avoid any linearisation issues.
ERC2771ContextUpgradeable is a very specific contract. Our position is that, while ERC2771 could be used by upgradeable contract, we feel its very likely that multiple proxies sharing the same implementation will also be sharing the same forwarder.
This means we can "hardcode" the forwarder directly in the implementation using immutable storage. This will save on gas cost when initializing the proxy, and also reduce gas costs when querying _msgSender() (which is a very frequent operation).
The workflow is to set the forwarder when deploying the implementation (through the constructor arguments) and then initialize the rest of the logic at the proxy level.
If for any reason you want to set different forwarders for different proxies ... then you should not be using the provided ERC2771ContextUpgradeable and instead write your own version (this should be really simple). Not that it would make all operations more expensive.
But when I call upgrades.deployProxy, how can I pass the trustedForwarder to the constructor? I think it's only passing the argument I gave to the initializer but not the constructor: await upgrades.deployProxy(MyContract, [forwarder.address], { kind: 'uups' });
Edit: Solved my own problem: need to add some additional fields to the last argument:
Hello @Amxx but what if I have deployed a first version (V1) contract as @EgyptianCactus mentions and then I try to upgrade the implementation creating a V2 contract that inherits V1? In that case I'm forced to use parent's constructor that tries to modify the immutable _trustedForwarder and that will fail.
When deploying your V2, you'll have to provide the constructor argument for the underlying V1 that you inherit. They don't necessarily have to match the ones that you provided when deploying V1. (though if you change them, you better make sure this is not going to break things)
This works for me thank you very much for sharing! what are the consequence of using the flag unsafeAllow? @Amxx do you think is safe to use in a production context?
@giova_colo The consequence of using unsafeAllow: ["constructor"] in the deployProxy function's arguments is that it allows constructors anywhere in the contract and its dependencies. If you want to be more granular, you can use the NatSpec annotation /// @custom:oz-upgrades-unsafe-allow constructor above a specific constructor instead, for example you can see this is being used in ERC2771ContextUpgradeable.sol. That way, you don't need to pass in unsafeAllow: ["constructor"] to deployProxy.