Remove reqSigs from output of decoderawtransaction and other RPCs #20102

1. 
   sanket1729 commented at 6:58 am on October 8, 2020: contributor

   IRC logs

   sanket1729: How can decoderawtransaction RPC know reqSigs if the output scriptpubkey is p2sh/p2wsh? I think it is returning 1 everytime. sipa: sanket1729: it only works for bare multisig sipa: we should probably just remove it

   The output parameter for reqSigs is confusing as it outputs 1 for all scripts but bare multisig in the scriptpubkey. Considering the limited applicability of reqSigs and (slightly) confusing output(1) in almost all cases, I think it is best if we remove it.

   I don’t have the bandwidth to tackle this, so opening up the issue for book-keeping. I also think this is a good first issue for new contributors if there are no concept NACKs.

   Affected RPCs:

   • gettxout

   • decodescript

   • decoderawtransaction

   • getrawtransaction

2. 
   MarcoFalke commented at 7:06 am on October 8, 2020: member

   Another option to removing it would be to only have it present when a bare multisig is found
3. 
   NicolasDorier commented at 7:19 am on October 8, 2020: contributor

   Another option to removing it would be to only have it present when a bare multisig is found

   meh, nobody care about bare multisig anymore anyway

4. MarcoFalke added the label good first issue on Oct 8, 2020
5. 
   MarcoFalke commented at 7:32 am on October 8, 2020: member

   Ok, made it a “good first issue” to remove.

   Useful skills:

   • Bitcoin Script
   • Bitcoin Core RPC server

   Want to work on this issue?

   For guidance on contributing, please read CONTRIBUTING.md before opening your pull request.

6. 
   ghost commented at 7:55 am on October 8, 2020: none

   I can work on this. Removing L1107 https://github.com/bitcoin/bitcoin/blob/dde104963b3ba2e24bb3b383351ddcbef52a0240/src/rpc/blockchain.cpp#L1107 should fix gettxout I guess or maybe need to make more changes.

   Was looking at this PR for reference: https://github.com/bitcoin/bitcoin/pull/19646

7. 
   sanket1729 commented at 9:05 am on October 8, 2020: contributor

   Agreed, nobody uses bare multi-sig. We should probably remove it.

   Hi @prayank23, here is a possible workflow that I would use.

   • You can start by looking at all the RPCs listed in the remove the reqSigs field from that.
   • You can also grep for reqSigs in the root directory to find all occurrences.
   • Try to run unit tests and let the failures guide you where things need to the fixed/removed.
   • My suspicion is that we can also remove the code that has the bare multisig logic that computes this field.
   • You may also need to run python functional tests (see test/functional) to check if there are tests that are broken because of this.
   • Since this field is being removed, try to find comments around the code are removing and remove those too.
8. 
   ghost commented at 5:35 am on October 11, 2020: none

   @sanket1729 I made changes in https://github.com/prayank23/bitcoin/commit/33d3757688b5269a1d033aea8aae7e358141dc4d

   But getting errors while running the command make check because tests are failing even though I removed reqSigs from json files used for testing except txcreatemultisig1.json (wasn’t sure about it):

9. mjdietzx referenced this in commit 30259787c3 on Nov 2, 2020
10. mjdietzx referenced this in commit a6843f0263 on Nov 2, 2020
11. mjdietzx referenced this in commit f5d3cf6ceb on Nov 18, 2020
12. 
    nginocchio commented at 5:35 am on December 28, 2020: none

    Hello, does this issue still require work to be done on it?
13. 
    sipa commented at 6:30 am on December 28, 2020: member

    It is being worked on in #20286.
14. MarcoFalke deleted a comment on Dec 28, 2020
15. MarcoFalke referenced this in commit 1c7be9ab90 on Mar 29, 2021
16. MarcoFalke closed this on Mar 29, 2021

    ---

17. meshcollider referenced this in commit d6492d4ed0 on Sep 28, 2021
18. DrahtBot locked this on Aug 18, 2022

Contributors
sanket1729 MarcoFalke NicolasDorier ghost nginocchio sipa

Labels
good first issue