Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multisig support #4

Open
markblundeberg opened this issue Apr 5, 2019 · 8 comments
Open

multisig support #4

markblundeberg opened this issue Apr 5, 2019 · 8 comments

Comments

@markblundeberg
Copy link
Contributor

markblundeberg commented Apr 5, 2019

It should definitely be possible to support multisig. But, there are some weird snags with this which one should watch out for. If someone wants to pull their hair out in confusion, take a gander at how it really works in interpreter.cpp.

  • A multisig can include the same pubkey multiple times, in which signatures can be replayed. (e.g., a single unique signature could satisfy a 2-of-3; two unique signatures would then be enough to malleate)
  • The same pubkey can even appear in different guises (compressed / uncompressed).
  • It's not obvious which signature goes with which pubkey until you just try them out.
  • A multisig can include invalid pubkeys at the end! (e.g., totally fine if third pubkey in a 2-of-3 is "RALF" as long as the first two signatures work with first two pubkeys).
  • We need NULLDUMMY for these to be unmalleable, but that's likely going to happen in November along with SCRIPTSIGMINIMALPUSH rule.
  • added -- see below for more info: Each signature can choose its own sighash flag; each of the six possible sighash flags has a different preimage which must be included in the DS proof packet, if that signature is to be validated.

So, it's going to take some delicate thinking...

@cpacia
Copy link

cpacia commented Apr 5, 2019

Why does the pubkey need to be extracted?

This kind of goes to my other comment in the other issue in that it seems the implementation would be much simpiler if the full scriptsig is relayed. Then you don't need to do any parsing or have the signature field in the ds message be formatted different ways depending on what type of script it is.

@markblundeberg
Copy link
Contributor Author

Why does the pubkey need to be extracted?

This kind of goes to my other comment in the other issue in that it seems the implementation would be much simpiler if the full scriptsig is relayed. Then you don't need to do any parsing or have the signature field in the ds message be formatted different ways depending on what type of script it is.

Interesting point! Hmm... the thing I dislike that wallets would have to in some sense code that convoluted EvalScript logic to 'execute' a scriptsig and redeemscript like that, including all malleability fixes etc. It would make the specification of DS proof easier but the implementation harder.

@cpacia
Copy link

cpacia commented Apr 5, 2019

That is true. I'm used to thinking about my code because it's very easy to use it a library a validate a script:

       tx := wire.MsgTx {
		TxIn: []*wire.TxIn{
			PreviousOutPoint: outpoint,
			SignatureScript: scriptSig,
                        Sequence: sequence,
		}
	}

	hashCache := txscript.TxSigHashes{
		HashOutputs: hashOutputs,
		HashPrevOuts: hashPrevOuts,
		HashSequence: hashSequence,
	}
	vm, _ := txscript.NewEngine(scriptPubkey, &tx, 0, txscript.StandardVerifyFlags, nil, hashCache, inputAmount)
	if err := vm.Execute(); err != nil {
		// Invalid scriptSig
	}

@markblundeberg
Copy link
Contributor Author

I'm trying to imagine in my mind how much work it is to build a separate system vs making a minimal EvalScript simulator in a wallet. The more I think about it, the EvalScript thing is probably not that bad actually -- remember that it can assume all modern flags being set (DS proofs are only for currently valid txns) so the logic gets simplified. Due to malleability fixes the scriptsig will always come in a standard form. I think you may be right that it's the best way to go. (Worth considering though: may be worth snipping off the final redeemscript push, that will be redundant with scriptCode.)

@markblundeberg
Copy link
Contributor Author

markblundeberg commented Apr 6, 2019

Another issue about multisigs is that each signature can have its own sighash flag. I'll assume that protected transactions must have all sighash ALL. But, the conflicting transaction might have a mixture of sighash flags!

Each sighash flag is its own preimage, and there are up to 6 sighash combinations. In order to fully execute the scriptSig + redeemscript, we need all the preimages. So fully executing the conflicting transaction's scriptsig may require 6 preimages ... ugh. (Double ugh since the preimages include scriptCode and are thus each significantly larger than in p2pkh case)

This is not ideal -- it would be more efficient (and sufficient) if DS proof in this case were to focus on one pubkey out of the whole bunch and prove that it alone signed with a non-protected sighash. One preimage, one signature.

Fortunately in the case where each given side has pure sighash ALL, it's still only necessary to have one preimage. The preimage stays the same for each signature.

@imaginaryusername
Copy link
Owner

If P2SH/multisig requires NULLDUMMY that is only going to be activated November, that does mean any potential dsproof implementation that comes before it should not support it yet (and only create proofs for ole P2PKH), no?

@markblundeberg
Copy link
Contributor Author

it doesn't require nulldummy any more than p2pkh requires the scriptsig minimal push fix (which we don't have now either)...

@imaginaryusername
Copy link
Owner

Even moar multisig, now with a new proof type: 8f4cbde

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants