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

Add support for ConsenSys multisig wallet #6153

Merged
merged 8 commits into from
Aug 9, 2017

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Jul 26, 2017

This PR adds support for this multisig wallet. It should also be easier to add support for other Wallets now.

It also fixes a bug in the Transfer modal when sending a token from a Wallet that has no ETH.

NB : there might be some gas estimation issues when for example confirming an owner addition. As this version of the Multisig Wallet doesn't throw when the executed call fails, the gas estimation is a bit undervalued.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jul 26, 2017
@jacogr
Copy link
Contributor

jacogr commented Aug 3, 2017

Looks good. Spend a lot of time on the new calculations as well, just double-checking the logic - was not 100% sure why it was re-done here without splitting it out, makes the PR bigger and the logic much more fragile and difficult to follow.

Would really need your help to port this to UI-2 as well, where everything now lives in separate repos (dapp-wallet is mostly affected, contract stubs would end with js-shared, nothing should change in the parity repo itself, i.e. all affected files here are now externally). Not sure it is possible for me to do this port without breaking things.

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 3, 2017
@gavofyork gavofyork merged commit d6eb053 into master Aug 9, 2017
@gavofyork gavofyork deleted the ng-consensys-multisig-wallet branch August 9, 2017 17:06
ngotchac added a commit that referenced this pull request Aug 18, 2017
* First draft of ConsenSys wallet

* Fix transfer store // WIP Consensys Wallet

* Rename walletABI JSON file

* Fix linting

* Fix wrong daylimit in wallet modal

* Confirm/Revoke ConsensysWallet txs

* Linting

* Change of settings for the Multisig Wallet
@ngotchac ngotchac mentioned this pull request Aug 18, 2017
arkpar pushed a commit that referenced this pull request Aug 18, 2017
* Time should not contribue to overall status. (#6276)

* Add warning to web browser and fix links. (#6232)

* Extension fixes (#6284)

* Fix token symbols in extension.

* Allow connections from firefox extension.

* Add support for ConsenSys multisig wallet (#6153)

* First draft of ConsenSys wallet

* Fix transfer store // WIP Consensys Wallet

* Rename walletABI JSON file

* Fix linting

* Fix wrong daylimit in wallet modal

* Confirm/Revoke ConsensysWallet txs

* Linting

* Change of settings for the Multisig Wallet
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.

3 participants