[Policy] Several transaction standardness rules #11423

pull jl2012 wants to merge 4 commits into bitcoin:master from jl2012:const_scriptcode changing 17 files +139 −53
  1. jl2012 commented at 9:44 PM on September 29, 2017: contributor

    This disables OP_CODESEPARATOR in non-segwit scripts (even in an unexecuted branch), and makes a positive FindAndDelete result invalid. This ensures that the scriptCode serialized in SignatureHash is always the same as the script passing to the EvalScript.

  2. jl2012 commented at 9:45 PM on September 29, 2017: contributor

    This is part of #8755 @TheBlueMatt

  3. TheBlueMatt commented at 9:48 PM on September 29, 2017: member

    Strong Concept ACK. We do ML posts for new standardness rules, right? Probably doesn't matter given their lack of use.

  4. jl2012 force-pushed on Sep 29, 2017
  5. sipa commented at 10:18 PM on September 29, 2017: member

    Any reason to support OP_CODESEPARATOR inside P2WSH?

  6. TheBlueMatt commented at 10:27 PM on September 29, 2017: member

    I'd prefer we be consistent. Especially if OP_CODESEPARATOR has no meaning in SegWit transactions we should just make it nonstandard everywhere.

    On September 29, 2017 6:18:36 PM EDT, Pieter Wuille notifications@github.com wrote:

    Any reason to support OP_CODESEPARATOR inside P2WSH?

    -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/bitcoin/bitcoin/pull/11423#issuecomment-333252262

  7. fanquake added the label Validation on Sep 30, 2017
  8. jl2012 commented at 5:33 AM on September 30, 2017: contributor

    @sipa I think @NicolasDorier has something that use CODESEPARATOR. I think it could make the size of some contracts smaller.

    This is actually part of the attempt to fix the pre-segwit quadratic sighash problem. A variable scriptCode makes sighash caching more difficult. That's why I proposed to do it for pre-segwit only

  9. MarcoFalke deleted a comment on Sep 30, 2017
  10. NicolasDorier commented at 4:40 AM on October 2, 2017: contributor 
    Any reason to support OP_CODESEPARATOR inside P2WSH?

    YES, I am using it for tumblebit, it saves 33 bytes per outputs. It allows you to sign a specific branch with one key instead of having separate public key per branch. Please do not do that until we have at least MAST. (only if signing in MAST requires the scriptCode to depends on the executed branch)

    OP_CODESEPARATOR is very useful, I am coupling it with SIGHASH_NONE. (The only case where SIGHASH_NONE is not similar to giving your private key)

  11. NicolasDorier commented at 5:01 AM on October 2, 2017: contributor

    Just to explain my case here: Imagine the following: Alice, expect Bob to publish a transaction satisfying the PAYMENT condition to get the preimage of <BOB_HASH>.

    OP_DEPTH 3 EQUAL
OP_IF
    2 <ALICE_KEY_PAYMENT> <BOB_KEY_PAYMENT> MULTICHECKSIGVERIFY HASH160 <BOB_HASH> EQUAL
OP_ELSE
    <ALICE_KEY_REDEEM> CHECKSIG CLTV DROP
OP_END

    If ALICE_KEY_PAYMENT == ALICE_KEY_REDEEM, this would be no good, as BOB could use Alice signature to satisfy the REDEEM condition, and get the money without revealing the pre image.

    Now you can save 33 bytes this way. (untested, but it get the idea of what I do for TB)

    <ALICE_KEY>
OP_DEPTH 3 EQUAL
OP_IF
    OP_SWAP <BOB_KEY_PAYMENT> CHECKSIGVERIFY HASH160 <BOB_HASH> EQUAL OP_CODESEP
OP_ELSE
     CLTV DROP
OP_END
CHECKSIG

    Notice that Alice can just pass a SIGHASH_NONE signature for the first branch to Bob, so she does not have to know how bob will spend the txout, while still being sure she can get the preimage as soon as Bob try to cashout.

    Concept ACK for removing on non-segwit scripts, but it should be kept for segwit. Using this trick, you can save quite a lot of data if there is lots of branches. (and also have more branches, since the scripts are limited to 512 bytes)

  12. in src/script/interpreter.h:110 in 98cea79fd2 outdated 
     105 | @@ -106,6 +106,10 @@ enum
 106 |      // Public keys in segregated witness scripts must be compressed
 107 |      //
 108 |      SCRIPT_VERIFY_WITNESS_PUBKEYTYPE = (1U << 15),
 109 | +
 110 | +    // Making OP_CODESEPARATOR and FindAndDelete non-standard in non-segwit scripts
    TheBlueMatt commented at 5:44 PM on October 3, 2017:

    For correctness, may wish to note that if FindAndDelete matches in either SegWit or non-SegWit, the script fails, OP_CODESEPARATOR limitation only applies to non-SegWit.

    jl2012 commented at 5:55 PM on October 3, 2017:

    FindAndDelete is not used anywhere in segwit

    TheBlueMatt commented at 6:00 PM on October 3, 2017:

    Oops, indeed, missed the sigversion check.

    TheBlueMatt commented at 9:51 PM on November 7, 2017:

    nit: comments on script flags shouldnt reference standardness - this flag "Makes OP_CODESEPARATOR and FindAndDelete fail any non-segwit scripts", not just make them nonstandard.

  13. in src/script/interpreter.cpp:304 in 98cea79fd2 outdated 
     300 | @@ -301,6 +301,10 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
 301 |                  opcode == OP_RSHIFT)
 302 |                  return set_error(serror, SCRIPT_ERR_DISABLED_OPCODE); // Disabled opcodes.
 303 |  
 304 | +            // OP_CODESEPARATOR in non-segwit transaction is invalid even in an unexecuted branch
    NicolasDorier commented at 3:17 AM on October 4, 2017:

    nit: would have pushed that in the switch case for OP_CODESEPARATOR.

    luke-jr commented at 2:13 PM on November 10, 2017:

    The switch case wouldn't get reached if it's in an unexecuted branch...?

    luke-jr commented at 2:13 PM on November 10, 2017:

    "Invalid" is the wrong term here, since it's just a policy rule (for now).

    NicolasDorier commented at 2:24 AM on November 11, 2017:

    Yep, disregard what I said, it would not have worked.

  14. jgarzik commented at 3:32 AM on October 4, 2017: contributor

    concept ACK

  15. TheBlueMatt commented at 9:52 PM on November 7, 2017: member

    utACK 98cea79fd2161ce0b42bdef27befc6a23047975f

  16. luke-jr commented at 2:16 PM on November 10, 2017: member

    Concept ACK

  17. jl2012 force-pushed on Nov 14, 2017
  18. jl2012 force-pushed on Nov 14, 2017
  19. TheBlueMatt commented at 6:57 PM on November 14, 2017: member

    utACK effb6f9568a92ad6fe0ebf9da308cb0237df327b. One interesting test-case you may consider adding is checking that a FindAndDelete does not match (and thus the script is valid, even with CONST_SCRIPTCODE) a push with a non-minimal push encoding.

  20. jonasschnelli commented at 7:12 AM on November 15, 2017: contributor

    utACK 0575b1831cd52987c76320d304674a27a140fe1f

    • IMO requires bitcoin-dev ML post (if merged)
    • requires release notes part (can be done after merging)
  21. in src/script/interpreter.cpp:309 in effb6f9568 outdated 
     300 | @@ -301,6 +301,10 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
 301 |                  opcode == OP_RSHIFT)
 302 |                  return set_error(serror, SCRIPT_ERR_DISABLED_OPCODE); // Disabled opcodes.
 303 |  
 304 | +            // With SCRIPT_VERIFY_CONST_SCRIPTCODE, OP_CODESEPARATOR in non-segwit script is invalid even in an unexecuted branch
    Sjors commented at 10:04 AM on November 16, 2017:

    Perhaps out of scope, but the word "invalid" is confusing. It's non-standard because this check is only part of STANDARD_SCRIPT_VERIFY_FLAGS. A future soft fork could make it actually invalid depending on e.g. block height. Perhaps we should use a different word like "reject"?

    TheBlueMatt commented at 4:40 PM on November 16, 2017:

    I mean it clearly indicates that "With SCRIPT_VERIFY_CONST_SCRIPTCODE", and in that case it is "invalid", same as any other invalid action.c

    esotericnonsense commented at 2:38 PM on February 20, 2018:

    Not particularly concerned but I agree that 'rejected' would be slightly clearer here.

  22. TheBlueMatt commented at 4:35 PM on November 27, 2017: member

    Relevant ML thread is at https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2017-November/015292.html

  23. NicolasDorier commented at 7:02 AM on November 28, 2017: contributor

    Just to clarify, OP_CODESEP could be removed completely from segwit if there is another way to explicitely sign a script path. (MAST could allow that)

  24. jl2012 force-pushed on Feb 6, 2018
  25. jl2012 commented at 7:35 PM on February 6, 2018: contributor

    rebased

  26. esotericnonsense commented at 2:48 PM on February 20, 2018: contributor

    utACK 850c41c2a33efd73eff0bbdefc6ba2762901b60e checked the diff of tx_valid and tx_invalid.json

  27. jl2012 force-pushed on Feb 20, 2018
  28. jl2012 commented at 6:11 PM on February 20, 2018: contributor

    Rebase with comments fixed (s/invalid/rejected/)

  29. MarcoFalke added the label TX fees and policy on May 2, 2018
  30. laanwj commented at 4:11 PM on May 3, 2018: member

    Rebased (for 6a7456ad6072f405e8b02bffa0fb4e9f0cfe71e0) version here: https://github.com/laanwj/bitcoin/tree/2018_05_temp_11423 Feel free to take over.

  31. MarcoFalke commented at 4:19 PM on May 3, 2018: member

    What is the status here? Given that we'll probably do a 0.16.1 release soon, should this be included?

  32. laanwj added this to the milestone 0.16.1 on May 3, 2018
  33. laanwj added the label Needs backport on May 3, 2018
  34. NicolasDorier commented at 5:56 PM on May 4, 2018: contributor

    utACK.

  35. Add constant scriptCode policy in non-segwit scripts
    This disables OP_CODESEPARATOR in non-segwit scripts (even in an unexecuted branch), and makes a positive FindAndDelete result invalid. This ensures that the scriptCode serialized in SignatureHash() is always the same as the script passing to the EvalScript.
    9dabfe49c0
  36. Add transaction tests for constant scriptCode
    Tests showing that CONST_SCRIPTCODE is applied only to non-segwit transactions
    0f8719bb03
  37. Policy to reject extremely small transactions
    A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes. Anything smaller than this have unnecessary malloc overhead and are not relayed/mined.
    7485488e90
  38. qa: Pad scriptPubKeys to get minimum sized txs 364bae5f7a
  39. jl2012 force-pushed on May 4, 2018
  40. jl2012 renamed this:
    [Policy] Make OP_CODESEPARATOR and FindAndDelete in non-segwit scripts non-std
    [Policy] Several transaction standardness rules
    on May 4, 2018
  41. jl2012 commented at 9:21 PM on May 4, 2018: contributor

    Rebased, and added another standardness rule to reject transaction smaller than 82 bytes (non-segwit size). 82 bytes is the size for a transaction with 1 segwit input and 1 P2WPKH output. Anything smaller than this have unnecessary malloc overhead

  42. TheBlueMatt commented at 7:46 PM on May 6, 2018: member

    utACK 364bae5f7a6b16eef63990154e48f19e7e693039

  43. sipa commented at 10:36 PM on May 6, 2018: member

    utACK 364bae5f7a6b16eef63990154e48f19e7e693039

  44. laanwj commented at 3:39 PM on May 12, 2018: member

    utACK 364bae5f7a6b16eef63990154e48f19e7e693039

  45. laanwj merged this on May 12, 2018
  46. laanwj closed this on May 12, 2018
  47. laanwj referenced this in commit 6af005c3eb on May 12, 2018
  48. fanquake added this to the "For backport" column in a project
  49. MarcoFalke referenced this in commit 13565a45bb on May 24, 2018
  50. MarcoFalke referenced this in commit c9cd14eb92 on May 24, 2018
  51. MarcoFalke referenced this in commit 5e644fe6b3 on May 24, 2018
  52. MarcoFalke referenced this in commit 7d539ddba3 on May 24, 2018
  53. MarcoFalke referenced this in commit fe26aa818d on May 24, 2018
  54. MarcoFalke referenced this in commit 9ae581f7d9 on May 24, 2018
  55. MarcoFalke referenced this in commit 0ddde84072 on May 24, 2018
  56. MarcoFalke referenced this in commit d353dd121b on May 24, 2018
  57. MarcoFalke referenced this in commit 1fffc2b346 on May 24, 2018
  58. MarcoFalke referenced this in commit 0a000b9b73 on May 24, 2018
  59. MarcoFalke referenced this in commit 08334b73be on May 24, 2018
  60. fanquake removed the label Needs backport on May 28, 2018
  61. fanquake commented at 9:36 AM on May 28, 2018: member

    Backported in #13317.

  62. fanquake removed this from the "For backport" column in a project
  63. azuchi referenced this in commit 8d4fcb57cd on May 31, 2018
  64. HashUnlimited referenced this in commit a76a700fab on Jun 29, 2018
  65. HashUnlimited referenced this in commit b14c9592ba on Jun 29, 2018
  66. HashUnlimited referenced this in commit d6f0266e0b on Jun 29, 2018
  67. HashUnlimited referenced this in commit 28e2d424d8 on Jun 29, 2018
  68. ccebrecos referenced this in commit 7aebff909b on Sep 14, 2018
  69. ccebrecos referenced this in commit 507cfc6e58 on Sep 14, 2018
  70. ccebrecos referenced this in commit 13c40ab7ad on Sep 14, 2018
  71. ccebrecos referenced this in commit a9f7a926bd on Sep 14, 2018
  72. braydonf referenced this in commit e866da81ee on Feb 2, 2019
  73. braydonf referenced this in commit efa78100e0 on Feb 2, 2019
  74. tuxcanfly referenced this in commit 586fef8219 on Apr 19, 2019
  75. fanquake referenced this in commit 9bf5768dd6 on Sep 19, 2019
  76. Fabcien referenced this in commit 82d1d9deaf on Jan 18, 2021
  77. UdjinM6 referenced this in commit 4cd23b54fb on May 21, 2021
  78. UdjinM6 referenced this in commit 44a486312a on Jun 5, 2021
  79. UdjinM6 referenced this in commit 906be7e144 on Jun 5, 2021
  80. Munkybooty referenced this in commit 7c5a70c227 on Dec 7, 2021
  81. Munkybooty referenced this in commit cdc4636a53 on Dec 7, 2021
  82. Munkybooty referenced this in commit 6362ec77a1 on Dec 7, 2021
  83. Munkybooty referenced this in commit 4aaf727acb on Dec 7, 2021
  84. Munkybooty referenced this in commit 5eeffca097 on Dec 15, 2021
  85. Munkybooty referenced this in commit 3b4c0e5a01 on Dec 15, 2021
  86. Munkybooty referenced this in commit 2c15abd6d1 on Dec 20, 2021
  87. Munkybooty referenced this in commit 6b6bd612e4 on Dec 23, 2021
  88. Munkybooty referenced this in commit 2b840433b7 on Dec 27, 2021
  89. Munkybooty referenced this in commit ea5a59fac8 on Dec 27, 2021
  90. Munkybooty referenced this in commit 088dd5b878 on Dec 28, 2021
  91. Munkybooty referenced this in commit 46bd0f467e on Dec 29, 2021
  92. Munkybooty referenced this in commit 1f308e00be on Dec 29, 2021
  93. Munkybooty referenced this in commit 49eecde8b2 on Dec 31, 2021
  94. Munkybooty referenced this in commit 906672ff1c on Jan 4, 2022
  95. Munkybooty referenced this in commit c938c7528a on Jan 19, 2022
  96. Munkybooty referenced this in commit 042207fa33 on Jan 20, 2022
  97. DrahtBot locked this on Feb 15, 2022
Contributors
jl2012TheBlueMattsipaNicolasDorierluke-jrjgarzikjonasschnelliSjorsesotericnonsenselaanwjMarcoFalkefanquake
Labels
TX fees and policyValidation

Milestone
0.16.1

Linked (view graph)
#16885 doc: Update tx-size-small comment with relevant CVE disclosure
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-30 06:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me