diff options
author | W. J. van der Laan <laanwj@protonmail.com> | 2021-07-22 17:36:32 +0200 |
---|---|---|
committer | W. J. van der Laan <laanwj@protonmail.com> | 2021-07-22 17:36:38 +0200 |
commit | 5d83e7d714e0b7cafeb2f14892184a9607cb55dc (patch) | |
tree | 2ad7ddc14d1b30eb9595395a0ac66098e826df03 /src | |
parent | bfa52cbddfaa0e6734f8d229a55a1d2971dea594 (diff) | |
parent | a806647d260132a00cd633160040625c7dd17803 (diff) | |
download | bitcoin-5d83e7d714e0b7cafeb2f14892184a9607cb55dc.tar.xz |
Merge bitcoin/bitcoin#21090: Default to NODE_WITNESS in nLocalServices
a806647d260132a00cd633160040625c7dd17803 [validation] Always include merkle root in coinbase commitment (Dhruv Mehta)
189128c220190a588500b8e74ee7ae47671b9558 [validation] Set witness script flag with p2sh for blocks (Dhruv Mehta)
ac82b99db77ec843af82dcdf040dfdbc98c8ff26 [p2p] remove redundant NODE_WITNESS checks (Dhruv Mehta)
6f8b198b8256a6703a6f5e592dfa77fa024a7035 [p2p] remove unused segwitheight=-1 option (Dhruv Mehta)
eba5b1cd6460c98e75d0422bd394e12af7f11e4c [test] remove or move tests using `-segwitheight=-1` (Dhruv Mehta)
Pull request description:
Builds on #21009 and makes progress on remaining items in #17862
Removing `RewindBlockIndex()` in #21009 allows the following:
- removal of tests using `segwitheight=-1` in `p2p_segwit.py`.
- move `test_upgrade_after_activation()` out of `p2p_segwit.py` reducing runtime
- in turn, that allows us to drop support for `-segwitheight=-1`, which is only supported for that test.
- that allows us to always set `NODE_WITNESS` in our local services. The only reason we don't do that is to support `-segwitheight=-1`.
- that in turn allows us to drop all of the `GetLocalServices() & NODE_WITNESS` checks inside `net_processing.cpp`, since our local services would always include `NODE_WITNESS`
ACKs for top commit:
mzumsande:
Code-Review ACK a806647d260132a00cd633160040625c7dd17803
laanwj:
Code review ACK a806647d260132a00cd633160040625c7dd17803, nice cleanup
jnewbery:
utACK a806647d260132a00cd633160040625c7dd17803
theStack:
ACK a806647d260132a00cd633160040625c7dd17803
Tree-SHA512: 73e1a69d1d7eca1f5c38558ec6672decd0b60b16c2ef6134df6f6af71bb159e6eea160f9bb5ab0eb6723c6632d29509811e29469d0d87abbe9b69a2890fbc73e
Diffstat (limited to 'src')
-rw-r--r-- | src/chainparams.cpp | 7 | ||||
-rw-r--r-- | src/chainparamsbase.cpp | 2 | ||||
-rw-r--r-- | src/init.cpp | 8 | ||||
-rw-r--r-- | src/net_processing.cpp | 14 | ||||
-rw-r--r-- | src/validation.cpp | 45 |
5 files changed, 28 insertions, 48 deletions
diff --git a/src/chainparams.cpp b/src/chainparams.cpp index eb6c382f23..a8b45685d0 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -490,11 +490,8 @@ void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args) { if (args.IsArgSet("-segwitheight")) { int64_t height = args.GetArg("-segwitheight", consensus.SegwitHeight); - if (height < -1 || height >= std::numeric_limits<int>::max()) { - throw std::runtime_error(strprintf("Activation height %ld for segwit is out of valid range. Use -1 to disable segwit.", height)); - } else if (height == -1) { - LogPrintf("Segwit disabled for testing\n"); - height = std::numeric_limits<int>::max(); + if (height < 0 || height >= std::numeric_limits<int>::max()) { + throw std::runtime_error(strprintf("Activation height %ld for segwit is out of valid range.", height)); } consensus.SegwitHeight = static_cast<int>(height); } diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index e71b4bc859..79c1bc25bc 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -20,7 +20,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-chain=<chain>", "Use the chain <chain> (default: main). Allowed values: main, test, signet, regtest", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-regtest", "Enter regression test mode, which uses a special chain in which blocks can be solved instantly. " "This is intended for regression testing tools and app development. Equivalent to -chain=regtest.", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); - argsman.AddArg("-segwitheight=<n>", "Set the activation height of segwit. -1 to disable. (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-segwitheight=<n>", "Set the activation height of segwit. (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-testnet", "Use the test chain. Equivalent to -chain=test.", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-vbparams=deployment:start:end[:min_activation_height]", "Use given start/end times and min_activation_height for specified version bits deployment (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-signet", "Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); diff --git a/src/init.cpp b/src/init.cpp index 3915fa8fcb..75394d96b1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -715,7 +715,7 @@ namespace { // Variables internal to initialization process only int nMaxConnections; int nUserMaxConnections; int nFD; -ServiceFlags nLocalServices = ServiceFlags(NODE_NETWORK | NODE_NETWORK_LIMITED); +ServiceFlags nLocalServices = ServiceFlags(NODE_NETWORK | NODE_NETWORK_LIMITED | NODE_WITNESS); int64_t peer_connect_timeout; std::set<BlockFilterType> g_enabled_filter_types; @@ -1588,12 +1588,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } - if (DeploymentEnabled(chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) { - // Advertise witness capabilities. - // The option to not set NODE_WITNESS is only used in the tests and should be removed. - nLocalServices = ServiceFlags(nLocalServices | NODE_WITNESS); - } - // ********************************************************* Step 11: import blocks if (!CheckDiskSpace(gArgs.GetDataDirNet())) { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 44f9879a23..a0c346b99f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -877,7 +877,7 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) } m_connman.ForNode(nodeid, [this](CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); - uint64_t nCMPCTBLOCKVersion = (pfrom->GetLocalServices() & NODE_WITNESS) ? 2 : 1; + uint64_t nCMPCTBLOCKVersion = 2; if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { // As per BIP152, we only get 3 of our peers to announce // blocks using compact encodings. @@ -1973,7 +1973,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic static uint32_t GetFetchFlags(const CNode& pfrom) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { uint32_t nFetchFlags = 0; - if ((pfrom.GetLocalServices() & NODE_WITNESS) && State(pfrom.GetId())->fHaveWitness) { + if (State(pfrom.GetId())->fHaveWitness) { nFetchFlags |= MSG_WITNESS_FLAG; } return nFetchFlags; @@ -2688,8 +2688,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // they may wish to request compact blocks from us bool fAnnounceUsingCMPCTBLOCK = false; uint64_t nCMPCTBLOCKVersion = 2; - if (pfrom.GetLocalServices() & NODE_WITNESS) - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); nCMPCTBLOCKVersion = 1; m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion)); } @@ -2707,7 +2706,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, bool fAnnounceUsingCMPCTBLOCK = false; uint64_t nCMPCTBLOCKVersion = 0; vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion; - if (nCMPCTBLOCKVersion == 1 || ((pfrom.GetLocalServices() & NODE_WITNESS) && nCMPCTBLOCKVersion == 2)) { + if (nCMPCTBLOCKVersion == 1 || nCMPCTBLOCKVersion == 2) { LOCK(cs_main); // fProvidesHeaderAndIDs is used to "lock in" version of compact blocks we send (fWantsCmpctWitness) if (!State(pfrom.GetId())->fProvidesHeaderAndIDs) { @@ -2721,10 +2720,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, pfrom.m_bip152_highbandwidth_from = fAnnounceUsingCMPCTBLOCK; } if (!State(pfrom.GetId())->fSupportsDesiredCmpctVersion) { - if (pfrom.GetLocalServices() & NODE_WITNESS) - State(pfrom.GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 2); - else - State(pfrom.GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 1); + State(pfrom.GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 2); } } return; diff --git a/src/validation.cpp b/src/validation.cpp index f81c27e8e3..842687e4d6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1645,13 +1645,8 @@ static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consens pindex->phashBlock == nullptr || // this is a new candidate block, eg from TestBlockValidity() *pindex->phashBlock != consensusparams.BIP16Exception) // this block isn't the historical exception { - flags |= SCRIPT_VERIFY_P2SH; - } - - // Enforce WITNESS rules whenever P2SH is in effect (and the segwit - // deployment is defined). - if (flags & SCRIPT_VERIFY_P2SH && DeploymentEnabled(consensusparams, Consensus::DEPLOYMENT_SEGWIT)) { - flags |= SCRIPT_VERIFY_WITNESS; + // Enforce WITNESS rules whenever P2SH is in effect + flags |= SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS; } // Enforce the DERSIG (BIP66) rule @@ -3101,25 +3096,23 @@ std::vector<unsigned char> GenerateCoinbaseCommitment(CBlock& block, const CBloc std::vector<unsigned char> commitment; int commitpos = GetWitnessCommitmentIndex(block); std::vector<unsigned char> ret(32, 0x00); - if (DeploymentEnabled(consensusParams, Consensus::DEPLOYMENT_SEGWIT)) { - if (commitpos == NO_WITNESS_COMMITMENT) { - uint256 witnessroot = BlockWitnessMerkleRoot(block, nullptr); - CHash256().Write(witnessroot).Write(ret).Finalize(witnessroot); - CTxOut out; - out.nValue = 0; - out.scriptPubKey.resize(MINIMUM_WITNESS_COMMITMENT); - out.scriptPubKey[0] = OP_RETURN; - out.scriptPubKey[1] = 0x24; - out.scriptPubKey[2] = 0xaa; - out.scriptPubKey[3] = 0x21; - out.scriptPubKey[4] = 0xa9; - out.scriptPubKey[5] = 0xed; - memcpy(&out.scriptPubKey[6], witnessroot.begin(), 32); - commitment = std::vector<unsigned char>(out.scriptPubKey.begin(), out.scriptPubKey.end()); - CMutableTransaction tx(*block.vtx[0]); - tx.vout.push_back(out); - block.vtx[0] = MakeTransactionRef(std::move(tx)); - } + if (commitpos == NO_WITNESS_COMMITMENT) { + uint256 witnessroot = BlockWitnessMerkleRoot(block, nullptr); + CHash256().Write(witnessroot).Write(ret).Finalize(witnessroot); + CTxOut out; + out.nValue = 0; + out.scriptPubKey.resize(MINIMUM_WITNESS_COMMITMENT); + out.scriptPubKey[0] = OP_RETURN; + out.scriptPubKey[1] = 0x24; + out.scriptPubKey[2] = 0xaa; + out.scriptPubKey[3] = 0x21; + out.scriptPubKey[4] = 0xa9; + out.scriptPubKey[5] = 0xed; + memcpy(&out.scriptPubKey[6], witnessroot.begin(), 32); + commitment = std::vector<unsigned char>(out.scriptPubKey.begin(), out.scriptPubKey.end()); + CMutableTransaction tx(*block.vtx[0]); + tx.vout.push_back(out); + block.vtx[0] = MakeTransactionRef(std::move(tx)); } UpdateUncommittedBlockStructures(block, pindexPrev, consensusParams); return commitment; |