Skip to content

MUL-620 - Change GSNMultisigFactory to replace the Factory pattern by a Clone Factory pattern (proxy)#8

Open
gjeanmart wants to merge 2 commits intomasterfrom
MUL-620-clone-factory-pattern
Open

MUL-620 - Change GSNMultisigFactory to replace the Factory pattern by a Clone Factory pattern (proxy)#8
gjeanmart wants to merge 2 commits intomasterfrom
MUL-620-clone-factory-pattern

Conversation

@gjeanmart
Copy link

@gjeanmart gjeanmart commented Aug 25, 2020

Overview

The purpose of this PR is to upgrade our GSNMultisigFactory smart-contract to use a Clone Factory pattern rather than a simple Factory pattern. Instead of deploying a new GSNMultisig contract for each creation, the Clone Factory will deploy a minimal contract only called Proxy connected to a unique contract called Master Copy (composed of the logic).

Impact

This doesn't have any impact on existing accounts.

Upgrade procedure

1. Deploy the master copy

$ npx oz create GSNMultiSigWalletWithDailyLimit --network rinkeby
Nothing to compile, all contracts are up to date.
All contracts are up to date
? Call a function to initialize the instance after creating it? Yes
? Select which function * initialize(_owners: address[], _required: uint256)
? _owners (address[]): 0x<DEPLOYER_ADDRESS>
? _required (uint256): 1
✓ Instance created at 0x77CCA5B37d17e7D94a28FC220F92A5504eefE9F4
0x77CCA5B37d17e7D94a28FC220F92A5504eefE9F4

Note the Master Copy address: 0x77CCA5B37d17e7D94a28FC220F92A5504eefE9F4

Change the owner to 0x0000000000000000000000000000000000000000 to make sure nobody can use this multisig.

$ npx oz send-tx --network rinkeby
? Pick an instance GSNMultiSigWalletWithDailyLimit at 0x77CCA5B37d17e7D94a28FC220F92A5504eefE9F4
? Select which function submitTransaction(destination: address, value: uint256, data: bytes)
? destination (address): 0x77CCA5B37d17e7D94a28FC220F92A5504eefE9F4
? value (uint256): 0
? data (bytes): 0xe20056e6000000000000000000000000<DEPLOYER_ADDRESS>0000000000000000000000000000000000000000000000000000000000000000
✓ Transaction successful. Transaction hash: 0x5d7828e38c1b24a637e02b8d0958e95faf9e60affff2116df6d6fbd2acd7b97f
Events emitted:
 - Submission(0)
 - Confirmation(0xF0f15Cedc719B5A55470877B0710d5c7816916b1, 0)
 - OwnerRemoval(0xF0f15Cedc719B5A55470877B0710d5c7816916b1)
 - OwnerAddition(0x0000000000000000000000000000000000000000)
 - Execution(0)

2. Change the code of the factory using CloneFactory

See GSNMultisigFactory.sol

3. Upgrade the Factory

$ npx oz upgrade --network rinkeby
(...)
? Which instances would you like to upgrade? Choose by name
? Pick an instance to upgrade GSNMultisigFactory
? Call a function on the instance after upgrading it? No
✓ Instance upgraded at 0xbBE9DD161ebfE7D792f4844a0C10aD7c1C3994e9. Transaction receipt: 0xbdb1f46a51b66d17036b92a8a0913fc46bd595b52dc8aefd0b0ed5de5a7b99c2
✓ Instance at 0xbBE9DD161ebfE7D792f4844a0C10aD7c1C3994e9 upgraded

4. Set the master copy

$ npx oz send-tx --network rinkeby
? Pick an instance GSNMultisigFactory at 0xbBE9DD161ebfE7D792f4844a0C10aD7c1C3994e9
? Select which function setMasterCopy(_masterCopy: address)
? _masterCopy (address): 0x77CCA5B37d17e7D94a28FC220F92A5504eefE9F4
✓ Transaction successful. Transaction hash: 0x72d12fb440fa462dfe4298c7f551dd17d59f29e9107a794c73db44d20b93de29

Testing strategy

TBD

Opened questions

  • Shall we take the opportunity to upgrade the code to soliidity > 0.6.0

@gjeanmart gjeanmart added the enhancement New feature or request label Aug 25, 2020
@gjeanmart gjeanmart requested a review from teawaterwire August 25, 2020 18:18
@gjeanmart gjeanmart changed the title MUL-620 - Change GSNMultisigFactory to use the Proxy pattern via a cloneFactory lib MUL-620 - Change GSNMultisigFactory to replace the Factory pattern via a Clone Factory pattern (proxy) Aug 25, 2020
@gjeanmart gjeanmart changed the title MUL-620 - Change GSNMultisigFactory to replace the Factory pattern via a Clone Factory pattern (proxy) MUL-620 - Change GSNMultisigFactory to replace the Factory pattern by a Clone Factory pattern (proxy) Aug 25, 2020
GSNMultiSigWalletWithDailyLimit multisig = new GSNMultiSigWalletWithDailyLimit();
multisig.initialize(_owners, _required, _dailyLimit);
wallet = address(multisig);
function create(address[] memory _owners, uint _required, uint _dailyLimit) public payable returns (address payable wallet) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if create has to be payable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants