feat: proxies and custom factory #23

0xDiscotech · 2024-05-24T01:25:31Z

🤖 Linear

* chore: clean factory contract a bit * fix: factory errors

* feat: add salt as param * chore: polish factory contract and remove some notes

* feat: add more values on deploy values struct * fix: remove broken deploy create 2 and init function usage * chore: update deployment scripts values

* chore: polish contract and scripts

* feat: add deployment addresses returned struct * chore: update natspec based on changes * feat: create internal functions for salt logic * refactor: create common script contract and update mainnet and sepolia ones

* refactor: update some vars visibility and names on the contract * chore: update natspec based on changes

* chore: add natspec over unit tests * fix: one return value on factory contract * chore: remove unused createx contract

chore: undo changes over pkg json and remappings

* refactor: update l1 factory and scripts

…oyer * feat: create bytecode deployer * refactor: pre calculate all addresses in l1 factory constructor * refactor: update interfaces with new and needed methods * refactor: update deployment scripts based on new changes

…ded function to l1 factory * fix: imports over deleted files and unneccessary imports * chore: add some comments on vars natspec

…with create deploy on l2 * refacor: update deploy functions params and natspec

* feat: add init txs calls while sending a message and execution on l2 factory * fix: upgrade manager tests * chore: add some comments and stack vars

linear · 2024-05-24T01:25:33Z

OPT-67 Implement the solution for deploying bridged usdc and l2 adapter to the same address

dependent on spike OPT-51

UpgradeManager:

l2AdapterImplementation should be a struct with data and address
bridgedUSDCImplementation should be a struct with data and address
Create our own minimalistic factory that will use create2 to deploy all contracts
The factory will be a onetime deployment and will deploy everything in its constructor leading it deploy all contracts in one message

0xDiscotech · 2024-05-24T02:04:34Z

Deployments info (precalculated addresses on the L1 Factory deployment):

Deploying L1OpUSDCFactory ...

L1:

  L1OpUSDCFactory deployed at: 0x10F5176D7c8d78789F714f5eec37F0A01c869D58
  L1OpUSDCBridgeAdapter deployed at: 0xeaaA51C30C4848Cdd0126337D9aBee983d1Fd533
  L1 UpgradeManager deployed at: 0xd6254ebE0Fb0F01EA7f9BBea8a136460C4D5EF1b

L2:

Aliased L1 factory deployment address: 0x2206176d7c8d78789F714F5EEc37f0a01c86AE69
L2OpUSDCBridgeAdapter impl deployment address: 0x449Ded3EAdd5A5b3b8402fBf4BFB29Ce61a698De
L2OpUSDCBridgeAdapter proxy deployment address: 0x0af46d456f122a1500fda51651ad4F37a87aA729
L2 USDC proxy deployed deployment address: 0x095C1dC0071753F24bc7874ad27f9fa2D3e60098
L2 USDC implementation deployed deployment address: 0xd4d9225d89CCF3BF09B9E08E8A12ec51947255f7

The addresses were deployed to OP-Sepolia and Base Sepolia.

The addresses are the same ✅

The deployments were done using address(0) as implementation address and an empty array as initTxs on the UpgradeManager, but that doesn't matter since the init code is doesn't affect the deployed address due to the CREATE usage
Sepolia L1 deployments tx: https://sepolia.etherscan.io/tx/0xc9f4c40551a08a05acdbfaf96c9bb0f6aa45d5c5b65a02ff69b27d22c24e4069
Op Sepolia L2 deployments tx: https://sepolia-optimism.etherscan.io/tx/0x1a1a454cf685da26533e56798c27e9b26f6df16ca4427c57ef9914a1e5297df1
Base Sepolia L2 deployments tx: https://sepolia.basescan.org/tx/0xc6828db4239922964c597621e96998b8e1029d415d34c9a7eabdeeba33a75b09

0xDiscotech · 2024-05-24T02:06:06Z

foundry.toml

-[profile.default]
-solc_version = '0.8.25'
-libs = ['node_modules']
-optimizer_runs = 10_000
-


After I added this to the toml:

[profile.optimized] via_ir = true out = 'out-via-ir'

forge build asked me (with a warning) to run forge config --fix. This was removed after running that command

0xDiscotech · 2024-05-24T02:06:44Z

foundry.toml

+[profile.default.optimizer_details]
+yul = false


I needed this becasue the msize() of the BytecodeDeployer contract didn't due to Yul optimizer issues

0xDiscotech · 2024-05-24T02:09:54Z

src/contracts/L1OpUSDCFactory.sol

+ bytes memory _usdcProxyCArgs = abi.encode(L2_USDC_IMPLEMENTATION);
+ bytes memory _usdcProxyInitCode = bytes.concat(USDC_PROXY_BYTECODE, _usdcProxyCArgs);


Since the proxy of USDC will be always the same, I think is better to just encode the implementation address as unique argument and deploy by the giving init code and not the bytecode.

* feat: add args to the expect deposit tx call on test

* fix: use expect call instead of mock call to avoid foundry issues

* feat: add unit test over bytecode deployer * chore: undo toml changes

* fix: revert the bytecode deployer to the previous version * fix: update tests based on new changes

excaliborr · 2024-05-27T11:10:19Z

src/contracts/BytecodeDeployer.sol

+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.25;
+
+contract BytecodeDeployer {


Can we add natspec here describing that this is a wrapper contract etc

Also I think it would be good if this was inside utils

excaliborr · 2024-05-27T11:12:05Z

src/contracts/L2OpUSDCFactory.sol

+ * @notice Factory contract for deploying the L2 USDC implementation, proxy, and `L2OpUSDCBridgeAdapter` contracts all
+ * at once on the constructor
+ */
+contract L2OpUSDCFactory is IL2OpUSDCFactory {


Will the address of this L2 factory always be the same because of the aliased sender? If thats the case can we use CREATE2 for proxy and impl deployments, It will make the logic much simpler on L1 and cheaper to check

Yeah that'd be better. However I need to check what happens when the deployment on L2 reverts

It should only revert if the address on L2 is already occupied, meaning we have already deployed our contracts on that given chain, according to OP team the portal shouldnt revert

excaliborr · 2024-05-27T11:12:59Z

src/contracts/L2OpUSDCFactory.sol

+ emit DeployedUSDCProxy(_usdcProxy);
+
+ // Deploy L2 adapter
+ address _adapter = address(new BytecodeDeployer(_l2AdapterBytecode));


We also need the L2 Adapter proxy deployed, we have the init code for this as its just the proxy contract with impl address as param

True, good catch

src/contracts/L2OpUSDCFactory.sol

excaliborr · 2024-05-27T11:15:50Z

src/contracts/L1OpUSDCFactory.sol

+ constructor(address _usdc, address _owner) {
+ // Calculate L1 adapter
+ uint256 _thisNonceFirstTx = 1;
+ L1_ADAPTER = _precalculateCreateAddress(address(this), _thisNonceFirstTx);


For these wouldnt CREATE2 be easier? For the L1 adapter and UpgradeManager? Probably also much cheaper to precalculate the address, and simpler

src/interfaces/IUpgradeManager.sol

excaliborr · 2024-05-27T11:19:40Z

src/contracts/L1OpUSDCFactory.sol

+ * @param _portal The address of the portal contract for the respective L2 chain
+ * @param _minGasLimit The minimum gas limit for the L2 deployment
+ */
+ function deployL2UsdcAndAdapter(address _portal, uint32 _minGasLimit) external {


lets send the _messenger and get the portal from there as we will need the messenger address for permissioned checks and logic later

Where will the _messenger be needed? on L2?

I thought we would set it on an init tx when deployed

Each op-chain has a unique _l1Messenger, which has the portal accessible through a variable iirc, we will only be able to deploy for a given whitelisted _messenger which will be a check we will add in a future PR, you only need the _messenger to get the portal for now

See tasks here:
https://linear.app/defi-wonderland/issue/OPT-70/add-upgrademanager-permissioned-functions-to-the-factory

Awesome. Yes, the portal will be accessible through the messenger

* fix: tests and deploy scripts based on new changes * chore: update interface and natspec based on changes

src/contracts/L1OpUSDCFactory.sol

* refactor: update mock and expect usage on l1 factory deploy function tests due to foundry error * chore: removed unnecessary imports on mainnet script

hexshire · 2024-05-27T17:36:53Z

src/contracts/L1OpUSDCFactory.sol

+ address _upgradeManagerImplementation = address(new UpgradeManager(L1_ADAPTER));
+ // Deploy and initialize the upgrade manager proxy
+ bytes memory _initializeTx = abi.encodeWithSelector(IUpgradeManager.initialize.selector, _owner);
+ UpgradeManager(address(new ERC1967Proxy(address(_upgradeManagerImplementation), _initializeTx)));


I think using ERC1967Proxy will left an incorrect name in Etherscan or any explorer... maybe is a good idea to wrap the proxy with a better name like UpgradeManagerProxy

Is a good idea to check this when we'll be verifying the contracts. If etherscan set the name to ERC1967Proxy, then makes sense to cast it to UpgradeManager. The same happens on L2 when deploying the l2 adapter

hexshire · 2024-05-27T18:52:09Z

src/contracts/L2OpUSDCFactory.sol

+ // Execute the USDC initialization transactions
+ if (_usdcImplInitTxs.length > 0) {
+ // Initialize usdc implementation
+ for (uint256 i = 0; i < _usdcImplInitTxs.length; i++) {


Suggested change

for (uint256 i = 0; i < _usdcImplInitTxs.length; i++) {

for (uint256 i; i < _usdcImplInitTxs.length; i++) {

excaliborr · 2024-05-27T19:27:13Z

src/contracts/L2OpUSDCFactory.sol

+ emit DeployedL2AdapterImplementation(_adapterImplementation);
+
+ // Deploy L2 adapter proxy
+ bytes memory _adapterInitTx = _l2AdapterInitTxs.length > 0 ? _l2AdapterInitTxs[0] : bytes('');


Its better if we always send bytes('') this will also effect CREATE2 later, and we can call the initialization functions in the loop later

Important to keep in mind we currently call _disableInitializors in the constructor of the L2 Adapter as well, do we need this call? Doesnt need to be in this PR but I wanted to point it out

Yeah good point. I'd do everything related to the CREATE2 update on that PR, but will create a note about this. Also, I don't think we should hardcode any call on that contract but instead receiving it from the init txs array.
We should research which calls do we need to do in order to properly setup the proxy - @hexshire Do you have the answer here?

Ye I agree, we should probably remove this call and handle initialization functions in future implementations imo

100% agreed

excaliborr · 2024-05-27T19:35:09Z

test/unit/L1OpUSDCFactory.t.sol

+ */
+ function test_callBridgedUSDCImplementation(address _l1Messenger, uint32 _minGasLimit) public {
+ // Mock all the `deployL2UsdcAndAdapter` function calls
+ _mockDeployFunctionCalls(_l1Messenger);


I believe tests are failing because we are fuzzing the _l1Messenger address and foundry cant fuzz that logic cause of its vm, we can set something like this in the base

// cant fuzz this because of foundry's VM address internal _messenger = makeAddr('messenger');

* chore: don't define i to zero on loop

excaliborr

lgtm!

0xDiscotech added 24 commits May 13, 2024 16:32

feat: create first version of factory contract

a262f7c

chore: move external interfaces to external folder

dcf3621

feat: create scripts for mainnet and sepolia deployment

d84629a

* chore: clean factory contract a bit * fix: factory errors

refactor: add owner on constructor params

2376741

feat: precalculate l1 create3 proxy deployer address

a77a13d

* feat: add salt as param * chore: polish factory contract and remove some notes

fix: send usdc creation code as param

60b49cc

feat: deploy impl and proxy usdc contracts

834b949

refactor: use struct params to send more data as arg on deploy

c9dcfa7

fix: calculate guarded salt for l2

cdd0f21

* feat: add more values on deploy values struct * fix: remove broken deploy create 2 and init function usage * chore: update deployment scripts values

feat: add natspec and organize the code on the interface and lib

513c911

* chore: polish contract and scripts

refactor: update constructor args and deploy struct values

7f61441

* feat: add deployment addresses returned struct * chore: update natspec based on changes * feat: create internal functions for salt logic * refactor: create common script contract and update mainnet and sepolia ones

Merge branch 'dev' into feat/factory

d85e5b0

feat: create unit tests for the factory contract

2b669e7

* refactor: update some vars visibility and names on the contract * chore: update natspec based on changes

feat: add return values unit test

bcef49d

* chore: add natspec over unit tests * fix: one return value on factory contract * chore: remove unused createx contract

chore: remove unused portal interface

268b1b1

chore: undo changes over pkg json and remappings

chore: remove console sol import due to a bug on ci

b486001

Merge branch 'dev' into feat/factory

2034e74

Merge branch 'dev' into feat/proxies-and-custom-factory

361e86c

feat: create l2 op usdc factory

06683fa

* refactor: update l1 factory and scripts

Merge branch 'dev' into feat/proxies-and-custom-factory

0060052

refactor: remove the create deployer contract and move the unique nee…

8dcd139

…ded function to l1 factory * fix: imports over deleted files and unneccessary imports * chore: add some comments on vars natspec

refactor: send the hardcoded usdc proxy bytecode on l1 and deploy it …

8ec77da

…with create deploy on l2 * refacor: update deploy functions params and natspec

refactor: add init txs on an upgrade manager implementation struct

955c22e

* feat: add init txs calls while sending a message and execution on l2 factory * fix: upgrade manager tests * chore: add some comments and stack vars

0xDiscotech self-assigned this May 24, 2024

0xDiscotech commented May 24, 2024

View reviewed changes

0xDiscotech added 7 commits May 24, 2024 19:23

chore: update portal address on deployment scripts

7a600ce

fix: change mock all calls order on l1 factory tests due to an issue

95cfbf7

fix: use struct impl address on l1 factory instead of constant

85ab68a

* feat: add args to the expect deposit tx call on test

fix: byecode bad usage on l2 factory tests

dc6bdb2

* fix: use expect call instead of mock call to avoid foundry issues

refactor: improve bytecode deployer

3846152

* feat: add unit test over bytecode deployer * chore: undo toml changes

fix: linter warnings

558b7fb

fix: bug on l1 factory

bee19b8

* fix: revert the bytecode deployer to the previous version * fix: update tests based on new changes

0xDiscotech marked this pull request as ready for review May 25, 2024 02:29

excaliborr reviewed May 27, 2024

View reviewed changes

src/contracts/L2OpUSDCFactory.sol Show resolved Hide resolved

excaliborr reviewed May 27, 2024

View reviewed changes

src/interfaces/IUpgradeManager.sol Show resolved Hide resolved

excaliborr reviewed May 27, 2024

View reviewed changes

0xDiscotech added 3 commits May 27, 2024 12:42

chore: add natspec to bytecode deployer and move it to utils

6429925

refactor: user l1 messenger to get the portal on l1 factory

dc0527a

fix: deploy adapter impl and then proxy with it as arg on l2 factory

d2b40f4

* fix: tests and deploy scripts based on new changes * chore: update interface and natspec based on changes

hexshire reviewed May 27, 2024

View reviewed changes

src/contracts/L1OpUSDCFactory.sol Show resolved Hide resolved

chore: use address alias lib

c65580e

* refactor: update mock and expect usage on l1 factory deploy function tests due to foundry error * chore: removed unnecessary imports on mainnet script

hexshire reviewed May 27, 2024

View reviewed changes

test: add test on extra zeros returned by the bytecode deplpoyer

6dd2afb

hexshire reviewed May 27, 2024

View reviewed changes

excaliborr reviewed May 27, 2024

View reviewed changes

refactor: add l1 messenger as var on l1 factory instead of fuzzing it

6b90c67

* chore: don't define i to zero on loop

excaliborr approved these changes May 27, 2024

View reviewed changes

hexshire approved these changes May 27, 2024

View reviewed changes

excaliborr merged commit a0eb0dd into dev May 27, 2024
4 checks passed

excaliborr deleted the feat/proxies-and-custom-factory branch May 27, 2024 20:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: proxies and custom factory #23

feat: proxies and custom factory #23

0xDiscotech commented May 24, 2024

linear bot commented May 24, 2024

0xDiscotech commented May 24, 2024 •

edited

Loading

0xDiscotech May 24, 2024

0xDiscotech May 24, 2024

0xDiscotech May 24, 2024

excaliborr May 27, 2024

0xDiscotech May 27, 2024

excaliborr May 27, 2024

0xDiscotech May 27, 2024

excaliborr May 27, 2024

excaliborr May 27, 2024 •

edited

Loading

0xDiscotech May 27, 2024

excaliborr May 27, 2024 •

edited

Loading

0xDiscotech May 27, 2024

excaliborr May 27, 2024 •

edited

Loading

0xDiscotech May 27, 2024

0xDiscotech May 27, 2024

excaliborr May 27, 2024

0xDiscotech May 27, 2024 •

edited

Loading

hexshire May 27, 2024

0xDiscotech May 27, 2024

hexshire May 27, 2024 •

edited

Loading

excaliborr May 27, 2024

excaliborr May 27, 2024

0xDiscotech May 27, 2024

excaliborr May 27, 2024

0xDiscotech May 27, 2024

excaliborr May 27, 2024

0xDiscotech May 27, 2024

excaliborr left a comment

		bytes memory _usdcProxyCArgs = abi.encode(L2_USDC_IMPLEMENTATION);
		bytes memory _usdcProxyInitCode = bytes.concat(USDC_PROXY_BYTECODE, _usdcProxyCArgs);

	for (uint256 i = 0; i < _usdcImplInitTxs.length; i++) {
	for (uint256 i; i < _usdcImplInitTxs.length; i++) {

		[profile.default.optimizer_details]
		yul = false

feat: proxies and custom factory #23

feat: proxies and custom factory #23

Conversation

0xDiscotech commented May 24, 2024

🤖 Linear

linear bot commented May 24, 2024

0xDiscotech commented May 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

excaliborr May 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

excaliborr May 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

excaliborr May 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xDiscotech May 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexshire May 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

excaliborr left a comment

Choose a reason for hiding this comment

0xDiscotech commented May 24, 2024 •

edited

Loading

excaliborr May 27, 2024 •

edited

Loading

excaliborr May 27, 2024 •

edited

Loading

excaliborr May 27, 2024 •

edited

Loading

0xDiscotech May 27, 2024 •

edited

Loading

hexshire May 27, 2024 •

edited

Loading