Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Lower gas usage for creating a Multisig Wallet #3773

Merged
merged 5 commits into from
Dec 9, 2016

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Dec 9, 2016

This introduces a new Multisig Wallet that only uses about 500k gas to deploy (compared to ~1.8 to 2M gas for the previous version).

This uses a Multisig Wallet Library that has to be deployed to the network (if not, it fallbacks to the old one).
One has been deployed to Ropsten already. To deploy a new one:

  • Get the bytecode in js/src/contracts/code/wallet.js
  • Deploy it (if using the UI, ABI set to [] is fine)
  • Register the contract address in the Registry with the key specified in js/src/contracts/code/wallet.js : walletLibrary

For new versions, when the Wallet is compiled, modify in the bytecode the references to cafecafe... with __WalletLibrary___________

The new contract is in js/src/contracts/snippets/enhanced-wallet.sol

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Dec 9, 2016
@jacogr
Copy link
Contributor

jacogr commented Dec 9, 2016

We should be doing the library deploys using the Parity keys. So when you are happy, let me know and I can deploy & register using the required keys.

I'm assuming there is no issues across multiple networks, e.g. Ropsten & Homestead from a UI & knowing where to find stuff perspective? (Doesn't look like there is, just confirming.)

import { ERROR_CODES } from '~/api/transport/error';
import { wallet as walletAbi } from '~/contracts/abi';
import { wallet as walletCode } from '~/contracts/code';
import { walletLibraryRegKey, fullWalletCode } from '~/contracts/code/wallet';
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot combine this import with the previous one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@ngotchac
Copy link
Contributor Author

ngotchac commented Dec 9, 2016

Okay sure. I registered it on Ropsten, so you can test. When everything is fine, I'll transfer the ownership of the registered key on Ropsten.
Nothing network related on the UI.

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 9, 2016
@gavofyork
Copy link
Contributor

there's ethcore/contracts repo for any contract stuff we do...

@gavofyork gavofyork merged commit b994037 into master Dec 9, 2016
@gavofyork gavofyork deleted the ng-enhanced-multisig branch December 9, 2016 18:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants