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

Already on GitHub? Sign in to your account

Behaviour of OP_CHECKMULTISIG in bitcoind docs not match docs #622

Closed
gsalgado opened this Issue Oct 28, 2014 · 2 comments

Comments

Projects
None yet
2 participants

Location: https://bitcoin.org/en/developer-reference#term-op-checkmultisig

The dev guide says that OP_CHECKMULTISIG "[...] compares each of public keys against each of the signatures looking for ECDSA matches", but bitcoind doesn't seem to do that. Instead, for every signature it starts looking for a matching pubkey from the pubkey that last matched a signature.

If my reading of the code is correct, a redeem script of the form "sig2 sig1 2 pubkey1 pubkey2 pubkey3 3 OP_CHECKMULTISIG" would evaluate to false in bitcoind but the docs suggest that would work.

Contributor

harding commented Oct 28, 2014

@gsalgado nice catch! Thanks! I just read the function, and it seems to match your description, so I'll update the docs. (Although I'm pasting annotated Bitcoin Core code below in case somebody knows I'm misreading it.)

bool fSuccess = true;
while (fSuccess && nSigsCount > 0)
{
    valtype& vchSig    = stacktop(-isig);
    valtype& vchPubKey = stacktop(-ikey);

    // Check signature
    //G sig2 sig1 2 pubkey1 pubkey2 pubkey3 3 would fail
    //
    //H Iteration 1: CheckSig(sig2, pubkey1)
    //H    FAIL: isig is not increased; ikey is increased
    //H Iteration 2: CheckSig(sig2, pubkey2)
    //H    SUCCESS: isig is increased; ikey is increased
    //H Iteration 3: CheckSig(sig1, pubkey3)
    //H    FAIL: ikey is increased
    //H    ULTIMATE FAIL, no more keys to check
    if (CheckSig(vchSig, vchPubKey, scriptCode, txTo, nIn, nHashType))
    {
        isig++;
        nSigsCount--;
    }
    ikey++;
    nKeysCount--;

    // If there are more signatures left than keys left,
    // then too many signatures have failed
    if (nSigsCount > nKeysCount)
        fSuccess = false;

}

harding added a commit to harding/bitcoin.org that referenced this issue Oct 29, 2014

Dev Docs Correction: CHECKMULTISIG Requires Sigs In Same Order As Pub…
…Keys

As reported by @gsalgado (thanks!), the docs incorrectly state that all
sigs are compared against all pubkeys.  This commit provides a corrected
description, additional details, and references in other parts of the
text where we mention multisig. (Fixes #622)

Thanks for taking care of that, @harding !

@harding harding closed this in #623 Oct 29, 2014

jmaurice added a commit to jmaurice/bitcoin.jp that referenced this issue Nov 19, 2014

Dev Docs Correction: CHECKMULTISIG Requires Sigs In Same Order As Pub…
…Keys

As reported by @gsalgado (thanks!), the docs incorrectly state that all
sigs are compared against all pubkeys.  This commit provides a corrected
description, additional details, and references in other parts of the
text where we mention multisig. (Fixes #622)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment