feat: deployment script #81
Conversation
|
|
||
| contract DeployMorphoTokenBase is Script { | ||
| address public constant MORPHO_DAO = 0xcBa28b38103307Ec8dA98377ffF9816C164f9AFa; | ||
| address public REMOTE_TOKEN; |
There was a problem hiding this comment.
should be set (when the token is deployed on Ethereym)
| address public implementationAddress; | ||
| MorphoTokenOptimism public token; | ||
| address public wrapperAddress; | ||
| address public newMorphoAddress; |
There was a problem hiding this comment.
I know it doesnt' really matter but why are these in "storage"
There was a problem hiding this comment.
should we create a specific token address like the previous token?
There was a problem hiding this comment.
related discussion: #81 (comment)
There was a problem hiding this comment.
True that they could be memory
| bytes32 public IMPLEMENTATION_SALT; | ||
| bytes32 public PROXY_SALT; | ||
| bytes32 public WRAPPER_SALT; |
There was a problem hiding this comment.
Once set I'll change them to constants
| console.log("Deployed token implementation at", implementationAddress); | ||
|
|
||
| // Deploy Token proxy | ||
| token = MorphoTokenEthereum(address(new ERC1967Proxy{salt: PROXY_SALT}(implementationAddress, hex""))); |
There was a problem hiding this comment.
same remark as on base
| address public implementationAddress; | ||
| MorphoTokenOptimism public token; | ||
| address public wrapperAddress; | ||
| address public newMorphoAddress; |
There was a problem hiding this comment.
should we create a specific token address like the previous token?
There was a problem hiding this comment.
(optional) maybe now we don't even need to have 2 separate folders for base and for ethereum
| /// Contract meant to be the implementation of an ERC1967Proxy at deployment. | ||
| contract DeploymentImplementation is UUPSUpgradeable { | ||
| /// @inheritdoc UUPSUpgradeable | ||
| function _authorizeUpgrade(address) internal override {} |
There was a problem hiding this comment.
shouldn't this be gated? the actions are not atomically batched right?
There was a problem hiding this comment.
What do you mean ?
There was a problem hiding this comment.
here anybody can upgrade the contract
meaning a MEV bot could upgrade it to a contract whose implementation is not upgradable (and we are blocked)
… into feat/deployment-script
There was a problem hiding this comment.
Couldn't we run tests in the test folder as well ?
There was a problem hiding this comment.
Why not
I prefered to separate
There was a problem hiding this comment.
I mean it would be interesting to check that the deployment script deploys a token that also has the properties that are tested in the test folder. For now it only checks the total supply IIUC
There was a problem hiding this comment.
Not reviewing the constants of this file yet
|
|
||
| vm.stopBroadcast(); | ||
|
|
||
| return (address(token), wrapper); |
There was a problem hiding this comment.
We don't need the return statement, because those addresses are in storage already (token and wrapper are storage variables), and the tests inherit from this contract
| /// @custom:contact security@morpho.org | ||
| /// @dev Extension of UUPSUpgradeable. | ||
| /// | ||
| /// Contract meant to be the implementation of an ERC1967Proxy at deployment. |
There was a problem hiding this comment.
Maybe explain why it's useful, to have the same address on many chains
| bytes32 public PROXY_SALT; | ||
| bytes32 public WRAPPER_SALT; | ||
|
|
||
| address constant DEPLOYER = 0x937Ce2d6c488b361825D2DB5e8A70e26d48afEd5; |
There was a problem hiding this comment.
What is this address ? Maybe document and secure it, so we know it will be available for future deployments (to keep the same address of the token on different chains)
| bytes32 public DEPLOYMENT_IMPLEMENTATION_SALT; | ||
| bytes32 public IMPLEMENTATION_SALT; |
There was a problem hiding this comment.
No need to use CREATE2 for the implementation, as we don't care about having a predictable address for the implementation
Co-authored-by: Quentin Garchery <QGarchery@users.noreply.github.com> Signed-off-by: Merlin Egalite <44097430+MerlinEgalite@users.noreply.github.com>
Fixes #54