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

First draft of the MultiSig Wallet #3700

Merged
merged 26 commits into from Dec 6, 2016
Merged

First draft of the MultiSig Wallet #3700

merged 26 commits into from Dec 6, 2016

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Dec 1, 2016

Would hopefully close #3282 for now...

This is a first draft and a WIP for support of multi-signatures wallets (attaching existing ones and creating new ones)....

@ngotchac ngotchac added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M7-ui labels Dec 1, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 83d4d8a on ng-mutisig-wallet into ** on master**.

padding: 0.75em 0.5em 0;

> * {
// height: 32px;
Copy link
Contributor

Choose a reason for hiding this comment

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

?

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Dec 5, 2016
@gavofyork
Copy link
Contributor

(for a separate PR) would be good to have a secondary wallet option of https://github.com/ethcore/contracts/blob/master/BasicWallet.sol

@ngotchac
Copy link
Contributor Author

ngotchac commented Dec 5, 2016

@gavofyork Yes, we talked about this with @jacogr ; it will be available

@jacogr
Copy link
Contributor

jacogr commented Dec 5, 2016

@ngotchac As discussed, functionality-wise there are some gaps (e.g. existing contract attachment, simple parameters, editing of options) and there are some UX nigglies along the way (layout inconsistencies, information better placed elsewhere). All these we chatted about.

However since we already trimmed this one down far as possible, it probably wouldn't make sense to get that in since it makes it un-reviewable again. So my suggestion is to get it in in the current form, which will also allow for some wider functionality review.

Will just triage with you again on my list of items and compare against yours so I don't log things unnecessarily.

@gavofyork
Copy link
Contributor

@jacogr mark looks good, then?

@gavofyork
Copy link
Contributor

functionality-wise, it's pretty sound.

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 6, 2016
@jacogr jacogr merged commit bec3539 into master Dec 6, 2016
@jacogr jacogr deleted the ng-mutisig-wallet branch December 6, 2016 08:38
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.

Proper multisig wallet support
4 participants