Segregated witness rebased #8149

pull sipa wants to merge 27 commits into bitcoin:master from sipa:segwit-master2 changing 80 files +5305 −571
  1. sipa commented at 1:06 pm on June 6, 2016: member

    This PR is a rebased and squashed version of #7910. As this is the form (in pieces or in whole) that we expect it to be merged in, I’m opening a separate pull request for it. I will leave the old one open for discussion and history.

    The tree here is identical to the resulting tree there:

    0$ git show -s --format="%T" 3cb46c1a4ac94f4a7f25368bc2ba3c784c901b89
18ddfe56cfedba64667c63dd0fef6ee9584889719
2$ git show -s --format="%T" 17389dc466f2acf8bfa64ce0416f3b5281445a5c
38ddfe56cfedba64667c63dd0fef6ee9584889719

    Where 3cb46c1a4ac94f4a7f25368bc2ba3c784c901b89 is #7910’s tip commit, and 17389dc466f2acf8bfa64ce0416f3b5281445a5c is this PR’s tip commit.

    Please make comments on #7910, so the history can be tracked, and everything stays in one place.

  2. sipa commented at 1:06 pm on June 6, 2016: member

    Here is a categorized list of the commits:

    • P2P/node/consensus (https://github.com/sipa/bitcoin/compare/4182520...7080d47)
      • 8199125 BIP144: Serialization, hashes, relay (sender side)
      • 04dd13a BIP141: Witness program
      • 1fff664 BIP141: Commitment structure and deployment
      • 902c279 BIP144: Handshake and relay (receiver side)
      • 113b3e5 Refactor script validation to observe amounts
      • 1f5bb93 BIP143: Verification logic
      • 87252e9 [RPC] Return witness data in blockchain RPCs
      • 94c2abb BIP141: Other consensus critical limits, and BIP145
      • f0b33a5 [libconsensus] Script verification API with amounts
      • 76cb63b Add rewind logic to deal with post-fork software updates
    • wallet (https://github.com/sipa/bitcoin/compare/7080d47...8a5665a)
      • b344e52 BIP143: Signing logic
      • ddb6682 [RPC] Add wallet support for witness transactions (using P2SH)
      • 0c57081 [RPC] signrawtransaction can sign P2WSH
    • tests (https://github.com/sipa/bitcoin/compare/8a5665a...37916d3)
      • f7d6e0a [qa] Witness version 0 signing unit tests
      • 2cbf540 [qa] Add transaction tests for segwit
      • 29344db [qa] Add segwit support to script_tests
      • ec00dc9 [qa] Autogeneration support for witness in script_tests
      • e711429 [qa] Add rpc test for segwit
      • 7d828f1 [qa] p2p segwit tests
      • 0612ad6 [qa] script_tests: witness tests can specify tx amount
      • 89179b0 [qa] Add GetTransactionSigOpCost unit tests
    • deployment (https://github.com/sipa/bitcoin/compare/37916d3...74300b9a5ed24e9f7d80ecc39d4e19690732ccbe)
      • 74300b9 BIP9 parameters for testnet
  3. kanzure commented at 1:34 pm on June 6, 2016: contributor
    ACK for same git tree hash for 3cb46c1a4ac94f4a7f25368bc2ba3c784c901b89 and 17389dc466f2acf8bfa64ce0416f3b5281445a5c.
  4. laanwj added the label Consensus on Jun 6, 2016
  5. laanwj added the label P2P on Jun 6, 2016
  6. in src/test/sigopcount_tests.cpp: in 17389dc466 outdated 
    71+ */
72+ScriptError VerifyWithFlag(const CTransaction& output, const CMutableTransaction& input, int flags)
73+{
74+    ScriptError error;
75+    CTransaction inputi(input);
76+    bool ret = VerifyScript(inputi.vin[0].scriptSig, output.vout[0].scriptPubKey, inputi.wit.vtxinwit.size() > 0 ? &inputi.wit.vtxinwit[0].scriptWitness : NULL, flags, TransactionSignatureChecker(&inputi, 0, output.vout[0].nValue), &error);
    sipa commented at 7:14 am on June 7, 2016:
    Please comment in #7910.
  7. sipa force-pushed on Jun 12, 2016
  8. sipa commented at 8:32 pm on June 12, 2016: member
    Updated with a rebased/squashed version of the changes in #7910. The resulting tree should still be identical.
  9. sipa force-pushed on Jun 13, 2016
  10. sipa commented at 8:33 pm on June 13, 2016: member
    Updated again.
  11. MarcoFalke added this to the milestone 0.13.0 on Jun 15, 2016
  12. sipa force-pushed on Jun 16, 2016
  13. sipa force-pushed on Jun 16, 2016
  14. sipa force-pushed on Jun 16, 2016
  15. sipa force-pushed on Jun 16, 2016
  16. sipa force-pushed on Jun 16, 2016
  17. sdaftuar commented at 5:05 am on June 17, 2016: member
    ACK 74300b9a5ed24e9f7d80ecc39d4e19690732ccbe
  18. jonasschnelli commented at 6:14 am on June 17, 2016: contributor
    Reviewed everything. Tested different scenarios. Mostly focused on the wallet. ACK 74300b9a5ed24e9f7d80ecc39d4e19690732ccbe
  19. NicolasDorier commented at 10:27 am on June 17, 2016: contributor
    ACK 74300b9a5ed24e9f7d80ecc39d4e19690732ccbe (focused more on the scripting side, and tx signature v2, lightly reviewed the rest)
  20. --- [SEGWIT] begin: P2P/node/consensus --- ecacfd98e6
  21. BIP144: Serialization, hashes, relay (sender side) 
    Contains refactorings by Eric Lombrozo.
Contains fixup by Nicolas Dorier.
Contains cleanup of CInv::GetCommand by Alex Morcos
    7030d9eb47
  22. BIP141: Witness program 449f9b8deb
  23. BIP141: Commitment structure and deployment 
    Includes a fix by Suhas Daftuar and LongShao007
    8b49040854
  24. BIP144: Handshake and relay (receiver side) 
    Service bit logic by Nicolas Dorier.

Only download blocks from witness peers after fork.
    b8a97498df
  25. Refactor script validation to observe amounts 
    This is a preparation for BIP143 support.
    0ef1dd3e11
  26. BIP143: Verification logic 
    Includes simplifications by Eric Lombrozo.
    3dd410294d
  27. [RPC] Return witness data in blockchain RPCs 
    Includes RPC field name changes by Luke-jr.
    7c4bf779e8
  28. BIP141: Other consensus critical limits, and BIP145 
    Includes changes by Suhas Daftuar, Luke-jr, and mruddy.
    2b1f6f9ccf
  29. [libconsensus] Script verification API with amounts 
    script_tests: always test bitcoinconsensus_verify_script_with_amount if VERIFY_WITNESS isn't set

Rename internal method + make it static

trim bitcoinconsensus_ prefix

Add SERIALIZE_TRANSACTION_WITNESS flag
    b7dbeb24eb
  30. Add rewind logic to deal with post-fork software updates 
    Includes logic for dealing with pruning by Suhas Daftuar.
    6032f6930a
  31. Do not use compact blocks when segwit is enabled af87a67eff
  32. --- [SEGWIT] begin: wallet --- 9757b57c25
  33. BIP143: Signing logic 605e8473a7
  34. [RPC] Add wallet support for witness transactions (using P2SH) 
    Includes support for pushkeyhash wit v0 by Alex Morcos.
    f4691ab3a9
  35. [RPC] signrawtransaction can sign P2WSH 745eb678ef
  36. --- [SEGWIT] begin: tests --- 978e2004ad
  37. [qa] Witness version 0 signing unit tests 0aa9207451
  38. [qa] Add transaction tests for segwit 
    Including BIP143 P2WSH examples by jl2012.
    00f46cbcd9
  39. [qa] Add segwit support to script_tests 
    Contains fix by Johnson Lau.
    06d3805c1a
  40. [qa] Autogeneration support for witness in script_tests 66cca79130
  41. [qa] Add rpc test for segwit 
    Amended by Pieter Wuille to use multisig 1-of-1 for P2WSH tests, and BIP9
based switchover logic.

Fixes and py3 conversion by Marco Falke.
    4f7ff00497
  42. [qa] p2p segwit tests 
    mininode now supports witness transactions/blocks, blocktools
has a helper for adding witness commitments to blocks, and script
has a function to calculate hashes for signature under sigversion
1, used by segwit.

Py3 conversion by Marco Falke

Test to make sure upgraded nodes don't ask for non-wit blocks by
Gregory Sanders.
    330b0f31ee
  43. [qa] script_tests: witness tests can specify tx amount 
    Add tests that witness signatures cover value
    d846e02372
  44. [qa] Add GetTransactionSigOpCost unit tests fdb43df23e
  45. --- [SEGWIT] begin: deployment --- 070dbc48a9
  46. BIP9 parameters for testnet f8528134fc
  47. sipa force-pushed on Jun 22, 2016
  48. sipa commented at 2:08 pm on June 22, 2016: member

    Rebased on top of #8068 and #8179.

    Note that since there is currently no written-down specification for how segwit should interact with compact blocks, I’ve taken the conservative approach of disabling compact blocks whenever the segwit softfork is defined (in practice this means that compact blocks won’t be used on testnet, but will be on mainnet).

    Defining how segwit + compact blocks work together should be trivial, but it shouldn’t burden this PR and testing thereof.

  49. sneurlax commented at 9:36 am on June 24, 2016: none
    will run #8149 node! finally!
  50. laanwj commented at 2:33 pm on June 24, 2016: member
    ACK f852813
  51. laanwj merged this on Jun 24, 2016
  52. laanwj closed this on Jun 24, 2016
  53. laanwj referenced this in commit d612837814 on Jun 24, 2016
  54. skaag commented at 8:16 pm on June 24, 2016: none
    Exciting release! Great job :)
  55. dcousens commented at 2:03 pm on June 26, 2016: contributor
    utACK 070dbc4 (w/ non-intensive testing done)
  56. petertodd commented at 1:41 pm on June 28, 2016: contributor

    utACK consensus critical parts, https://github.com/bitcoin/bitcoin/pull/8149/commits/070dbc48a9338375fd7ce0a86ee05b476cf487a4

    I wrote up an in-depth review here: https://petertodd.org/2016/segwit-consensus-critical-code-review

    Note that I think we may have an issue with the P2P code with the (non-consensus) handling of transactions; see the last part of my writeup: https://petertodd.org/2016/segwit-consensus-critical-code-review#peer-to-peer-networking

  57. ysangkok referenced this in commit 3b058bba32 on Jul 20, 2016
  58. ysangkok referenced this in commit ab4f511c5c on Jul 20, 2016
  59. in src/main.cpp: in f8528134fc 
    472@@ -470,6 +473,10 @@ void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) {
473 }
474 
475 void MaybeSetPeerAsAnnouncingHeaderAndIDs(const CNodeState* nodestate, CNode* pfrom) {
476+    if (nLocalServices & NODE_WITNESS) {
    rebroad commented at 2:35 am on August 25, 2016:
    How does SegWit impact on the usefulness of Compact Blocks?
    sipa commented at 6:32 am on August 25, 2016:

    @rebroad Read BIP 141, 143, 144.

    Yes, segregated witness full nodes only download from other witness nodes. There is no way around that: they require the witness data to be able to verify blocks, and non-witness nodes cannot provide that.

    See #8393 for how to make compact blocks support segregated witness.

  60. in src/main.cpp: in f8528134fc 
    4939@@ -4684,6 +4940,14 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
4940     }
4941 }
4942 
4943+uint32_t GetFetchFlags(CNode* pfrom, CBlockIndex* pprev, const Consensus::Params& chainparams) {
4944+    uint32_t nFetchFlags = 0;
4945+    if (IsWitnessEnabled(pprev, chainparams) && State(pfrom->GetId())->fHaveWitness) {
    rebroad commented at 10:10 am on August 25, 2016:
    Ok, I have to ask (as I’ve been wondering about this for a while), but what is the purpose of this GetId() function? Why use it instead of the variable id?
  61. in src/main.cpp: in f8528134fc 
    5053@@ -4790,6 +5054,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
5054 
5055         pfrom->fClient = !(pfrom->nServices & NODE_NETWORK);
5056 
5057+        if((pfrom->nServices & NODE_WITNESS))
5058+        {
5059+            LOCK(cs_main);
5060+            State(pfrom->GetId())->fHaveWitness = true;
    rebroad commented at 10:24 am on August 26, 2016:
    How is this useful? (pfrom->nServices & NODE_WITNESS) is 33 characters, State(pfrom->GetId())->fHaveWitness is 35 characters. It’s easier and quicker and uses up less memory to type the thing this variable returns.
  62. in src/main.cpp: in f8528134fc 
    5291@@ -5016,8 +5292,10 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
5292                     pfrom->PushMessage(NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), inv.hash);
5293                     CNodeState *nodestate = State(pfrom->GetId());
5294                     if (CanDirectFetch(chainparams.GetConsensus()) &&
5295-                        nodestate->nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
5296-                        if (nodestate->fProvidesHeaderAndIDs)
5297+                        nodestate->nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER &&
5298+                        (!IsWitnessEnabled(chainActive.Tip(), chainparams.GetConsensus()) || State(pfrom->GetId())->fHaveWitness)) {
    rebroad commented at 10:30 am on August 26, 2016:
    Maybe I’m misreading the code, but once SegWit activates then where is the code that downloads blocks upon receiving a block inv? Block invs are no longer responded to?
    sipa commented at 10:34 am on August 26, 2016:
    Read the part of the line after ||.
    rebroad commented at 10:51 am on August 26, 2016:
    Sorry, it’s taking me a while to get my head around this - I keep making the assumption that most nodes won’t be running SegWit for some reason!!
  63. in src/init.cpp: in f8528134fc 
    1393+        // Note that setting NODE_WITNESS is never required: the only downside from not
1394+        // doing so is that after activation, no upgraded nodes will fetch from you.
1395+        nLocalServices = ServiceFlags(nLocalServices | NODE_WITNESS);
1396+        // Only care about others providing witness capabilities if there is a softfork
1397+        // defined.
1398+        nRelevantServices = ServiceFlags(nRelevantServices | NODE_WITNESS);
    rebroad commented at 4:05 am on September 16, 2016:
    So witness nodes won’t make outbound connections to non-witness nodes? This puts a reliance on witness nodes that can receive inbound connections as these will be the only witness nodes talking to the non-witness nodes.
  64. in src/main.cpp: in f8528134fc 
    661@@ -655,6 +662,9 @@ CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& loc
662             CBlockIndex* pindex = (*mi).second;
663             if (chain.Contains(pindex))
664                 return pindex;
665+            if (pindex->GetAncestor(chain.Height()) == chain.Tip()) {
666+                return chain.Tip();
    jprupp commented at 1:23 pm on November 2, 2016:
    I understand this as: if we have a block hash that the caller sent us in our mapBlockIndex that is not in the main chain, but is a descendant of our best block, then send our best block. Is there a case where our mapBlockIndex knows about a block that is higher than the tip of our chain? Are these data structures not kept in sync so that all the blocks in mapBlockIndex are also in chain?
  65. sipa commented at 4:29 pm on November 2, 2016: member
    Since headers first sync (v0.10.0) we very regularly have blocks in mapBlockIndex that are not yet in the main chain. We first receive them as headers, validate them and add them to mapBlockIndex. Then we use this information to determine which full blocks to download from which peers, and when we have them, they become candidates to be added to the main chain.
  66. sdaftuar commented at 5:45 pm on November 2, 2016: member

    @sipa @xenog: FYI the specific change to FindForkInGlobalIndex() was precipitated by the introduction of RewindBlockIndex(), which causes nodes that upgrade after segwit activation to disconnect blocks back to the activation point, so that the witness-version of those blocks could then be redownloaded and reconnected. During that window, mapBlockIndex will be (far) ahead of chainActive.Tip().

    If I remember correctly, the wallet rescan logic uses FindForkInGlobalIndex, and prior to this change, it might return a block older than was necessary (which could be a problem for pruning nodes).

  67. zkbot referenced this in commit e6850571dd on Feb 7, 2018
  68. zkbot referenced this in commit 84c19b8a87 on Feb 8, 2018
  69. zkbot referenced this in commit e1f3a15fdc on Feb 8, 2018
  70. zkbot referenced this in commit eb3128ab0a on Feb 19, 2018
  71. zkbot referenced this in commit 6db10127a9 on Feb 20, 2018
  72. zkbot referenced this in commit 8487be8360 on Feb 20, 2018
  73. MarcoFalke referenced this in commit c65c77c721 on Apr 25, 2019
  74. sidhujag referenced this in commit fd6ff32bc6 on Apr 27, 2019
  75. str4d referenced this in commit 5c76b1df64 on Nov 14, 2019
  76. str4d referenced this in commit de2eddd21d on Dec 3, 2019
  77. str4d referenced this in commit 3c8c8009db on Dec 4, 2019
  78. fanquake deleted a comment on Apr 26, 2020
  79. random-zebra referenced this in commit 5a092159f6 on Aug 5, 2020
  80. KolbyML referenced this in commit 69a5889173 on Apr 17, 2021
  81. KolbyML referenced this in commit 8575e22844 on Apr 19, 2021
  82. kittywhiskers referenced this in commit 1c7f00ccde on May 25, 2021
  83. kittywhiskers referenced this in commit 5246a86646 on May 25, 2021
  84. kittywhiskers referenced this in commit 931e3e73d8 on May 25, 2021
  85. UdjinM6 referenced this in commit 05e5ee1be6 on May 28, 2021
  86. UdjinM6 referenced this in commit 95209acdc3 on May 28, 2021
  87. MarcoFalke referenced this in commit f2e41d1109 on Aug 1, 2021
  88. sidhujag referenced this in commit 37caae2f93 on Aug 1, 2021
  89. zkbot referenced this in commit 1d7ed06174 on Aug 13, 2021
  90. zkbot referenced this in commit 56b5f95897 on Aug 17, 2021
  91. christiancfifi referenced this in commit e63db06138 on Aug 24, 2021
  92. christiancfifi referenced this in commit 6590ec9b49 on Aug 24, 2021
  93. christiancfifi referenced this in commit 2f94d70034 on Aug 25, 2021
  94. christiancfifi referenced this in commit c436fd7d5b on Aug 26, 2021
  95. christiancfifi referenced this in commit eb71903958 on Aug 28, 2021
  96. christiancfifi referenced this in commit 1b8d4e607e on Aug 29, 2021
  97. christiancfifi referenced this in commit ecf066ebaa on Aug 29, 2021
  98. MarcoFalke locked this on Feb 20, 2022
  99. MarcoFalke deleted a comment on Feb 20, 2022
  100. MarcoFalke deleted a comment on Feb 20, 2022


sipa kanzure sdaftuar jonasschnelli NicolasDorier sneurlax laanwj skaag dcousens petertodd rebroad jprupp

Labels
P2P Consensus

Milestone
0.13.0

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: 2024-11-21 18:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me