In this contract:

pragma solidity ^0.4.18;

// This contract is only used for testing purposes.
contract TestRegistry {

  mapping(address => uint) public registry;

  function register(uint x) payable public {
    registry[msg.sender] = x;
  }

}

What exactly is stopping me from calling function register directly???

Isn't the purpose of Multi-Sig to restrict permission further than onlyOwner?

In this example, the function is not restricted even to onlyOwner!

I believe that it would be advisable to:

1. Import Ownable.sol
2. Inherit from Ownable
3. Declare function register as onlyOwner

Then, in simplemultisig.js, after the line

let reg = await TestRegistry.new({from: accounts[0]})

Add the line:

await reg.transferOwnership(multisig.address);

Side-note - during execution, an exception is thrown at:

await multisig.execute(sigs.sigV, sigs.sigR, sigs.sigS, reg.address, value, data, {from: accounts[0], gasLimit: 1000000})

So I have yet to figure out the entire process, but I nevertheless believe that this is the type of capability which should be demonstrated here (i.e., calling a previously-only-owner function via the MultiSig contract).

Update on the side-note above:

The error was thrown because I removed the payable keyword from the declaration of function register.

After "re-enforcing" it, the test runs to completion.

Was there reasoning for setting the maximum number of owners to 10? I noticed the value was changed here from 20 to 10 a while back. Consider adding some documentation around that choice.

executor right?

I have downloaded code in zip and extracted the project.
I have installed ganache-cli and truffle.
After,
#ganche-cli (running in one window),
#npm run test (not successful, showed below error)

Error: Cannot find module '/usr/local/lib/node_modules/truffle/node_modules/sqlite3/lib/binding/node-v64-linux-x64/node_sqlite3.node' at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15) at Function.Module._load (internal/modules/cjs/loader.js:562:25) at Module.require (internal/modules/cjs/loader.js:692:17) at require (internal/modules/cjs/helpers.js:25:18) at Object.<anonymous> (/usr/local/lib/node_modules/truffle/node_modules/sqlite3/lib/sqlite3-binding.js:4:15) at Module._compile (internal/modules/cjs/loader.js:778:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10) at Module.load (internal/modules/cjs/loader.js:653:32) at tryModuleLoad (internal/modules/cjs/loader.js:593:12) at Function.Module._load (internal/modules/cjs/loader.js:585:3) at Module.require (internal/modules/cjs/loader.js:692:17) at require (internal/modules/cjs/helpers.js:25:18) at Object.<anonymous> (/usr/local/lib/node_modules/truffle/node_modules/sqlite3/lib/sqlite3.js:2:15) at Module._compile (internal/modules/cjs/loader.js:778:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10) at Module.load (internal/modules/cjs/loader.js:653:32) at tryModuleLoad (internal/modules/cjs/loader.js:593:12) at Function.Module._load (internal/modules/cjs/loader.js:585:3) at Module.require (internal/modules/cjs/loader.js:692:17) at require (internal/modules/cjs/helpers.js:25:18) at Object.<anonymous> (/usr/local/lib/node_modules/truffle/node_modules/websql/lib/sqlite/SQLiteDatabase.js:3:15) at Module._compile (internal/modules/cjs/loader.js:778:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10) at Module.load (internal/modules/cjs/loader.js:653:32) at tryModuleLoad (internal/modules/cjs/loader.js:593:12) at Function.Module._load (internal/modules/cjs/loader.js:585:3) at Module.require (internal/modules/cjs/loader.js:692:17) at require (internal/modules/cjs/helpers.js:25:18) Truffle v5.2.1 (core: 5.2.0) Node v10.19.0 npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! [email protected] test: truffle test`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR! /root/.npm/_logs/2021-03-01T11_34_09_513Z-debug.log
`

In a scenario where there are multiple messages being collected signatures for there is a race condition to update the nonce. Logically the simplest sequence is as follows:

• nonce = 0, derive message hash X and start collecting signatures off chain
• nonce = 0, derive message hash Y and start collecting signatures off chain
• collect signatures for X and execute successfully => nonce updated to 1
• collect signatures for Y and execute => fails

I don't see a reason why to limit the number of signatures to threshold.
It could be a common case that more owners than threshold decide to sign a transaction, e.g. because they want to express their consensus with the transaction.

I would therefore propose to change require(sigR.length == threshold); into require(sigR.length >= threshold);

https://github.com/christianlundkvist/simple-multisig/blob/71136f4d9c8c1352ceefaf96dd2ed480b8e5fb60/contracts/SimpleMultiSig.sol#L24

Sorry to ask so stupid question on github. I‘m new to eth&contract, i want to implement multisig with simple-multisig. But i do not know how to modify the code in “SimpleMultiSig.sol”. At least i think "EIP712DOMAINTYPE_HASH", "TXTYPE_HASH" can be change to mine, but i do not kwon how to generate them. Although i have the metamask and with account open，“browsertest” can't work with the error “web3” can't found.

i find https://ipfs.io/ipns/simplemultisig.io/wallet, it not work too.

please tell me what is "EIP712DOMAINTYPE_HASH", "TXTYPE_HASH" or tell me where can i find the function "EIP712Domain" and "MultiSigTransaction"

It's not clear what this piece of code does.

address lastAdd = address(0); 
for (uint i = 0; i < owners_.length; i++) {
  require(owners_[i] > lastAdd);
  isOwner[owners_[i]] = true;
  lastAdd = owners_[i];
}

Why do you require(owners_[i] > lastAdd);? Are you checking that the address array is sorted (this should be document above the constructor)? Why? Is it to prevent duplicate addresses?

This line, but also others:

require(owners_.length <= 10 && threshold_ <= owners_.length && threshold_ != 0);

... collates several checks into one, which IMO is a bad practice (the expressions eventually grow too complex); and, more importantly, will hinder providing a useful failure message when that becomes available.

Instead of the current signed payload, switch to using the newly finalized EIP712 standard.

Here is a good Solidity example to follow:

https://github.com/ethereum/EIPs/blob/master/assets/eip-712/Example.sol

In the constructor, it is required that "threshold_ >= 0" other than "threshold_ > 0" as follows:
require(owners_.length <= 10 && threshold_ <= owners_.length && threshold_ >= 0);
I can't think of a use case for threshold == 0.

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? 🤷‍♀️
It would make the code even simpler.

From a reddit post of @naterush:
The multi-sig specified allows failed transactions to be replayed.

To see why, check out this line. If the sub-call is not successful, the multi-sig will throw. This means the nonce is not updated if the sub-call throws, reverts, or runs out of gas.

Thus, all failed transactions can be replayed by anyone in the world (just by watching the multi-sig) at any point in time, up until the next successful transaction is executed by this multisig. This very much defies the expectations we come to expect from an Ethereum tx.

https://github.com/christianlundkvist/simple-multisig/blob/71136f4d9c8c1352ceefaf96dd2ed480b8e5fb60/test/simplemultisig.js#L3

Currently, it's not possible to presign transactions with web3.eth.sign because you can't pass a hex string to the signing function.

Or any ideas how to to so?

At present, Truffle uses Web3 v0.18.4, which in turn uses BigNumber v2.0.0 (or similar).

This means that the Web3 and BigNumber modules are tightly-coupled within Truffle.

Hence, in your Truffle test (file simplemultisig.js), you should replace every new BigNumber instance passed to a web3 function, with a call to web3.toBigNumber (which creates a BigNumber instance compatible for the Web3 version used by Truffle).

Failure-scenario example:

I am installing:

• "bignumber.js": "^7.0.0"
• "truffle": "4.1.11"

Where bignumber.js is used in some of my tests for "non-truffle" purposes.

Function web3.eth.sendTransaction expects a value argument of type BigNumber v2.0.0, which implements function lessThan.

However, in BigNumber v7.0.0, the function has been renamed to isLessThan.

Hence, in your test (file simplemultisig.js), the call to:

await web3SendTransaction({from: accounts[0], to: multisig.address, value: web3.toWei(new BigNumber(0.1), 'ether')})

Ends with an error:

number.lessThan is not a function

Hi,

I would like to use an adaption of your contract for a research project, would you be ok with licensing it (e.g. MIT license)?

Thanks,
Jakob

https://github.com/christianlundkvist/simple-multisig/blob/15a8095818008b0bf3eb901e488d0c5b488513d4/contracts/SimpleMultiSig.sol#L39

Use the call opcode instead like:
https://github.com/uport-project/uport-identity/pull/99/files
because address.call pads an extra 32 bytes
ethereum/solidity#2884
(which can cause failure on token contracts such as https://blog.coinfabrik.com/smart-contract-short-address-attack-mitigation-failure/)

a properly functioning ecrecover seems pretty essential here:
https://github.com/ethereum/solidity/releases/tag/v0.4.14

I cloned this repository and then ran npm install to install the dependencies. But when everything is OK and I ran npm run test, no test case pass. They are all failed because of Uncaught Error: exceeds block gas limit. Can this be solved?

Hi,

We tried to contact you but couldn't find your email, and you didn't respond to a tweet we sent you a week ago.

We are looking into this contract for production purposes, would like to understand a few things. Specifically, my question right now is: does the contract support ER20 tokens? If yes or no, perhaps you can add a comment about it in the readme, or if it doesn't support, perhaps leave this issue open as a TODO?

In any case we would be interested in discussing further, would appreciate it if you dropped me a line at [email protected]