Define SIGHASH_MASK in validation and determine the use of SIGHASH_SINGLE in signing #13357

1. 
   jl2012 commented at 11:15 am on May 31, 2018: contributor

   We should use exactly the same code to determine the use of SIGHASH_SINGLE, in both validation and signing.

   The existing signing code is safe because SIGHASH is restricted to 6 types. However, if further types are introduced (e.g. SIGHASH_NOINPUT) and we forget to make changes here, people might accidentally sign with SIGHASH_SINGLE when there is no corresponding output and lose money. So it’s better to fix it when we still remember

2. Accurately determine the use of SIGHASH_SINGLE in signing 283b96b7c2
3. MarcoFalke added the label RPC/REST/ZMQ on May 31, 2018
4. MarcoFalke added the label Utils/log/libs on May 31, 2018
5. 
   Empact commented at 11:32 am on June 1, 2018: member

   How about giving 0x1f a name e.g. SIGHASH_MASK and use it in other places anywhere either value is used? The only other use I see is in IsDefinedHashtypeSignature.
6. 
   jl2012 commented at 4:35 pm on June 1, 2018: contributor

   @Empact : 0x1f also used in SignatureHash
7. 
   Empact commented at 8:25 am on June 5, 2018: member

   @jl2012 Sorry to be unclear, was referring to ~SIGHASH_ANYONECANPAY
8. 
   jl2012 commented at 10:26 am on June 5, 2018: contributor

   (& ~SIGHASH_ANYONECANPAY) has no consensus meaning. Not sure why it was used here in the first place
9. 
   Christewart commented at 0:38 am on June 6, 2018: member

   I would like to see it named SIGHASH_MASK and then placed as a constant some where as well.

   utack 283b96b

10. Define SIGHASH_MASK b84c353216
11. 
    jl2012 commented at 2:29 am on June 7, 2018: contributor

    @Christewart added a commit to define SIGHASH_MASK. I’m fine with or without that. @MarcoFalke does it need a Validation tag?
12. jl2012 renamed this:
    Accurately determine the use of SIGHASH_SINGLE in signing
    Define SIGHASH_MASK in validation and determine the use of SIGHASH_SINGLE in signing
    on Jun 7, 2018
13. MarcoFalke added the label Validation on Jun 7, 2018
14. 
    Christewart commented at 9:45 pm on June 7, 2018: member

    re-utack b84c353 . Don’t we have a Policy label?
15. 
    Empact commented at 10:05 pm on June 7, 2018: member

    One more use of ~SIGHASH_ANYONECANPAY at https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L193
16. 
    jl2012 commented at 7:28 pm on June 10, 2018: contributor

    @Empact that one is correct. It makes only 6 types of SIGHASH standard: 1, 2, 3, 0x81, 0x82, 0x83
17. 
    DrahtBot commented at 3:16 pm on July 29, 2018: member
18. DrahtBot closed this on Jul 29, 2018

    ---

19. DrahtBot reopened this on Jul 29, 2018

    ---

20. 
    Empact commented at 6:36 am on August 30, 2018: member

    utACK 283b96b
21. 
    DrahtBot commented at 6:52 pm on September 7, 2018: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15639 (bitcoin-wallet tool: Drop libbitcoin_server.a dependency by ryanofsky)
    • #15638 (Move-only: Pull wallet code out of libbitcoin_server by ryanofsky)
    • #13462 (Make SER_GETHASH implicit for CHashWriter and SerializeHash by Empact)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

22. DrahtBot added the label Needs rebase on Apr 10, 2019
23. 
    DrahtBot commented at 2:22 pm on April 10, 2019: member
24. fanquake removed the label RPC/REST/ZMQ on Jul 29, 2019
25. fanquake removed the label Utils/log/libs on Jul 29, 2019
26. fanquake added the label Needs Conceptual Review on Jul 29, 2019
27. fanquake added the label Up for grabs on Jul 29, 2019
28. fanquake closed this on Jul 29, 2019

    ---

29. laanwj removed the label Needs rebase on Oct 24, 2019
30. MarcoFalke locked this on Dec 16, 2021

Contributors
jl2012 Empact Christewart DrahtBot

Labels
Validation Up for grabs Needs Conceptual Review