search

coder5876 / simple-multisig

Simple multisig for Ethereum using detached signatures
MIT License
247 stars 108 forks source link

Why not hardcode values? #9

Closed neuhaus closed 6 years ago

neuhaus commented 7 years ago

The goal is for the contract to be "designed to be as simple as possible". Wouldn't it be advantageous to hardcode the ownersArr and threshold values instead of specifying them as parameters to the constructor? :woman_shrugging: It would make the code even simpler.

christoph2806 commented 7 years ago

for a sample implementation of this, see https://github.com/etherisc/simplestWallet

neuhaus commented 7 years ago

Great start! Right now in simplestWallet the code in the execute function has to be changed whenever the number of owners and/or the threshold are changed. Changing it to deal with this automatically would remove a potential source of errors.

coder5876 commented 7 years ago

Interesting approach! 😃

Hardcoding these values would indeed make the complexity of the constructor go away. Not sure how to deal with that in terms of formal semantics, since I feel you’d have to create a formal specification for each contract with hard coded values 🤔

@christoph2806 nice implementation! Although the indices in the signatures are off by one (goes from 1 to 3 instead of 0 to 2)

christoph2806 commented 7 years ago

@christianlundkvist fixed it - thank you for heads up! Regarding formal spec: I think you could write a meta-spec which is generic and then you have a template for a proof (like proof by induction). @neuhaus The idea is that the wallet is so small that creating a new one is cheap and simple. A UX could completely hide this complexity from the user, giving him the usual "addOwner" / "removeOwner" / "changeThreshold" functionality. This implementation is thought as a starting point for an insurable wallet, it can't be simple enough for that.

neuhaus commented 7 years ago

OK. I think we should strive for a program that asks your for the parameters (owners, threshold) and generates the custom contract. If we generate the contract, we can vary the number of parameters passed to execute() instead of passing arrays (sigV, sigR, sigS), too. However that would complicate the counterparts.

Checking for duplicate owner addresses should also be done in the contract generator.

coder5876 commented 6 years ago

@christoph2806 @neuhaus I'm personally feeling like hardcoding values is taking things a bit too far, and I do think it will be harder to formally verify using this. It's probably more error prone that people have to fiddle with the source code manually in order to hard code these values.