From 9eec4cc2e10fd83c42f6e29eaccd54682f75c157 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 19 Nov 2020 10:12:52 +0100 Subject: [PATCH 01/10] Merge #20288: script, doc: contrib/seeds updates 961f148cb1b4caab86b4354357999e03433b04b1 doc: update contrib/seeds/README dnspython installation info (Jon Atack) dd7b5f46d85401254630abf6976f59b5b8eed181 script: fix deprecation warning in makeseeds.py (Jon Atack) Pull request description: Seen while reviewing #20237. 1. Fix a deprecation warning in `contrib/seeds/makeseeds.py` ``` makeseeds.py:139: DeprecationWarning: please use dns.resolver.resolve() instead asn = int([x.to_text() for x in dns.resolver.query('.'.join( ``` - Per https://dnspython.readthedocs.io/en/latest/whatsnew.html, `dns.resolver.query()` was deprecated in `dnspython` version 2.0.0. - See https://dnspython.readthedocs.io/en/latest/resolver-class.html for more info on the resolver class. 2. Update the `dnspython` dependency installation instructions in `contrib/seeds/README` - The markdown rendering can be seen here: https://github.com/jonatack/bitcoin/tree/contrib-seeds-fixups/contrib/seeds ACKs for top commit: laanwj: code review ACK 961f148cb1b4caab86b4354357999e03433b04b1 Tree-SHA512: f9c4f318a1a0d35b8de147d24b72c534a1f58eece31e7cfa00b4149a63b6a618d8ca0312f52fd8056f3c645cf2ee68574ca02319fddffdad919a70cd33395d33 --- contrib/seeds/README.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/contrib/seeds/README.md b/contrib/seeds/README.md index 85dfaa8d12..c4b678604c 100644 --- a/contrib/seeds/README.md +++ b/contrib/seeds/README.md @@ -26,7 +26,12 @@ that the list is as expected. ## Dependencies -Ubuntu: +Ubuntu, Debian: - sudo apt-get install python3-pip - pip3 install dnspython + sudo apt-get install python3-dnspython + +and/or for other operating systems: + + pip3 install dnspython3 + +See https://dnspython.readthedocs.io/en/latest/installation.html for more information. From 455bb2e117e39c71bc3f8f68d55653ef0cb41264 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 19 Nov 2020 17:59:17 +0100 Subject: [PATCH 02/10] Merge #20329: docs/descriptors.md: Remove hardened marker in the path after xpub dc80a7d0b0817b43d41af3a08b5800c425ebd5d8 docs/descriptors.md: Remove hardened marker in the path after xpub (Dmitry Petukhov) Pull request description: As described in "Key origin identification" section, a descriptor that has hardened derivation after xpub does not let you compute scripts without access to the corresponding private keys. Such a descriptor is practically useless. The text after the descriptor said "with child key *1'/2* of the specified xpub", and clearly an xpub cannot have "child key" with hardened derivation. Therefore it makes sense to fix this inconsistency to not confuse the reader of the doc Top commit has no ACKs. Tree-SHA512: 9f35951d90868585303681c27ea2ef9d36d4036181e337cfbd48730de0c346320b08dd089350b116d4c8542f3b506868cf318796bb713e5f80600ccbd96f9970 --- doc/descriptors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/descriptors.md b/doc/descriptors.md index f8040e823d..24af88396e 100644 --- a/doc/descriptors.md +++ b/doc/descriptors.md @@ -40,7 +40,7 @@ Output descriptors currently support: - `combo(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)` describes any P2PK, P2PKH with the specified public key. - `multi(1,022f8bde4d1a07209355b4a7250a5c5128e88b84bddc619ab7cba8d569b240efe4,025cbdf0646e5db4eaa398f365f2ea7a0e3d419b7e0330e39ce92bddedcac4f9bc)` describes a bare *1-of-2* multisig with the specified public key. - `sortedmulti(1,022f8bde4d1a07209355b4a7250a5c5128e88b84bddc619ab7cba8d569b240efe4,025cbdf0646e5db4eaa398f365f2ea7a0e3d419b7e0330e39ce92bddedcac4f9bc)` describes a bare *1-of-2* multisig with keys sorted lexicographically in the resulting redeemScript. -- `pkh(xpub68Gmy5EdvgibQVfPdqkBBCHxA5htiqg55crXYuXoQRKfDBFA1WEjWgP6LHhwBZeNK1VTsfTFUHCdrfp1bgwQ9xv5ski8PX9rL2dZXvgGDnw/1'/2)` describes a P2PKH output with child key *1'/2* of the specified xpub. +- `pkh(xpub68Gmy5EdvgibQVfPdqkBBCHxA5htiqg55crXYuXoQRKfDBFA1WEjWgP6LHhwBZeNK1VTsfTFUHCdrfp1bgwQ9xv5ski8PX9rL2dZXvgGDnw/1/2)` describes a P2PKH output with child key *1/2* of the specified xpub. - `pkh([d34db33f/44'/0'/0']xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1/*)` describes a set of P2PKH outputs, but additionally specifies that the specified xpub is a child of a master with fingerprint `d34db33f`, and derived using path `44'/0'/0'`. ## Reference From d01973cc08693f8426552439a2ee8005a3ac7c03 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 30 Nov 2020 15:45:37 +0800 Subject: [PATCH 03/10] Merge #20491: refactor: Drop noop gcc version checks 830ddf413934226d0b6ca99165916790cc52ca18 Drop noop gcc version checks (Hennadii Stepanov) Pull request description: Since #20413 the minimum required GCC version is 7. ACKs for top commit: fanquake: ACK 830ddf413934226d0b6ca99165916790cc52ca18 Tree-SHA512: 36264661d6ced1683a0c907efba7c700502acaf8e9fd50d9066bc9c7b877b25165b0684c2d7fe74bd58e500a77d7702bdbdd53691c274f29e4abccd241c10964 --- src/test/fuzz/addition_overflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp index 71bda68a7a..372c1a370e 100644 --- a/src/test/fuzz/addition_overflow.cpp +++ b/src/test/fuzz/addition_overflow.cpp @@ -15,7 +15,7 @@ #if __has_builtin(__builtin_add_overflow) #define HAVE_BUILTIN_ADD_OVERFLOW #endif -#elif defined(__GNUC__) && (__GNUC__ >= 5) +#elif defined(__GNUC__) #define HAVE_BUILTIN_ADD_OVERFLOW #endif From def356e2cbdae6a4c6ca9372ca9283aea795cd43 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Tue, 1 Dec 2020 11:23:19 +0100 Subject: [PATCH 04/10] Merge #20512: doc: Add bash as an OpenBSD dependency 1d578c078f0ce00cb032d3c6c689fd199b8d2f35 doc: Add bash as an OpenBSD dependency (Emil Engler) Pull request description: If we require Python for the test framework, we should also require bash. It is required for the linters and other scripts and does not comes in a default OpenBSD installation. ACKs for top commit: practicalswift: ACK 1d578c078f0ce00cb032d3c6c689fd199b8d2f35 RiccardoMasutti: ACK 1d578c0 theStack: ACK 1d578c078f0ce00cb032d3c6c689fd199b8d2f35 Tree-SHA512: ef14e801983093a99d4c28d134adbc589ca06e5437bf1ff34f8e593968c59793e9d99e8a48f577f847acdfc5185e193276526894b4c632c59d66d87939977910 --- doc/build-openbsd.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/build-openbsd.md b/doc/build-openbsd.md index 93fafa76d2..bbda6de352 100644 --- a/doc/build-openbsd.md +++ b/doc/build-openbsd.md @@ -16,6 +16,7 @@ pkg_add autoconf # (select highest version, e.g. 2.69) pkg_add automake # (select highest version, e.g. 1.15) pkg_add python # (select highest version, e.g. 3.8) pkg_add gmp +pkg_add bash pkg_add boost git clone https://github.com/dashpay/dash.git From d186b3714aa1aa330be3ef6efcb4c339d2eaa456 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 10 Dec 2020 14:40:51 +0100 Subject: [PATCH 05/10] Merge #20587: [doc] Tidy up Tor doc (more stringent) 32045bbfd5d77513efc162be8d4e24ea67539e27 [doc] Tidy up Tor doc (more stringent) (wodry) Pull request description: This is a follow up to https://github.com/bitcoin/bitcoin/pull/19638 that left two deprecated "hidden service/server" naming occurences. It also shall make the chapter titles regarding creation of onion services stringent and easy to read and distinguish. It removes the one and only reference to the testnet (here the testnet onion service port), as it is not explained that it references to the testnet and I do not know why it is mentioned there. It is only confusing. Also, as said, the testnet is not referenced at any other place in this document. ACKs for top commit: practicalswift: ACK 32045bbfd5d77513efc162be8d4e24ea67539e27 laanwj: Review ACK 32045bbfd5d77513efc162be8d4e24ea67539e27 RiccardoMasutti: ACK 32045bb Tree-SHA512: c623387b76d68845c0fa47f67a6f8ef70c9c560e3f8f8754e45a4f51e43198c2092be789588acd4ada607f42fbe62d51a4b1888d81b225df19b6557a081835c0 --- doc/tor.md | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/tor.md b/doc/tor.md index a54093ec31..66bc8e309e 100644 --- a/doc/tor.md +++ b/doc/tor.md @@ -164,7 +164,6 @@ versions of Tor see [Section 4](#4-automatically-listen-on-tor).* HiddenServiceDir /var/lib/tor/dashcore-service/ HiddenServicePort 9999 127.0.0.1:9996 - HiddenServicePort 19999 127.0.0.1:19996 The directory can be different of course, but virtual port numbers should be equal to your dashd's P2P listen port (9999 by default), and target addresses and ports From 41505e64aa358134536f095fed32aa7c3728088c Mon Sep 17 00:00:00 2001 From: fanquake Date: Sun, 13 Dec 2020 09:50:03 +0800 Subject: [PATCH 06/10] Merge #20588: Remove unused and confusing CTransaction constructor fac39c198324715565897f4240709340477af0bf wallet: document that tx in CreateTransaction is purely an out-param (MarcoFalke) faac31521bb7ecbf999541cf918d3750ff589de4 Remove unused and confusing CTransaction constructor (MarcoFalke) Pull request description: The constructor is confusing and dangerous (as explained in the TODO), fix that by removing it. ACKs for top commit: laanwj: Code review ACK fac39c198324715565897f4240709340477af0bf promag: Code review ACK fac39c198324715565897f4240709340477af0bf. theStack: Code review ACK fac39c198324715565897f4240709340477af0bf Tree-SHA512: e0c8cffce8d8ee0166b8e1cbfe85ed0657611e26e2af0d69fde70eceaa5d75cbde3eb489af0428fe4fc431360b4c791fb1cc21b8dee7d4c7a4f17df00836229d --- src/bench/duplicate_inputs.cpp | 2 +- src/blockencodings.cpp | 2 +- src/coinjoin/coinjoin.h | 5 ++--- src/primitives/transaction.cpp | 2 -- src/primitives/transaction.h | 8 ++------ src/test/fuzz/script_sigcache.cpp | 2 +- src/test/fuzz/transaction.cpp | 2 +- src/wallet/test/coinjoin_tests.cpp | 2 +- src/wallet/wallet.cpp | 3 ++- 9 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/bench/duplicate_inputs.cpp b/src/bench/duplicate_inputs.cpp index 19da79cb56..173a2faca9 100644 --- a/src/bench/duplicate_inputs.cpp +++ b/src/bench/duplicate_inputs.cpp @@ -44,7 +44,7 @@ static void DuplicateInputs(benchmark::Bench& bench) naughtyTx.vout[0].nValue = 0; naughtyTx.vout[0].scriptPubKey = SCRIPT_PUB; - uint64_t n_inputs = (((MaxBlockSize() / ::GetSerializeSize(CTransaction(), PROTOCOL_VERSION)) - (CTransaction(coinbaseTx).GetTotalSize() + CTransaction(naughtyTx).GetTotalSize())) / 41) - 100; + uint64_t n_inputs = (((MaxBlockSize() / ::GetSerializeSize(CMutableTransaction(), PROTOCOL_VERSION)) - (CTransaction(coinbaseTx).GetTotalSize() + CTransaction(naughtyTx).GetTotalSize())) / 41) - 100; for (uint64_t x = 0; x < (n_inputs - 1); ++x) { naughtyTx.vin.emplace_back(GetRandHash(), 0, CScript(), 0); } diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index ec169725d8..567d04d54c 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -16,7 +16,7 @@ #include -#define MIN_TRANSACTION_SIZE (::GetSerializeSize(CTransaction(), PROTOCOL_VERSION)) +#define MIN_TRANSACTION_SIZE (::GetSerializeSize(CMutableTransaction(), PROTOCOL_VERSION)) CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block) : nonce(GetRand(std::numeric_limits::max())), diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index f2214a329b..20bbbd808d 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -152,7 +152,7 @@ public: CService addr; CCoinJoinEntry() : - txCollateral(MakeTransactionRef()) + txCollateral(MakeTransactionRef(CMutableTransaction{})) { } @@ -246,9 +246,8 @@ public: uint256 m_protxHash; std::vector vchSig; int64_t sigTime{0}; - CCoinJoinBroadcastTx() : - tx(MakeTransactionRef()) + tx(MakeTransactionRef(CMutableTransaction{})) { } diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 61c488a5f1..feb97c0894 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -92,8 +92,6 @@ uint256 CTransaction::ComputeHash() const return SerializeHash(*this); } -/* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */ -CTransaction::CTransaction() : vin(), vout(), nVersion(CTransaction::CURRENT_VERSION), nType(TRANSACTION_NORMAL), nLockTime(0), hash{} {} CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash{ComputeHash()} {} CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash{ComputeHash()} {} diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 73f2e50e3b..cc0652a40b 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -212,12 +212,9 @@ private: uint256 ComputeHash() const; public: - /** Construct a CTransaction that qualifies as IsNull() */ - CTransaction(); - /** Convert a CMutableTransaction into a CTransaction. */ - explicit CTransaction(const CMutableTransaction &tx); - CTransaction(CMutableTransaction &&tx); + explicit CTransaction(const CMutableTransaction& tx); + CTransaction(CMutableTransaction&& tx); template inline void Serialize(Stream& s) const { @@ -319,7 +316,6 @@ struct CMutableTransaction }; typedef std::shared_ptr CTransactionRef; -static inline CTransactionRef MakeTransactionRef() { return std::make_shared(); } template static inline CTransactionRef MakeTransactionRef(Tx&& txIn) { return std::make_shared(std::forward(txIn)); } /** Implementation of BIP69 diff --git a/src/test/fuzz/script_sigcache.cpp b/src/test/fuzz/script_sigcache.cpp index c2b38be391..9fb76ae793 100644 --- a/src/test/fuzz/script_sigcache.cpp +++ b/src/test/fuzz/script_sigcache.cpp @@ -28,7 +28,7 @@ FUZZ_TARGET_INIT(script_sigcache, initialize_script_sigcache) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); const std::optional mutable_transaction = ConsumeDeserializable(fuzzed_data_provider); - const CTransaction tx = mutable_transaction ? CTransaction{*mutable_transaction} : CTransaction{}; + const CTransaction tx{mutable_transaction ? *mutable_transaction : CMutableTransaction{}}; const unsigned int n_in = fuzzed_data_provider.ConsumeIntegral(); const CAmount amount = ConsumeMoney(fuzzed_data_provider); const bool store = fuzzed_data_provider.ConsumeBool(); diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index ae9cc9b4b1..3665b465ac 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -41,7 +41,7 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) return CTransaction(deserialize, ds); } catch (const std::ios_base::failure&) { valid_tx = false; - return CTransaction(); + return CTransaction{CMutableTransaction{}}; } }(); bool valid_mutable_tx = true; diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index ed9fe3285d..e53dc78eb8 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -173,7 +173,6 @@ public: CompactTallyItem GetTallyItem(const std::vector& vecAmounts) { CompactTallyItem tallyItem; - CTransactionRef tx; ReserveDestination reserveDest(wallet.get()); CAmount nFeeRet; int nChangePosRet = -1; @@ -185,6 +184,7 @@ public: BOOST_CHECK(reserveDest.GetReservedDestination(tallyItem.txdest, false)); } for (CAmount nAmount : vecAmounts) { + CTransactionRef tx; BOOST_CHECK(wallet->CreateTransaction({{GetScriptForDestination(tallyItem.txdest), nAmount, false}}, tx, nFeeRet, nChangePosRet, strError, coinControl)); { LOCK2(wallet->cs_wallet, cs_main); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7e57de0a5f..6be105ba06 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3760,7 +3760,7 @@ bool CWallet::CreateTransaction( int nExtraPayloadSize) { int nChangePosIn = nChangePosInOut; - CTransactionRef tx2 = tx; + Assert(!tx); // tx is an out-param. TODO change the return type from bool to tx (or nullptr) bool res = CreateTransactionInternal(vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, sign, nExtraPayloadSize); // try with avoidpartialspends unless it's enabled already if (res && nFeeRet > 0 /* 0 means non-functional fee rate estimation */ && m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { @@ -3774,6 +3774,7 @@ bool CWallet::CreateTransaction( } CAmount nFeeRet2; + CTransactionRef tx2; int nChangePosInOut2 = nChangePosIn; bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results if (CreateTransactionInternal(vecSend, tx2, nFeeRet2, nChangePosInOut2, error2, tmp_cc, sign, nExtraPayloadSize)) { From b193c63fedc7422c28177272fa9581014d18b3d2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 15 Dec 2020 22:06:18 +0100 Subject: [PATCH 07/10] Merge #20611: Move TX_MAX_STANDARD_VERSION to policy fade6195b1c230edd561443637a7bde81c2594a4 Move TX_MAX_STANDARD_VERSION to policy (MarcoFalke) Pull request description: `primitives` should only be used for the raw datastructures (parsing and format). It is not the right place to document relay policy. ACKs for top commit: laanwj: Code review ACK fade6195b1c230edd561443637a7bde81c2594a4 lontivero: Concept ACK https://github.com/bitcoin/bitcoin/pull/20611/commits/fade6195b1c230edd561443637a7bde81c2594a4 Tree-SHA512: f809c4aecd14d7e9feaa7b50b9c0697232991eef36190cd960bcfb0ad6e20c71a4f6aab48c7747cf8a681eb14feda60c55b09a37f128673d519567224f29cd97 --- src/bitcoin-tx.cpp | 4 +++- src/policy/policy.cpp | 2 +- src/policy/policy.h | 25 ++++++++++++++++--------- src/primitives/transaction.h | 6 ------ 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index a1b1d983ad..a599a2ed25 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include