Multisig Wallets

[rithvikvibhu]

Adds multisig wallets to Bob! Closes #493, #549.

The wallet creation and import flows have one new section for multisig. This also works with Ledger devices. The general flow to use multisig wallets is:

1. Create N wallets (one for each party) - need not be on the same computer
2. Finish setting them up by filling in other account keys
3. To spend from a wallet, do any action as usual (send HNS, bid, etc.)
4. This will show your transaction and can export to a file
5. Share the file with others to Load it in their wallet
6. Once signatures are collected, it can be broadcasted

Pics below may make more sense.

Screenshots

Click items to expand screenshots.

Creating / Importing multisig wallets

Wallet setup

Ready to use

Transaction Viewer

Multisig + Ledger

Gotchas / Notes

• Currently, when using Ledger wallets, approving transactions does not show the change output. With multisig, the device doesn't know about other parties and needs the change to be approved. Bob shows "own" in all outputs where the address belongs to your multisig wallet; this can be used to compare and verify with the device.
• Only people who have signed a bid tranaction can initiate a reveal (i.e. sign the reveal first). Example: In a 3-of-5 multisig (A to E), if A,B,C sign a bid, then then D and E do not know the true bid value. So, either:
  • A or B or C must be the first to sign the reveal (and can be signed by D or E later without issues), or
  • D or E must enter the true bid value to reveal the bid

What works and what doesn't

	Hot	Ledger	Hot Multisig	Ledger Multisig
Send / Receive HNS	✅	✅	✅	✅
Auction transactions (bid, reveal, redeem, etc.)	✅	✅	✅	✅
Domain Management (register, update, transfer, etc.)	✅	✅	✅	✅
Bulk actions (uses batching)	✅	✅	✅	✅
Sign messages	✅	➖ *	➖ **	➖ * / **
Verify messages	✅	✅	✅	✅
Watchlist	✅	✅	✅	✅
Atomic Name Swaps (send and receive)	✅	✅	✅	✅
Exchange (Shakedex)	✅	➖ &	➖ &	➖ &
Airdrop and Name Claims	✅	✅	✅	✅

symbol	explain
*	Signing is not supported by the Ledger Handshake app, there was a grant request on Discord for that
**	Multisig signing does not have a one true standard, can be added in the future if hsd supports one
&	The shakedex library requires a (probably major) refactor to be able to work with Ledger, Multisig

For reviewers

• I've broken up into commits after completing everything, so while reading commit-wise should be fine, do not expect Bob to function at every commit.
• This is based on wallet: batch bulk actions #546 to make sure batching works with multisig + ledger (and also simplifies code a bit) so either that needs to be merged first, or that should be closed that and this merged directly.
• This PR touches a lot of wallet code that is used for regular non-multisig wallets! So even if you're not interested in multisig, try using regular wallets to check if anything is broken.

Tips to review

• It's easy to revert back to a previous state if you have a copy of the regtest folder (just replace it back). If you like commands:

  # reset regtest back to how it was at `regtest-before-bid`
  cd /path/to/bobs/hsd_data/
  rm -rf regtest && cp -r regtest-before-bid regtest

• Starting with a 3-of-4 or 3-of-5 policy with one being Ledger covers most test cases (starting with ledger, ending with ledger, etc.) Can also leave one of the wallets as not setup.

Transaction File

This is conceptually similar to Bitcoin's PSBT, but in JSON format so easier to handle. It includes a transaction as a hex string, and metadata for inputs and outputs. app/background/wallet/service.js has JSDoc typedefs at the end of the file and helps with autocomplete if your editor supports it.

{
  "version": number,
  "tx": string,
  "metadata": {
    "inputs": [{"sighashType": Script.hashType (number)}],
    "outputs": [{"name": string, bid: number}],
  }
}

where:

• version is always 1 (for now)
• metadata.{inputs, outputs} are arrays with index matching input/output numbers (from 0)
• inputs:
  • sighashType (optional, defaults to ALL): refer to hsd's Script.hashType
• outputs:
  • name - required if a covenant is present (non-NONE)
  • bid - required if covenant is BID, in dollarydoos

To Do / Discuss

• Batching requires hsd master and multisig requires wallet: derive all keys in generateNonce for multisig handshake-org/hsd#767. If we consider master as unstable, it should be fine to update package.json with hsd master
• Should multisig show a warning / be a preview feature / be behind a feature flag?

[Falci]

There are a few other places we could use the t function, such as Error constructor and some of/or

[rithvikvibhu]

Thanks @Falci! Addressed all comments.

[pinheadmz]

Lots to review here. I read through all the wallet / background stuff and the new signing flow looks really good. I wrote a few review comments that I eventually deleted after reading more code ;-) I'm going to build locally next and try to break it, but great job so far on the wallet flow!

[pinheadmz]

I think this display may be a bit too verbose for Bob. Maybe the goodies can be hidden behind a "show advanced" arrow or something. IMO, all we need here is the main output / action / value (dont even bother showing destination address if its not a NONE send ?) and "who needs to sign".

actually the change output might be confusing to ledger users as well. hm....

[pinheadmz]

also I dont think we should allow users to export TX before they sign it themselves? I think thats going to lead to confusion

[pinheadmz]

Ok this is fun: create 2of2 ... I signed a tx, but then altered the exported file (changed the sighash flag). Importing that into the second wallet, it showed the first signer as having not signed (because you cleverly actually check the sig with secp256k1) BUT when I signed with the second wallet, the logic still detected what it thinks is two valid signatures. I'm not sure there's any great way to check for this (palm reader doesnt either I dont think) BUT we could:

1. check the entire TX one last time before broadcasting
2. i think the "broadcast" button needs some feedback: success/failuer

[pinheadmz]

copypasta typo?

Suggested change

	if (isNaN(mNum) || nNum <= 0) {
	if (isNaN(nNum) || nNum <= 0) {

[pinheadmz]

I found this max of 15 easy to bypass. I didn't even get a warning unless I hovered the mouse over the value. You can easily create a 1-of-100 multisig from this form. bob and hsd dont complain even though that redeem script (with 100 pubkeys) wouldnt fit in a witness item.

I even was able to create a wallet with n=1000000 and hsd threw silently in the background:

Sending IPC method error. {
  jsonrpc: '2.0',
  method: 'Wallet.createNewWallet',
  params: [ '<SANITIZED>' ],
  id: 347
} AssertionError [ERR_ASSERTION]: Assertion failed.
    at Account.fromOptions (/Users/matthewzipkin/Desktop/work/bob-wallet/node_modules/hsd/lib/wallet/account.js:116:7)
    at Function.fromOptions (/Users/matthewzipkin/Desktop/work/bob-wallet/node_modules/hsd/lib/wallet/account.js:164:26)
    at Wallet._createAccount (/Users/matthewzipkin/Desktop/work/bob-wallet/node_modules/hsd/lib/wallet/wallet.js:635:29)
    at Wallet.init (/Users/matthewzipkin/Desktop/work/bob-wallet/node_modules/hsd/lib/wallet/wallet.js:193:21)
    at WalletDB._create (/Users/matthewzipkin/Desktop/work/bob-wallet/node_modules/hsd/lib/wallet/walletdb.js:1272:5)
    at WalletDB.create (/Users/matthewzipkin/Desktop/work/bob-wallet/node_modules/hsd/lib/wallet/walletdb.js:1249:14)
    at WalletService.createNewWallet (/Users/matthewzipkin/Desktop/work/bob-wallet/app/background/wallet/service.js:369:13) {
  type: 'AssertionError',
  code: 'ERR_ASSERTION',
  generatedMessage: true,
  actual: false,
  expected: true,
  operator: '=='
}

[pinheadmz]

oops I shouldve made this comment on n not m. I think the only rule on m is that it is <= n... if you can fit all the pubkeys you want in a script then you can always fit the signatures (pretty sure)

[pinheadmz]

Oct-18-2022.11-13-45.mp4

Here I tried to import the same xpub a second time. Instead of throwing an error, what it did was RENAME the original signer and then reset the second input field

[pinheadmz]

while were talking about "advanced" features... maybe you can offer a "copy hex to clipboard" or something to be compatible with palm reader multisig UI 😬 . Its a good addition to export as file anyway

[rithvikvibhu]

while were talking about "advanced" features... maybe you can offer a "copy hex to clipboard" or something to be compatible with palm reader multisig UI grimacing . Its a good addition to export as file anyway

added a tertiary button so that it's visible, but it doesn't attract attention from regular bob users won't need it @pinheadmz: