[RPC] show script verification errors in signrawtransaction result #5937

pull dexX7 wants to merge 2 commits into bitcoin:master from dexX7:rpc-signrawtransaction-script-errors changing 3 files +149 −8
  1. dexX7 commented at 7:21 PM on March 22, 2015: contributor

    If there are any script verification errors, when using signrawtransaction, they are shown in the RPC result.

    Example:

    ./bitcoin-cli "signrawtransaction" "0100000002d8fde545ff10eec10a6bef0c293c892b556aaa8c9e2f5f4c8545ca5c88e41c930100000000ffffffff62a717b7379c9fd900bf8cd42d6cbe1992f51aac2edff637dd291ec0b710acf23800000000ffffffff01a0410000000000001976a9142d0242ee11731e1fb95f3917b61a11aa400f6d5388ac00000000" "[{\"txid\":\"931ce4885cca45854c5f2f9e8caa6a552b893c290cef6b0ac1ee10ff45e5fdd8\",\"vout\":1,\"scriptPubKey\":\"512102a6bce06d76dfa05974f50e71660936966be40fceee99fed481a526016b3b240e21160000000100000001d865ecba07d000000000000000000000000000000000000052ae\"},{\"txid\":\"f2ac10b7c01e29dd37f6df2eac1af59219be6c2dd48cbf00d99f9c37b717a762\",\"vout\":56,\"scriptPubKey\":\"01\"}]" "[\"xxxxxx\"]"

    {
  "hex" : "0100000002d8fde545ff10eec10a6bef0c293c892b556aaa8c9e2f5f4c8545ca5c88e41c93010000004a00483045022100f5e5d54c229c241c8525a9e01c9774d0cf974f2262a7045636c2308dbae43af90220151439bb3913379327fcf10ee51b779f9b349c58870962a605276419122a2fb601ffffffff62a717b7379c9fd900bf8cd42d6cbe1992f51aac2edff637dd291ec0b710acf23800000000ffffffff01a0410000000000001976a9142d0242ee11731e1fb95f3917b61a11aa400f6d5388ac00000000",
  "complete" : false,
  "errors" : [
    {
      "txid" : "931ce4885cca45854c5f2f9e8caa6a552b893c290cef6b0ac1ee10ff45e5fdd8",
      "vout" : 1,
      "scriptSig" : "00483045022100f5e5d54c229c241c8525a9e01c9774d0cf974f2262a7045636c2308dbae43af90220151439bb3913379327fcf10ee51b779f9b349c58870962a605276419122a2fb601",
      "sequence" : 4294967295,
      "error" : "Public key is neither compressed or uncompressed"
    },
    {
      "txid" : "f2ac10b7c01e29dd37f6df2eac1af59219be6c2dd48cbf00d99f9c37b717a762",
      "vout" : 56,
      "scriptSig" : "",
      "sequence" : 4294967295,
      "error" : "Opcode missing or not understood"
    }
  ]
}

    The field errors is currently omitted, if no script verification error occured, but alternatively an empty array could be returned, which might be easier to process, but right now it basically mirrors the behavior of getrawtransaction and others, where some fields are only returned, if available.

    This could be perceived as inconsistent, as signrawtransaction also returns actual RPC errors, and not all error descriptions might be intuitive, for example, when partially signing a script-hash transaction, which then results in the message "Operation not valid with the current stack size", but addressing this seemed out of scope of this PR.

  2. jonasschnelli commented at 7:34 PM on March 22, 2015: contributor

    Looks good to me. utACK (will test). It would be nice if we would have some rpc tests for this in qa/rpc-tests/. If you do like to do this, you might start by taking this: https://github.com/jonasschnelli/bitcoin/blob/2014_12_fundtransaction/qa/rpc-tests/rawtransactions.py from PR #5503

  3. jgarzik commented at 11:42 AM on March 23, 2015: contributor

    ut ACK

    Agree w/ @jonasschnelli RE tests

  4. dexX7 force-pushed on Mar 23, 2015
  5. dexX7 force-pushed on Mar 23, 2015
  6. dexX7 commented at 5:59 PM on March 23, 2015: contributor

    The two tests are not generic, but use hardcoded values, such as the private key used for signing.

    Expected behavior:

    Test 1: create and sign a valid raw transaction with one input:

      1. The transaction has a complete set of signatures
      1. No script verification error occurred

    Test 2: create and sign a raw transaction with one valid, and one invalid input script:

      1. The transaction has no complete set of signatures
      1. One script verification error occurred
      1. Script verification errors have certain properties ("txid", "vout", "scriptSig", "sequence", "error")
      1. The verification error referrs to the invalid input (vin 1)

    As extension, but I'm not sure, if this should be done in this PR or maybe in another, it is also thinkable to report:

    • missing inputs or input scripts
    • signing failures

    ... to cover all cases where complete is false.

    In particular it would be nice, if there were a distinction between actual "script errors" and "a key for signing is missing", but I'm not sure how this could be easily done, because missing keys always result in script verification errors, and signing invalid scripts is never successful.

  7. jonasschnelli commented at 7:16 PM on March 23, 2015: contributor

    @dexX7 test looks good! Nice. We should try to keep the amount of rpc test scripts at minimum because of possible time limits during a test run (iirc travis has one). So maybe "rawtransactions.py" would be a better name (other rawtransaction testes could be added there).

  8. jgarzik commented at 7:24 PM on March 23, 2015: contributor

    Added: bitcoin-tx does the same function. It would be nice to update that at the same time.

  9. sipa commented at 12:58 PM on March 24, 2015: member

    utACK.

    Adding it to bitcoin-tx (along with an equivalent unit test for bitcoin-tx?) can be added later.

  10. dexX7 commented at 6:00 PM on March 26, 2015: contributor

    @jgarzik: Once this is final, I can add something similar to bitcoin-tx. @jonasschnelli: I see, I renamed the test to rawtransactions.py, but I guess there are going to be some conflicts, so please let me know, if there is something specific to change.

    To cover all cases where complete is false I extended the error reporting, and missing or already spent inputs are now also shown, with the error message:

    "Input not found or already spent"`

    To avoid code repetition, a local helper function is used:

    static void TxInErrorToJSON(const CTxIn& txin, Array& vErrorsRet, const std::string& strMessage)

    The test was extended to cover this case as well.

    If this is fine, I'd rebase and squash the commits into two.

  11. jgarzik added the label RPC on Apr 5, 2015
  12. jonasschnelli commented at 6:24 PM on April 7, 2015: contributor

    Tested ACK. @dexX7 don't worry about rawtransactions.py (in qa/rpc-tests/) conflicts. Depends on merge time there will be merge conflicts in this or other PRs. But trivial to solve.

  13. in src/rpcrawtransaction.cpp:None in 7c1d71489f outdated 
     545 | -            "  \"complete\": n       (numeric) if transaction has a complete set of signature (0 if not)\n"
 546 | +            "  \"hex\" : \"value\",   (string) The hex-encoded raw transaction with signature(s)\n"
 547 | +            "  \"complete\" : b,    (boolean) Whether the transaction has a complete set of signatures\n"
 548 | +            "  \"errors\" : [       (json array of objects) Script verification errors (if there are any)\n"
 549 | +            "    {\n"
 550 | +            "      \"txid\" : \"hash\",       (string) The transaction hash\n"
    sipa commented at 9:59 PM on April 7, 2015:

    It wasn't clear to me that the txid and vout here are about the outputs being spent, not about the transaction itself.

    dexX7 commented at 12:09 AM on April 8, 2015:

    Hm.. I'm not sure, if the purpose of this PR was not clearly communicated by me, or if I misread your comment, or if the labels are faulty. The goal is to report any errors related to the signing and spending when using signrawtransaction - basically anything that causes a result with "complete": false. Reporting which inputs in particular caused failures should help to locate the underlying issues. Same goes for sequence: if it can cause errors, it seems relevant in my opinion.

    sipa commented at 4:33 AM on April 8, 2015:

    Sorry if I wasn't clear. I just suggest making the help output a bit clearer. I understand what it does and why, but the help text doesn't explain which txid is being reported.

    dexX7 commented at 8:46 PM on April 9, 2015:

    Ahh, good point. I updated it to:

    • The hash of the referenced, previous transaction
    • The index of the output to spent and used as input
    • Verification or signing error related to the input
  14. in src/rpcrawtransaction.cpp:None in 7c1d71489f outdated 
     548 | +            "  \"errors\" : [       (json array of objects) Script verification errors (if there are any)\n"
 549 | +            "    {\n"
 550 | +            "      \"txid\" : \"hash\",       (string) The transaction hash\n"
 551 | +            "      \"vout\" : n,            (numeric) The output number\n"
 552 | +            "      \"scriptSig\" : \"hex\",   (string) The hex-encoded script\n"
 553 | +            "      \"sequence\" : n,        (numeric) Script sequence number\n"
    sipa commented at 9:59 PM on April 7, 2015:

    Seems overkill...

    sipa commented at 4:34 AM on April 8, 2015:

    Sequence number only has an effect on finality of transactions, which shouldn't usually be a cause of non-acceptance (if it is, it'll just be reported as non-final...).

  15. dexX7 force-pushed on Apr 9, 2015
  16. dexX7 force-pushed on Apr 26, 2015
  17. laanwj commented at 11:41 AM on May 4, 2015: member

    Needs rebase again (sorry)

  18. RPC: show script verification errors in "signrawtransaction" result
    If there are any script verification errors, when using "signrawtransaction", they are shown in the RPC result:

```
// ...

Result:
{
  "hex" : "value",           (string) The hex-encoded raw transaction with signature(s)
  "complete" : true|false,   (boolean) If the transaction has a complete set of signatures
  "errors" : [                 (json array of objects) Script verification errors (if there are any)
    {
      "txid" : "hash",           (string) The hash of the referenced, previous transaction
      "vout" : n,                (numeric) The index of the output to spent and used as input
      "scriptSig" : "hex",       (string) The hex-encoded signature script
      "sequence" : n,            (numeric) Script sequence number
      "error" : "text"           (string) Verification or signing error related to the input
    }
    ,...
  ]
}
```
    8ac2a4e178
  19. QA: add RPC tests for error reporting of "signrawtransaction"
    Tests error reporting of transaction signing via RPC call "signrawtransaction".

Expected results:

Test 1: create and sign a valid raw transaction with one input:
- 1) The transaction has a complete set of signatures
- 2) No script verification error occurred

Test 2: create and sign a raw transaction with one valid, one invalid and one missing input script:
- 3) The transaction has no complete set of signatures
- 4) Two script verification errors occurred
- 5) Script verification errors have certain properties ("txid", "vout", "scriptSig", "sequence", "error")
- 6) The verification errors refer to the invalid (vin 1) and missing input (vin 2)
    a71ab10f99
  20. dexX7 force-pushed on May 5, 2015
  21. laanwj merged this on May 5, 2015
  22. laanwj closed this on May 5, 2015
  23. laanwj referenced this in commit 40f5e8dc2a on May 5, 2015
  24. MarcoFalke locked this on Sep 8, 2021
Contributors
dexX7jonasschnellijgarziksipalaanwj
Labels
RPC/REST/ZMQ
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me