diff options
Diffstat (limited to 'src')
47 files changed, 318 insertions, 166 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index feed4a0061..e542a067a4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -191,6 +191,7 @@ BITCOIN_CORE_H = \ kernel/mempool_options.h \ kernel/mempool_persist.h \ kernel/mempool_removal_reason.h \ + kernel/messagestartchars.h \ kernel/notifications_interface.h \ kernel/validation_cache_sizes.h \ key.h \ diff --git a/src/addrdb.cpp b/src/addrdb.cpp index c5b474339b..50f576624c 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -90,10 +90,10 @@ void DeserializeDB(Stream& stream, Data&& data, bool fCheckSum = true) { HashVerifier verifier{stream}; // de-serialize file header (network specific magic number) and .. - unsigned char pchMsgTmp[4]; + MessageStartChars pchMsgTmp; verifier >> pchMsgTmp; // ... verify the network matches ours - if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) { + if (pchMsgTmp != Params().MessageStart()) { throw std::runtime_error{"Invalid network magic number"}; } diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 4d032cefc5..0737144b84 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -6,6 +6,7 @@ #include <consensus/validation.h> #include <crypto/sha256.h> #include <node/miner.h> +#include <random.h> #include <test/util/mining.h> #include <test/util/script.h> #include <test/util/setup_common.h> diff --git a/src/bench/duplicate_inputs.cpp b/src/bench/duplicate_inputs.cpp index b3799ad1b7..a1c8d4d942 100644 --- a/src/bench/duplicate_inputs.cpp +++ b/src/bench/duplicate_inputs.cpp @@ -7,6 +7,7 @@ #include <consensus/merkle.h> #include <consensus/validation.h> #include <pow.h> +#include <random.h> #include <test/util/setup_common.h> #include <txmempool.h> #include <validation.h> diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index 1f94461d19..993db66653 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -5,6 +5,7 @@ #include <bench/bench.h> #include <kernel/mempool_entry.h> #include <policy/policy.h> +#include <random.h> #include <test/util/setup_common.h> #include <txmempool.h> #include <util/chaintype.h> diff --git a/src/bench/streams_findbyte.cpp b/src/bench/streams_findbyte.cpp index 77f5940926..22b8f1b356 100644 --- a/src/bench/streams_findbyte.cpp +++ b/src/bench/streams_findbyte.cpp @@ -16,7 +16,7 @@ static void FindByte(benchmark::Bench& bench) data[file_size-1] = 1; fwrite(&data, sizeof(uint8_t), file_size, file); rewind(file); - CBufferedFile bf(file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size, 0, 0); + BufferedFile bf{file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size, 0}; bench.run([&] { bf.SetPos(0); diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 19c4d36126..fc83a4ad3a 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -22,6 +22,7 @@ #include <node/blockstorage.h> #include <node/caches.h> #include <node/chainstate.h> +#include <random.h> #include <scheduler.h> #include <script/sigcache.h> #include <util/chaintype.h> diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp index 582c18c9c6..8b68d04b2b 100644 --- a/src/bitcoin-util.cpp +++ b/src/bitcoin-util.cpp @@ -17,6 +17,7 @@ #include <core_io.h> #include <streams.h> #include <util/exception.h> +#include <util/strencodings.h> #include <util/translation.h> #include <version.h> diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index c95937ba75..775496e21b 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -4,7 +4,6 @@ #include <dbwrapper.h> -#include <clientversion.h> #include <logging.h> #include <random.h> #include <serialize.h> @@ -156,9 +155,9 @@ struct CDBBatch::WriteBatchImpl { leveldb::WriteBatch batch; }; -CDBBatch::CDBBatch(const CDBWrapper& _parent) : parent(_parent), - m_impl_batch{std::make_unique<CDBBatch::WriteBatchImpl>()}, - ssValue(SER_DISK, CLIENT_VERSION){}; +CDBBatch::CDBBatch(const CDBWrapper& _parent) + : parent{_parent}, + m_impl_batch{std::make_unique<CDBBatch::WriteBatchImpl>()} {}; CDBBatch::~CDBBatch() = default; @@ -168,7 +167,7 @@ void CDBBatch::Clear() size_estimate = 0; } -void CDBBatch::WriteImpl(Span<const std::byte> key, CDataStream& ssValue) +void CDBBatch::WriteImpl(Span<const std::byte> key, DataStream& ssValue) { leveldb::Slice slKey(CharCast(key.data()), key.size()); ssValue.Xor(dbwrapper_private::GetObfuscateKey(parent)); diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 2f7448e878..63c2f99d2a 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -80,11 +80,11 @@ private: const std::unique_ptr<WriteBatchImpl> m_impl_batch; DataStream ssKey{}; - CDataStream ssValue; + DataStream ssValue{}; size_t size_estimate{0}; - void WriteImpl(Span<const std::byte> key, CDataStream& ssValue); + void WriteImpl(Span<const std::byte> key, DataStream& ssValue); void EraseImpl(Span<const std::byte> key); public: diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index 3c9ec84e24..6b38e19d81 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -79,7 +79,7 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe return false; } - CAutoFile file(m_chainstate->m_blockman.OpenBlockFile(postx, true), SER_DISK, CLIENT_VERSION); + CAutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true), CLIENT_VERSION}; if (file.IsNull()) { return error("%s: OpenBlockFile failed", __func__); } diff --git a/src/init.cpp b/src/init.cpp index f0847bd4f7..6dd3d5970b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -510,7 +510,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-networkactive", "Enable all P2P network activity (default: 1). Can be changed by the setnetworkactive RPC command", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-timeout=<n>", strprintf("Specify socket connection timeout in milliseconds. If an initial attempt to connect is unsuccessful after this amount of time, drop it (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peertimeout=<n>", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); - argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control host and port to use if onion listening enabled (default: %s). If no port is specified, the default port of %i will be used.", DEFAULT_TOR_CONTROL, DEFAULT_TOR_CONTROL_PORT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION); #ifdef USE_UPNP #if USE_UPNP @@ -618,7 +618,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-rpccookiefile=<loc>", "Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); argsman.AddArg("-rpcport=<port>", strprintf("Listen for JSON-RPC connections on <port> (default: %u, testnet: %u, signet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), signetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::RPC); - argsman.AddArg("-rpcserialversion", strprintf("Sets the serialization of raw transaction or block hex returned in non-verbose mode, non-segwit(0) or segwit(1) (default: %d)", DEFAULT_RPC_SERIALIZE_VERSION), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); + argsman.AddArg("-rpcserialversion", strprintf("Sets the serialization of raw transaction or block hex returned in non-verbose mode, non-segwit(0) (DEPRECATED) or segwit(1) (default: %d)", DEFAULT_RPC_SERIALIZE_VERSION), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcservertimeout=<n>", strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_SERVER_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC); argsman.AddArg("-rpcthreads=<n>", strprintf("Set the number of threads to service RPC calls (default: %d)", DEFAULT_HTTP_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); @@ -988,6 +988,10 @@ bool AppInitParameterInteraction(const ArgsManager& args) if (args.GetIntArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) > 1) return InitError(Untranslated("Unknown rpcserialversion requested.")); + if (args.GetIntArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) == 0 && !IsDeprecatedRPCEnabled("serialversion")) { + return InitError(Untranslated("-rpcserialversion=0 is deprecated and will be removed in the future. Specify -deprecatedrpc=serialversion to allow anyway.")); + } + // Also report errors from parsing before daemonization { kernel::Notifications notifications{}; @@ -1219,7 +1223,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.connman); node.connman = std::make_unique<CConnman>(GetRand<uint64_t>(), GetRand<uint64_t>(), - *node.addrman, *node.netgroupman, args.GetBoolArg("-networkactive", true)); + *node.addrman, *node.netgroupman, chainparams, args.GetBoolArg("-networkactive", true)); assert(!node.fee_estimator); // Don't initialize fee estimation with old data if we don't relay transactions, diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 733a3339b3..6cee379faf 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -10,6 +10,7 @@ #include <consensus/merkle.h> #include <consensus/params.h> #include <hash.h> +#include <kernel/messagestartchars.h> #include <logging.h> #include <primitives/block.h> #include <primitives/transaction.h> @@ -359,7 +360,7 @@ public: HashWriter h{}; h << consensus.signet_challenge; uint256 hash = h.GetHash(); - memcpy(pchMessageStart, hash.begin(), 4); + std::copy_n(hash.begin(), 4, pchMessageStart.begin()); nDefaultPort = 38333; nPruneAfterHeight = 1000; diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h index 2d38af609c..63837bb23e 100644 --- a/src/kernel/chainparams.h +++ b/src/kernel/chainparams.h @@ -7,9 +7,8 @@ #define BITCOIN_KERNEL_CHAINPARAMS_H #include <consensus/params.h> -#include <netaddress.h> +#include <kernel/messagestartchars.h> #include <primitives/block.h> -#include <protocol.h> #include <uint256.h> #include <util/chaintype.h> #include <util/hash_type.h> @@ -87,17 +86,8 @@ public: }; const Consensus::Params& GetConsensus() const { return consensus; } - const CMessageHeader::MessageStartChars& MessageStart() const { return pchMessageStart; } + const MessageStartChars& MessageStart() const { return pchMessageStart; } uint16_t GetDefaultPort() const { return nDefaultPort; } - uint16_t GetDefaultPort(Network net) const - { - return net == NET_I2P ? I2P_SAM31_PORT : GetDefaultPort(); - } - uint16_t GetDefaultPort(const std::string& addr) const - { - CNetAddr a; - return a.SetSpecial(addr) ? GetDefaultPort(a.GetNetwork()) : GetDefaultPort(); - } const CBlock& GenesisBlock() const { return genesis; } /** Default value for -checkmempool and -checkblockindex argument */ @@ -165,7 +155,7 @@ protected: CChainParams() {} Consensus::Params consensus; - CMessageHeader::MessageStartChars pchMessageStart; + MessageStartChars pchMessageStart; uint16_t nDefaultPort; uint64_t nPruneAfterHeight; uint64_t m_assumed_blockchain_size; diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index 6be07da222..ff655c5ffa 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -41,7 +41,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active if (load_path.empty()) return false; FILE* filestr{opts.mockable_fopen_function(load_path, "rb")}; - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + CAutoFile file{filestr, CLIENT_VERSION}; if (file.IsNull()) { LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); return false; @@ -157,7 +157,7 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock return false; } - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + CAutoFile file{filestr, CLIENT_VERSION}; uint64_t version = MEMPOOL_DUMP_VERSION; file << version; diff --git a/src/kernel/messagestartchars.h b/src/kernel/messagestartchars.h new file mode 100644 index 0000000000..829616dc8b --- /dev/null +++ b/src/kernel/messagestartchars.h @@ -0,0 +1,13 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_KERNEL_MESSAGESTARTCHARS_H +#define BITCOIN_KERNEL_MESSAGESTARTCHARS_H + +#include <array> +#include <cstdint> + +using MessageStartChars = std::array<uint8_t, 4>; + +#endif // BITCOIN_KERNEL_MESSAGESTARTCHARS_H diff --git a/src/key_io.cpp b/src/key_io.cpp index a061165613..1a0b51a28b 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -8,6 +8,7 @@ #include <bech32.h> #include <script/interpreter.h> #include <script/solver.h> +#include <tinyformat.h> #include <util/strencodings.h> #include <algorithm> diff --git a/src/net.cpp b/src/net.cpp index af2855932d..bfa2738e45 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -470,8 +470,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo Ticks<HoursDouble>(pszDest ? 0h : Now<NodeSeconds>() - addrConnect.nTime)); // Resolve - const uint16_t default_port{pszDest != nullptr ? Params().GetDefaultPort(pszDest) : - Params().GetDefaultPort()}; + const uint16_t default_port{pszDest != nullptr ? GetDefaultPort(pszDest) : + m_params.GetDefaultPort()}; if (pszDest) { const std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)}; if (!resolved.empty()) { @@ -730,7 +730,7 @@ V1Transport::V1Transport(const NodeId node_id, int nTypeIn, int nVersionIn) noex m_node_id(node_id), hdrbuf(nTypeIn, nVersionIn), vRecv(nTypeIn, nVersionIn) { assert(std::size(Params().MessageStart()) == std::size(m_magic_bytes)); - std::copy(std::begin(Params().MessageStart()), std::end(Params().MessageStart()), m_magic_bytes); + m_magic_bytes = Params().MessageStart(); LOCK(m_recv_mutex); Reset(); } @@ -759,7 +759,7 @@ int V1Transport::readHeader(Span<const uint8_t> msg_bytes) } // Check start string, network magic - if (memcmp(hdr.pchMessageStart, m_magic_bytes, CMessageHeader::MESSAGE_START_SIZE) != 0) { + if (hdr.pchMessageStart != m_magic_bytes) { LogPrint(BCLog::NET, "Header error: Wrong MessageStart %s received, peer=%d\n", HexStr(hdr.pchMessageStart), m_node_id); return -1; } @@ -1144,7 +1144,7 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept // they receive our uniformly random key and garbage, but detecting this case specially // means we can log it. static constexpr std::array<uint8_t, 12> MATCH = {'v', 'e', 'r', 's', 'i', 'o', 'n', 0, 0, 0, 0, 0}; - static constexpr size_t OFFSET = sizeof(CMessageHeader::MessageStartChars); + static constexpr size_t OFFSET = std::tuple_size_v<MessageStartChars>; if (!m_initiating && m_recv_buffer.size() >= OFFSET + MATCH.size()) { if (std::equal(MATCH.begin(), MATCH.end(), m_recv_buffer.begin() + OFFSET)) { LogPrint(BCLog::NET, "V2 transport error: V1 peer with wrong MessageStart %s\n", @@ -2184,7 +2184,7 @@ void CConnman::WakeMessageHandler() void CConnman::ThreadDNSAddressSeed() { FastRandomContext rng; - std::vector<std::string> seeds = Params().DNSSeeds(); + std::vector<std::string> seeds = m_params.DNSSeeds(); Shuffle(seeds.begin(), seeds.end(), rng); int seeds_right_now = 0; // Number of seeds left before testing if we have enough connections int found = 0; @@ -2275,7 +2275,7 @@ void CConnman::ThreadDNSAddressSeed() const auto addresses{LookupHost(host, nMaxIPs, true)}; if (!addresses.empty()) { for (const CNetAddr& ip : addresses) { - CAddress addr = CAddress(CService(ip, Params().GetDefaultPort()), requiredServiceBits); + CAddress addr = CAddress(CService(ip, m_params.GetDefaultPort()), requiredServiceBits); addr.nTime = rng.rand_uniform_delay(Now<NodeSeconds>() - 3 * 24h, -4 * 24h); // use a random age between 3 and 7 days old vAdd.push_back(addr); found++; @@ -2480,7 +2480,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) } if (add_fixed_seeds_now) { - std::vector<CAddress> seed_addrs{ConvertSeeds(Params().FixedSeeds())}; + std::vector<CAddress> seed_addrs{ConvertSeeds(m_params.FixedSeeds())}; // We will not make outgoing connections to peers that are unreachable // (e.g. because of -onlynet configuration). // Therefore, we do not add them to addrman in the first place. @@ -2769,7 +2769,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo() const } for (const std::string& strAddNode : lAddresses) { - CService service(LookupNumeric(strAddNode, Params().GetDefaultPort(strAddNode))); + CService service(LookupNumeric(strAddNode, GetDefaultPort(strAddNode))); AddedNodeInfo addedNode{strAddNode, CService(), false, false}; if (service.IsValid()) { // strAddNode is an IP:port @@ -3075,11 +3075,12 @@ void CConnman::SetNetworkActive(bool active) } CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, AddrMan& addrman_in, - const NetGroupManager& netgroupman, bool network_active) + const NetGroupManager& netgroupman, const CChainParams& params, bool network_active) : addrman(addrman_in) , m_netgroupman{netgroupman} , nSeed0(nSeed0In) , nSeed1(nSeed1In) + , m_params(params) { SetTryNewOutboundPeer(false); @@ -3093,6 +3094,16 @@ NodeId CConnman::GetNewNodeId() return nLastNodeId.fetch_add(1, std::memory_order_relaxed); } +uint16_t CConnman::GetDefaultPort(Network net) const +{ + return net == NET_I2P ? I2P_SAM31_PORT : m_params.GetDefaultPort(); +} + +uint16_t CConnman::GetDefaultPort(const std::string& addr) const +{ + CNetAddr a; + return a.SetSpecial(addr) ? GetDefaultPort(a.GetNetwork()) : m_params.GetDefaultPort(); +} bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlags permissions) { @@ -10,15 +10,16 @@ #include <chainparams.h> #include <common/bloom.h> #include <compat/compat.h> -#include <node/connection_types.h> #include <consensus/amount.h> #include <crypto/siphash.h> #include <hash.h> #include <i2p.h> +#include <kernel/messagestartchars.h> #include <net_permissions.h> #include <netaddress.h> #include <netbase.h> #include <netgroup.h> +#include <node/connection_types.h> #include <policy/feerate.h> #include <protocol.h> #include <random.h> @@ -46,6 +47,7 @@ class AddrMan; class BanMan; +class CChainParams; class CNode; class CScheduler; struct bilingual_str; @@ -360,7 +362,7 @@ public: class V1Transport final : public Transport { private: - CMessageHeader::MessageStartChars m_magic_bytes; + MessageStartChars m_magic_bytes; const NodeId m_node_id; // Only for logging mutable Mutex m_recv_mutex; //!< Lock for receive state mutable CHash256 hasher GUARDED_BY(m_recv_mutex); @@ -1080,7 +1082,7 @@ public: } CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, const NetGroupManager& netgroupman, - bool network_active = true); + const CChainParams& params, bool network_active = true); ~CConnman(); @@ -1356,6 +1358,9 @@ private: // Whether the node should be passed out in ForEach* callbacks static bool NodeFullyConnected(const CNode* pnode); + uint16_t GetDefaultPort(Network net) const; + uint16_t GetDefaultPort(const std::string& addr) const; + // Network usage totals mutable Mutex m_total_bytes_sent_mutex; std::atomic<uint64_t> nTotalBytesRecv{0}; @@ -1565,6 +1570,8 @@ private: std::vector<CNode*> m_nodes_copy; }; + const CChainParams& m_params; + friend struct ConnmanTestMsg; }; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 70f11be586..f21c94c0a0 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -11,6 +11,7 @@ #include <flatfile.h> #include <hash.h> #include <kernel/chainparams.h> +#include <kernel/messagestartchars.h> #include <logging.h> #include <pow.h> #include <reverse_iterator.h> @@ -21,6 +22,7 @@ #include <util/batchpriority.h> #include <util/fs.h> #include <util/signalinterrupt.h> +#include <util/strencodings.h> #include <util/translation.h> #include <validation.h> @@ -822,7 +824,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const { // Open history file to append - CAutoFile fileout(OpenBlockFile(pos), SER_DISK, CLIENT_VERSION); + CAutoFile fileout{OpenBlockFile(pos), CLIENT_VERSION}; if (fileout.IsNull()) { return error("WriteBlockToDisk: OpenBlockFile failed"); } @@ -878,7 +880,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons block.SetNull(); // Open history file to read - CAutoFile filein(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION); + CAutoFile filein{OpenBlockFile(pos, true), CLIENT_VERSION}; if (filein.IsNull()) { return error("ReadBlockFromDisk: OpenBlockFile failed for %s", pos.ToString()); } @@ -927,12 +929,12 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF } try { - CMessageHeader::MessageStartChars blk_start; + MessageStartChars blk_start; unsigned int blk_size; filein >> blk_start >> blk_size; - if (memcmp(blk_start, GetParams().MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) { + if (blk_start != GetParams().MessageStart()) { return error("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(), HexStr(blk_start), HexStr(GetParams().MessageStart())); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index c79fd2c6a1..b251ece31f 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -11,7 +11,7 @@ #include <kernel/blockmanager_opts.h> #include <kernel/chainparams.h> #include <kernel/cs_main.h> -#include <protocol.h> +#include <kernel/messagestartchars.h> #include <sync.h> #include <util/fs.h> #include <util/hasher.h> @@ -73,7 +73,7 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB /** Size of header written by WriteBlockToDisk before a serialized CBlock */ -static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAGE_START_SIZE + sizeof(unsigned int); +static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = std::tuple_size_v<MessageStartChars> + sizeof(unsigned int); extern std::atomic_bool fReindex; diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp index 6e70a94088..a901ef8f38 100644 --- a/src/policy/packages.cpp +++ b/src/policy/packages.cpp @@ -37,6 +37,13 @@ bool CheckPackage(const Package& txns, PackageValidationState& state) std::unordered_set<uint256, SaltedTxidHasher> later_txids; std::transform(txns.cbegin(), txns.cend(), std::inserter(later_txids, later_txids.end()), [](const auto& tx) { return tx->GetHash(); }); + + // Package must not contain any duplicate transactions, which is checked by txid. This also + // includes transactions with duplicate wtxids and same-txid-different-witness transactions. + if (later_txids.size() != txns.size()) { + return state.Invalid(PackageValidationResult::PCKG_POLICY, "package-contains-duplicates"); + } + for (const auto& tx : txns) { for (const auto& input : tx->vin) { if (later_txids.find(input.prevout.hash) != later_txids.end()) { diff --git a/src/protocol.cpp b/src/protocol.cpp index 2105480c72..cb956191e4 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -92,7 +92,7 @@ const static std::vector<std::string> g_all_net_message_types{ CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn) { - memcpy(pchMessageStart, pchMessageStartIn, MESSAGE_START_SIZE); + pchMessageStart = pchMessageStartIn; // Copy the command name size_t i = 0; diff --git a/src/protocol.h b/src/protocol.h index a7ca0c6f3e..22e2108afb 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -6,6 +6,7 @@ #ifndef BITCOIN_PROTOCOL_H #define BITCOIN_PROTOCOL_H +#include <kernel/messagestartchars.h> // IWYU pragma: export #include <netaddress.h> #include <primitives/transaction.h> #include <serialize.h> @@ -13,6 +14,7 @@ #include <uint256.h> #include <util/time.h> +#include <array> #include <cstdint> #include <limits> #include <string> @@ -26,14 +28,12 @@ class CMessageHeader { public: - static constexpr size_t MESSAGE_START_SIZE = 4; static constexpr size_t COMMAND_SIZE = 12; static constexpr size_t MESSAGE_SIZE_SIZE = 4; static constexpr size_t CHECKSUM_SIZE = 4; - static constexpr size_t MESSAGE_SIZE_OFFSET = MESSAGE_START_SIZE + COMMAND_SIZE; + static constexpr size_t MESSAGE_SIZE_OFFSET = std::tuple_size_v<MessageStartChars> + COMMAND_SIZE; static constexpr size_t CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE; - static constexpr size_t HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE; - typedef unsigned char MessageStartChars[MESSAGE_START_SIZE]; + static constexpr size_t HEADER_SIZE = std::tuple_size_v<MessageStartChars> + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE; explicit CMessageHeader() = default; @@ -47,7 +47,7 @@ public: SERIALIZE_METHODS(CMessageHeader, obj) { READWRITE(obj.pchMessageStart, obj.pchCommand, obj.nMessageSize, obj.pchChecksum); } - char pchMessageStart[MESSAGE_START_SIZE]{}; + MessageStartChars pchMessageStart{}; char pchCommand[COMMAND_SIZE]{}; uint32_t nMessageSize{std::numeric_limits<uint32_t>::max()}; uint8_t pchChecksum[CHECKSUM_SIZE]{}; diff --git a/src/random.cpp b/src/random.cpp index 51b8b3ad9d..9bd8ff9f1a 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -5,6 +5,7 @@ #include <random.h> +#include <compat/compat.h> #include <compat/cpuid.h> #include <crypto/chacha20.h> #include <crypto/sha256.h> diff --git a/src/randomenv.cpp b/src/randomenv.cpp index 581612bccf..da81a61651 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -10,6 +10,7 @@ #include <randomenv.h> #include <clientversion.h> +#include <compat/compat.h> #include <compat/cpuid.h> #include <crypto/sha512.h> #include <support/cleanse.h> diff --git a/src/serialize.h b/src/serialize.h index 2d790190a0..f1595077e9 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -285,6 +285,7 @@ template<typename Stream> inline void Serialize(Stream& s, int64_t a ) { ser_wri template<typename Stream> inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); } template<typename Stream, int N> inline void Serialize(Stream& s, const char (&a)[N]) { s.write(MakeByteSpan(a)); } template<typename Stream, int N> inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(MakeByteSpan(a)); } +template <typename Stream, typename B, std::size_t N> void Serialize(Stream& s, const std::array<B, N>& a) { (void)/* force byte-type */UCharCast(a.data()); s.write(MakeByteSpan(a)); } template <typename Stream, typename B> void Serialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); } #ifndef CHAR_EQUALS_INT8 @@ -301,6 +302,7 @@ template<typename Stream> inline void Unserialize(Stream& s, int64_t& a ) { a = template<typename Stream> inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); } template<typename Stream, int N> inline void Unserialize(Stream& s, char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } template<typename Stream, int N> inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(MakeWritableByteSpan(a)); } +template <typename Stream, typename B, std::size_t N> void Unserialize(Stream& s, std::array<B, N>& a) { (void)/* force byte-type */UCharCast(a.data()); s.read(MakeWritableByteSpan(a)); } template <typename Stream, typename B> void Unserialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.read(AsWritableBytes(span)); } template <typename Stream> inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); } diff --git a/src/streams.h b/src/streams.h index 5ff952be76..f9a817c9b6 100644 --- a/src/streams.h +++ b/src/streams.h @@ -550,12 +550,10 @@ public: class CAutoFile : public AutoFile { private: - const int nType; const int nVersion; public: - explicit CAutoFile(std::FILE* file, int type, int version, std::vector<std::byte> data_xor = {}) : AutoFile{file, std::move(data_xor)}, nType{type}, nVersion{version} {} - int GetType() const { return nType; } + explicit CAutoFile(std::FILE* file, int version, std::vector<std::byte> data_xor = {}) : AutoFile{file, std::move(data_xor)}, nVersion{version} {} int GetVersion() const { return nVersion; } template<typename T> @@ -579,10 +577,9 @@ public: * Will automatically close the file when it goes out of scope if not null. * If you need to close the file early, use file.fclose() instead of fclose(file). */ -class CBufferedFile +class BufferedFile { private: - const int nType; const int nVersion; FILE *src; //!< source file @@ -603,7 +600,7 @@ private: return false; size_t nBytes = fread((void*)&vchBuf[pos], 1, readNow, src); if (nBytes == 0) { - throw std::ios_base::failure(feof(src) ? "CBufferedFile::Fill: end of file" : "CBufferedFile::Fill: fread failed"); + throw std::ios_base::failure(feof(src) ? "BufferedFile::Fill: end of file" : "BufferedFile::Fill: fread failed"); } nSrcPos += nBytes; return true; @@ -632,25 +629,24 @@ private: } public: - CBufferedFile(FILE* fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nTypeIn, int nVersionIn) - : nType(nTypeIn), nVersion(nVersionIn), nReadLimit(std::numeric_limits<uint64_t>::max()), nRewind(nRewindIn), vchBuf(nBufSize, std::byte{0}) + BufferedFile(FILE* fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nVersionIn) + : nVersion{nVersionIn}, nReadLimit{std::numeric_limits<uint64_t>::max()}, nRewind{nRewindIn}, vchBuf(nBufSize, std::byte{0}) { if (nRewindIn >= nBufSize) throw std::ios_base::failure("Rewind limit must be less than buffer size"); src = fileIn; } - ~CBufferedFile() + ~BufferedFile() { fclose(); } // Disallow copies - CBufferedFile(const CBufferedFile&) = delete; - CBufferedFile& operator=(const CBufferedFile&) = delete; + BufferedFile(const BufferedFile&) = delete; + BufferedFile& operator=(const BufferedFile&) = delete; int GetVersion() const { return nVersion; } - int GetType() const { return nType; } void fclose() { @@ -715,7 +711,7 @@ public: } template<typename T> - CBufferedFile& operator>>(T&& obj) { + BufferedFile& operator>>(T&& obj) { ::Unserialize(*this, obj); return (*this); } diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 8c1182b5e1..6e740a4f53 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -142,7 +142,7 @@ static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerM BOOST_AUTO_TEST_CASE(stale_tip_peer_management) { NodeId id{0}; - auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS; @@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_AUTO_TEST_CASE(block_relay_only_eviction) { NodeId id{0}; - auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {}); constexpr int max_outbound_block_relay{MAX_BLOCK_RELAY_ONLY_CONNECTIONS}; @@ -305,7 +305,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) LOCK(NetEventsInterface::g_msgproc_mutex); auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, {}); CNetAddr tor_netaddr; @@ -407,7 +407,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) LOCK(NetEventsInterface::g_msgproc_mutex); auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, {}); banman->ClearBanned(); diff --git a/src/test/fuzz/buffered_file.cpp b/src/test/fuzz/buffered_file.cpp index 2f7ce60c7f..1116274e3d 100644 --- a/src/test/fuzz/buffered_file.cpp +++ b/src/test/fuzz/buffered_file.cpp @@ -18,10 +18,10 @@ FUZZ_TARGET(buffered_file) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; FuzzedFileProvider fuzzed_file_provider = ConsumeFile(fuzzed_data_provider); - std::optional<CBufferedFile> opt_buffered_file; + std::optional<BufferedFile> opt_buffered_file; FILE* fuzzed_file = fuzzed_file_provider.open(); try { - opt_buffered_file.emplace(fuzzed_file, fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegral<int>(), fuzzed_data_provider.ConsumeIntegral<int>()); + opt_buffered_file.emplace(fuzzed_file, fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegral<int>()); } catch (const std::ios_base::failure&) { if (fuzzed_file != nullptr) { fclose(fuzzed_file); @@ -62,7 +62,6 @@ FUZZ_TARGET(buffered_file) }); } opt_buffered_file->GetPos(); - opt_buffered_file->GetType(); opt_buffered_file->GetVersion(); } } diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index cdf240dc59..e46e085ee7 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -36,6 +36,7 @@ FUZZ_TARGET(connman, .init = initialize_connman) fuzzed_data_provider.ConsumeIntegral<uint64_t>(), *g_setup->m_node.addrman, *g_setup->m_node.netgroupman, + Params(), fuzzed_data_provider.ConsumeBool()}; CNetAddr random_netaddr; CNode random_node = ConsumeNode(fuzzed_data_provider); diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp index 26c219d6c8..57129a60b8 100644 --- a/src/test/fuzz/descriptor_parse.cpp +++ b/src/test/fuzz/descriptor_parse.cpp @@ -8,6 +8,7 @@ #include <script/descriptor.h> #include <test/fuzz/fuzz.h> #include <util/chaintype.h> +#include <util/strencodings.h> //! Types are raw (un)compressed pubkeys, raw xonly pubkeys, raw privkeys (WIF), xpubs, xprvs. static constexpr uint8_t KEY_TYPES_COUNT{6}; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 3eb7bdec62..34d7867079 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1114,7 +1114,7 @@ public: } /** Send V1 version message header to the transport. */ - void SendV1Version(const CMessageHeader::MessageStartChars& magic) + void SendV1Version(const MessageStartChars& magic) { CMessageHeader hdr(magic, "version", 126 + InsecureRandRange(11)); CDataStream ser(SER_NETWORK, CLIENT_VERSION); diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 589a2fd766..99740ee779 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -260,7 +260,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) // The buffer size (second arg) must be greater than the rewind // amount (third arg). try { - CBufferedFile bfbad(file, 25, 25, 222, 333); + BufferedFile bfbad{file, 25, 25, 333}; BOOST_CHECK(false); } catch (const std::exception& e) { BOOST_CHECK(strstr(e.what(), @@ -268,11 +268,10 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) } // The buffer is 25 bytes, allow rewinding 10 bytes. - CBufferedFile bf(file, 25, 10, 222, 333); + BufferedFile bf{file, 25, 10, 333}; BOOST_CHECK(!bf.eof()); - // These two members have no functional effect. - BOOST_CHECK_EQUAL(bf.GetType(), 222); + // This member has no functional effect. BOOST_CHECK_EQUAL(bf.GetVersion(), 333); uint8_t i; @@ -357,7 +356,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) BOOST_CHECK(false); } catch (const std::exception& e) { BOOST_CHECK(strstr(e.what(), - "CBufferedFile::Fill: end of file") != nullptr); + "BufferedFile::Fill: end of file") != nullptr); } // Attempting to read beyond the end sets the EOF indicator. BOOST_CHECK(bf.eof()); @@ -392,7 +391,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_skip) rewind(file); // The buffer is 25 bytes, allow rewinding 10 bytes. - CBufferedFile bf(file, 25, 10, 222, 333); + BufferedFile bf{file, 25, 10, 333}; uint8_t i; // This is like bf >> (7-byte-variable), in that it will cause data @@ -446,7 +445,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) size_t bufSize = InsecureRandRange(300) + 1; size_t rewindSize = InsecureRandRange(bufSize); - CBufferedFile bf(file, bufSize, rewindSize, 222, 333); + BufferedFile bf{file, bufSize, rewindSize, 333}; size_t currentPos = 0; size_t maxPos = 0; for (int step = 0; step < 100; ++step) { diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index df5f8b4cce..10ab656d38 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -65,6 +65,17 @@ BOOST_FIXTURE_TEST_CASE(package_sanitization_tests, TestChain100Setup) BOOST_CHECK(!CheckPackage(package_too_large, state_too_large)); BOOST_CHECK_EQUAL(state_too_large.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(state_too_large.GetRejectReason(), "package-too-large"); + + // Packages can't contain transactions with the same txid. + Package package_duplicate_txids_empty; + for (auto i{0}; i < 3; ++i) { + CMutableTransaction empty_tx; + package_duplicate_txids_empty.emplace_back(MakeTransactionRef(empty_tx)); + } + PackageValidationState state_duplicates; + BOOST_CHECK(!CheckPackage(package_duplicate_txids_empty, state_duplicates)); + BOOST_CHECK_EQUAL(state_duplicates.GetResult(), PackageValidationResult::PCKG_POLICY); + BOOST_CHECK_EQUAL(state_duplicates.GetRejectReason(), "package-contains-duplicates"); } BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) @@ -809,18 +820,20 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup) expected_pool_size += 1; BOOST_CHECK_MESSAGE(submit_rich_parent.m_state.IsInvalid(), "Package validation unexpectedly succeeded"); - // The child would have been validated on its own and failed, then submitted as a "package" of 1. + // The child would have been validated on its own and failed. BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetResult(), PackageValidationResult::PCKG_TX); BOOST_CHECK_EQUAL(submit_rich_parent.m_state.GetRejectReason(), "transaction failed"); auto it_parent = submit_rich_parent.m_tx_results.find(tx_parent_rich->GetWitnessHash()); + auto it_child = submit_rich_parent.m_tx_results.find(tx_child_poor->GetWitnessHash()); BOOST_CHECK(it_parent != submit_rich_parent.m_tx_results.end()); + BOOST_CHECK(it_child != submit_rich_parent.m_tx_results.end()); BOOST_CHECK(it_parent->second.m_result_type == MempoolAcceptResult::ResultType::VALID); + BOOST_CHECK(it_child->second.m_result_type == MempoolAcceptResult::ResultType::INVALID); BOOST_CHECK(it_parent->second.m_state.GetRejectReason() == ""); BOOST_CHECK_MESSAGE(it_parent->second.m_base_fees.value() == high_parent_fee, strprintf("rich parent: expected fee %s, got %s", high_parent_fee, it_parent->second.m_base_fees.value())); BOOST_CHECK(it_parent->second.m_effective_feerate == CFeeRate(high_parent_fee, GetVirtualTransactionSize(*tx_parent_rich))); - auto it_child = submit_rich_parent.m_tx_results.find(tx_child_poor->GetWitnessHash()); BOOST_CHECK(it_child != submit_rich_parent.m_tx_results.end()); BOOST_CHECK_EQUAL(it_child->second.m_result_type, MempoolAcceptResult::ResultType::INVALID); BOOST_CHECK_EQUAL(it_child->second.m_state.GetResult(), TxValidationResult::TX_MEMPOOL_POLICY); diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index c1f6226982..790eabc7c1 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -4,6 +4,7 @@ #include <consensus/validation.h> #include <key.h> +#include <random.h> #include <script/sign.h> #include <script/signingprovider.h> #include <test/util/setup_common.h> diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 331199709e..2947bc3fcb 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -253,7 +253,7 @@ TestingSetup::TestingSetup( /*deterministic=*/false, m_node.args->GetIntArg("-checkaddrman", 0)); m_node.banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME); - m_node.connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Deterministic randomness for tests. + m_node.connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); // Deterministic randomness for tests. PeerManager::Options peerman_opts; ApplyArgsManOptions(*m_node.args, peerman_opts); m_node.peerman = PeerManager::make(*m_node.connman, *m_node.addrman, diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 9cf976a700..4c99aa5746 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -42,8 +42,8 @@ #include <event2/thread.h> #include <event2/util.h> -/** Default control port */ -const std::string DEFAULT_TOR_CONTROL = "127.0.0.1:9051"; +/** Default control ip and port */ +const std::string DEFAULT_TOR_CONTROL = "127.0.0.1:" + ToString(DEFAULT_TOR_CONTROL_PORT); /** Tor cookie size (from control-spec.txt) */ static const int TOR_COOKIE_SIZE = 32; /** Size of client/server nonce for SAFECOOKIE */ @@ -144,7 +144,7 @@ bool TorControlConnection::Connect(const std::string& tor_control_center, const Disconnect(); } - const std::optional<CService> control_service{Lookup(tor_control_center, 9051, fNameLookup)}; + const std::optional<CService> control_service{Lookup(tor_control_center, DEFAULT_TOR_CONTROL_PORT, fNameLookup)}; if (!control_service.has_value()) { LogPrintf("tor: Failed to look up control center %s\n", tor_control_center); return false; diff --git a/src/torcontrol.h b/src/torcontrol.h index 1a9065b01a..4a0eef223e 100644 --- a/src/torcontrol.h +++ b/src/torcontrol.h @@ -19,6 +19,7 @@ #include <string> #include <vector> +constexpr int DEFAULT_TOR_CONTROL_PORT = 9051; extern const std::string DEFAULT_TOR_CONTROL; static const bool DEFAULT_LISTEN_ONION = true; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 226dd9f353..92379484e3 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -993,6 +993,7 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { if (ptx) { if (outpoint.n < ptx->vout.size()) { coin = Coin(ptx->vout[outpoint.n], MEMPOOL_HEIGHT, false); + m_non_base_coins.emplace(outpoint); return true; } else { return false; @@ -1005,8 +1006,14 @@ void CCoinsViewMemPool::PackageAddTransaction(const CTransactionRef& tx) { for (unsigned int n = 0; n < tx->vout.size(); ++n) { m_temp_added.emplace(COutPoint(tx->GetHash(), n), Coin(tx->vout[n], MEMPOOL_HEIGHT, false)); + m_non_base_coins.emplace(COutPoint(tx->GetHash(), n)); } } +void CCoinsViewMemPool::Reset() +{ + m_temp_added.clear(); + m_non_base_coins.clear(); +} size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); diff --git a/src/txmempool.h b/src/txmempool.h index 869612a4a2..fcef19e807 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -826,15 +826,27 @@ class CCoinsViewMemPool : public CCoinsViewBacked * validation, since we can access transaction outputs without submitting them to mempool. */ std::unordered_map<COutPoint, Coin, SaltedOutpointHasher> m_temp_added; + + /** + * Set of all coins that have been fetched from mempool or created using PackageAddTransaction + * (not base). Used to track the origin of a coin, see GetNonBaseCoins(). + */ + mutable std::unordered_set<COutPoint, SaltedOutpointHasher> m_non_base_coins; protected: const CTxMemPool& mempool; public: CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn); + /** GetCoin, returning whether it exists and is not spent. Also updates m_non_base_coins if the + * coin is not fetched from base. */ bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; /** Add the coins created by this transaction. These coins are only temporarily stored in * m_temp_added and cannot be flushed to the back end. Only used for package validation. */ void PackageAddTransaction(const CTransactionRef& tx); + /** Get all coins in m_non_base_coins. */ + std::unordered_set<COutPoint, SaltedOutpointHasher> GetNonBaseCoins() const { return m_non_base_coins; } + /** Clear m_temp_added and m_non_base_coins. */ + void Reset(); }; /** diff --git a/src/util/time.h b/src/util/time.h index b6aab615ba..6aa776137c 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -6,8 +6,6 @@ #ifndef BITCOIN_UTIL_TIME_H #define BITCOIN_UTIL_TIME_H -#include <compat/compat.h> - #include <chrono> // IWYU pragma: export #include <cstdint> #include <string> diff --git a/src/validation.cpp b/src/validation.cpp index e3a00e4241..1d4786bb17 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -23,6 +23,7 @@ #include <hash.h> #include <kernel/chainparams.h> #include <kernel/mempool_entry.h> +#include <kernel/messagestartchars.h> #include <kernel/notifications_interface.h> #include <logging.h> #include <logging/timer.h> @@ -456,7 +457,7 @@ public: * any transaction spending the same inputs as a transaction in the mempool is considered * a conflict. */ const bool m_allow_replacement; - /** When true, the mempool will not be trimmed when individual transactions are submitted in + /** When true, the mempool will not be trimmed when any transactions are submitted in * Finalize(). Instead, limits should be enforced at the end to ensure the package is not * partially submitted. */ @@ -517,7 +518,7 @@ public: /* m_coins_to_uncache */ package_args.m_coins_to_uncache, /* m_test_accept */ package_args.m_test_accept, /* m_allow_replacement */ true, - /* m_package_submission */ false, + /* m_package_submission */ true, // do not LimitMempoolSize in Finalize() /* m_package_feerates */ false, // only 1 transaction }; } @@ -556,6 +557,19 @@ public: PackageMempoolAcceptResult AcceptMultipleTransactions(const std::vector<CTransactionRef>& txns, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** + * Submission of a subpackage. + * If subpackage size == 1, calls AcceptSingleTransaction() with adjusted ATMPArgs to avoid + * package policy restrictions like no CPFP carve out (PackageMempoolChecks) and disabled RBF + * (m_allow_replacement), and creates a PackageMempoolAcceptResult wrapping the result. + * + * If subpackage size > 1, calls AcceptMultipleTransactions() with the provided ATMPArgs. + * + * Also cleans up all non-chainstate coins from m_view at the end. + */ + PackageMempoolAcceptResult AcceptSubPackage(const std::vector<CTransactionRef>& subpackage, ATMPArgs& args) + EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); + + /** * Package (more specific than just multiple transactions) acceptance. Package must be a child * with all of its unconfirmed parents, and topologically sorted. */ @@ -640,10 +654,9 @@ private: // Submit all transactions to the mempool and call ConsensusScriptChecks to add to the script // cache - should only be called after successful validation of all transactions in the package. - // The package may end up partially-submitted after size limiting; returns true if all - // transactions are successfully added to the mempool, false otherwise. + // Does not call LimitMempoolSize(), so mempool max_size_bytes may be temporarily exceeded. bool SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& workspaces, PackageValidationState& package_state, - std::map<const uint256, const MempoolAcceptResult>& results) + std::map<uint256, MempoolAcceptResult>& results) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Compare a package's feerate against minimum allowed. @@ -1125,7 +1138,7 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& workspaces, PackageValidationState& package_state, - std::map<const uint256, const MempoolAcceptResult>& results) + std::map<uint256, MempoolAcceptResult>& results) { AssertLockHeld(cs_main); AssertLockHeld(m_pool.cs); @@ -1180,32 +1193,21 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& } } - // It may or may not be the case that all the transactions made it into the mempool. Regardless, - // make sure we haven't exceeded max mempool size. - LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip()); - std::vector<uint256> all_package_wtxids; all_package_wtxids.reserve(workspaces.size()); std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); - // Find the wtxids of the transactions that made it into the mempool. Allow partial submission, - // but don't report success unless they all made it into the mempool. + + // Add successful results. The returned results may change later if LimitMempoolSize() evicts them. for (Workspace& ws : workspaces) { const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate : CFeeRate{ws.m_modified_fees, static_cast<uint32_t>(ws.m_vsize)}; const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids : std::vector<uint256>({ws.m_ptx->GetWitnessHash()}); - if (m_pool.exists(GenTxid::Wtxid(ws.m_ptx->GetWitnessHash()))) { - results.emplace(ws.m_ptx->GetWitnessHash(), - MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, - ws.m_base_fees, effective_feerate, effective_feerate_wtxids)); - GetMainSignals().TransactionAddedToMempool(ws.m_ptx, m_pool.GetAndIncrementSequence()); - } else { - all_submitted = false; - ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full"); - package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); - results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); - } + results.emplace(ws.m_ptx->GetWitnessHash(), + MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, + ws.m_base_fees, effective_feerate, effective_feerate_wtxids)); + GetMainSignals().TransactionAddedToMempool(ws.m_ptx, m_pool.GetAndIncrementSequence()); } return all_submitted; } @@ -1255,7 +1257,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: workspaces.reserve(txns.size()); std::transform(txns.cbegin(), txns.cend(), std::back_inserter(workspaces), [](const auto& tx) { return Workspace(tx); }); - std::map<const uint256, const MempoolAcceptResult> results; + std::map<uint256, MempoolAcceptResult> results; LOCK(m_pool.cs); @@ -1332,6 +1334,54 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: return PackageMempoolAcceptResult(package_state, std::move(results)); } +PackageMempoolAcceptResult MemPoolAccept::AcceptSubPackage(const std::vector<CTransactionRef>& subpackage, ATMPArgs& args) +{ + AssertLockHeld(::cs_main); + AssertLockHeld(m_pool.cs); + auto result = [&]() EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_pool.cs) { + if (subpackage.size() > 1) { + return AcceptMultipleTransactions(subpackage, args); + } + const auto& tx = subpackage.front(); + ATMPArgs single_args = ATMPArgs::SingleInPackageAccept(args); + const auto single_res = AcceptSingleTransaction(tx, single_args); + PackageValidationState package_state_wrapped; + if (single_res.m_result_type != MempoolAcceptResult::ResultType::VALID) { + package_state_wrapped.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); + } + return PackageMempoolAcceptResult(package_state_wrapped, {{tx->GetWitnessHash(), single_res}}); + }(); + // Clean up m_view and m_viewmempool so that other subpackage evaluations don't have access to + // coins they shouldn't. Keep some coins in order to minimize re-fetching coins from the UTXO set. + // + // There are 3 kinds of coins in m_view: + // (1) Temporary coins from the transactions in subpackage, constructed by m_viewmempool. + // (2) Mempool coins from transactions in the mempool, constructed by m_viewmempool. + // (3) Confirmed coins fetched from our current UTXO set. + // + // (1) Temporary coins need to be removed, regardless of whether the transaction was submitted. + // If the transaction was submitted to the mempool, m_viewmempool will be able to fetch them from + // there. If it wasn't submitted to mempool, it is incorrect to keep them - future calls may try + // to spend those coins that don't actually exist. + // (2) Mempool coins also need to be removed. If the mempool contents have changed as a result + // of submitting or replacing transactions, coins previously fetched from mempool may now be + // spent or nonexistent. Those coins need to be deleted from m_view. + // (3) Confirmed coins don't need to be removed. The chainstate has not changed (we are + // holding cs_main and no blocks have been processed) so the confirmed tx cannot disappear like + // a mempool tx can. The coin may now be spent after we submitted a tx to mempool, but + // we have already checked that the package does not have 2 transactions spending the same coin. + // Keeping them in m_view is an optimization to not re-fetch confirmed coins if we later look up + // inputs for this transaction again. + for (const auto& outpoint : m_viewmempool.GetNonBaseCoins()) { + // In addition to resetting m_viewmempool, we also need to manually delete these coins from + // m_view because it caches copies of the coins it fetched from m_viewmempool previously. + m_view.Uncache(outpoint); + } + // This deletes the temporary and mempool coins. + m_viewmempool.Reset(); + return result; +} + PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, ATMPArgs& args) { AssertLockHeld(cs_main); @@ -1388,21 +1438,12 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, m_view.SetBackend(m_dummy); LOCK(m_pool.cs); - // Stores final results that won't change - std::map<const uint256, const MempoolAcceptResult> results_final; - // Node operators are free to set their mempool policies however they please, nodes may receive - // transactions in different orders, and malicious counterparties may try to take advantage of - // policy differences to pin or delay propagation of transactions. As such, it's possible for - // some package transaction(s) to already be in the mempool, and we don't want to reject the - // entire package in that case (as that could be a censorship vector). De-duplicate the - // transactions that are already in the mempool, and only call AcceptMultipleTransactions() with - // the new transactions. This ensures we don't double-count transaction counts and sizes when - // checking ancestor/descendant limits, or double-count transaction fees for fee-related policy. - ATMPArgs single_args = ATMPArgs::SingleInPackageAccept(args); - // Results from individual validation. "Nonfinal" because if a transaction fails by itself but - // succeeds later (i.e. when evaluated with a fee-bumping child), the result changes (though not - // reflected in this map). If a transaction fails more than once, we want to return the first - // result, when it was considered on its own. So changes will only be from invalid -> valid. + // Stores results from which we will create the returned PackageMempoolAcceptResult. + // A result may be changed if a mempool transaction is evicted later due to LimitMempoolSize(). + std::map<uint256, MempoolAcceptResult> results_final; + // Results from individual validation which will be returned if no other result is available for + // this transaction. "Nonfinal" because if a transaction fails by itself but succeeds later + // (i.e. when evaluated with a fee-bumping child), the result in this map may be discarded. std::map<uint256, MempoolAcceptResult> individual_results_nonfinal; bool quit_early{false}; std::vector<CTransactionRef> txns_package_eval; @@ -1414,6 +1455,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, // we know is that the inputs aren't available. if (m_pool.exists(GenTxid::Wtxid(wtxid))) { // Exact transaction already exists in the mempool. + // Node operators are free to set their mempool policies however they please, nodes may receive + // transactions in different orders, and malicious counterparties may try to take advantage of + // policy differences to pin or delay propagation of transactions. As such, it's possible for + // some package transaction(s) to already be in the mempool, and we don't want to reject the + // entire package in that case (as that could be a censorship vector). De-duplicate the + // transactions that are already in the mempool, and only call AcceptMultipleTransactions() with + // the new transactions. This ensures we don't double-count transaction counts and sizes when + // checking ancestor/descendant limits, or double-count transaction fees for fee-related policy. auto iter = m_pool.GetIter(txid); assert(iter != std::nullopt); results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee())); @@ -1432,7 +1481,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, } else { // Transaction does not already exist in the mempool. // Try submitting the transaction on its own. - const auto single_res = AcceptSingleTransaction(tx, single_args); + const auto single_package_res = AcceptSubPackage({tx}, args); + const auto& single_res = single_package_res.m_tx_results.at(wtxid); if (single_res.m_result_type == MempoolAcceptResult::ResultType::VALID) { // The transaction succeeded on its own and is now in the mempool. Don't include it // in package validation, because its fees should only be "used" once. @@ -1459,32 +1509,52 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, } } - // Quit early because package validation won't change the result or the entire package has - // already been submitted. - if (quit_early || txns_package_eval.empty()) { - for (const auto& [wtxid, mempoolaccept_res] : individual_results_nonfinal) { - Assume(results_final.emplace(wtxid, mempoolaccept_res).second); - Assume(mempoolaccept_res.m_result_type == MempoolAcceptResult::ResultType::INVALID); + auto multi_submission_result = quit_early || txns_package_eval.empty() ? PackageMempoolAcceptResult(package_state_quit_early, {}) : + AcceptSubPackage(txns_package_eval, args); + PackageValidationState& package_state_final = multi_submission_result.m_state; + + // Make sure we haven't exceeded max mempool size. + // Package transactions that were submitted to mempool or already in mempool may be evicted. + LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip()); + + for (const auto& tx : package) { + const auto& wtxid = tx->GetWitnessHash(); + if (multi_submission_result.m_tx_results.count(wtxid) > 0) { + // We shouldn't have re-submitted if the tx result was already in results_final. + Assume(results_final.count(wtxid) == 0); + // If it was submitted, check to see if the tx is still in the mempool. It could have + // been evicted due to LimitMempoolSize() above. + const auto& txresult = multi_submission_result.m_tx_results.at(wtxid); + if (txresult.m_result_type == MempoolAcceptResult::ResultType::VALID && !m_pool.exists(GenTxid::Wtxid(wtxid))) { + package_state_final.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); + TxValidationState mempool_full_state; + mempool_full_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full"); + results_final.emplace(wtxid, MempoolAcceptResult::Failure(mempool_full_state)); + } else { + results_final.emplace(wtxid, txresult); + } + } else if (const auto it{results_final.find(wtxid)}; it != results_final.end()) { + // Already-in-mempool transaction. Check to see if it's still there, as it could have + // been evicted when LimitMempoolSize() was called. + Assume(it->second.m_result_type != MempoolAcceptResult::ResultType::INVALID); + Assume(individual_results_nonfinal.count(wtxid) == 0); + // Query by txid to include the same-txid-different-witness ones. + if (!m_pool.exists(GenTxid::Txid(tx->GetHash()))) { + package_state_final.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); + TxValidationState mempool_full_state; + mempool_full_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "mempool full"); + // Replace the previous result. + results_final.erase(wtxid); + results_final.emplace(wtxid, MempoolAcceptResult::Failure(mempool_full_state)); + } + } else if (const auto it{individual_results_nonfinal.find(wtxid)}; it != individual_results_nonfinal.end()) { + Assume(it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID); + // Interesting result from previous processing. + results_final.emplace(wtxid, it->second); } - return PackageMempoolAcceptResult(package_state_quit_early, std::move(results_final)); - } - // Validate the (deduplicated) transactions as a package. Note that submission_result has its - // own PackageValidationState; package_state_quit_early is unused past this point. - auto submission_result = AcceptMultipleTransactions(txns_package_eval, args); - // Include already-in-mempool transaction results in the final result. - for (const auto& [wtxid, mempoolaccept_res] : results_final) { - Assume(submission_result.m_tx_results.emplace(wtxid, mempoolaccept_res).second); - Assume(mempoolaccept_res.m_result_type != MempoolAcceptResult::ResultType::INVALID); - } - if (submission_result.m_state.GetResult() == PackageValidationResult::PCKG_TX) { - // Package validation failed because one or more transactions failed. Provide a result for - // each transaction; if AcceptMultipleTransactions() didn't return a result for a tx, - // include the previous individual failure reason. - submission_result.m_tx_results.insert(individual_results_nonfinal.cbegin(), - individual_results_nonfinal.cend()); - Assume(submission_result.m_tx_results.size() == package.size()); } - return submission_result; + Assume(results_final.size() == package.size()); + return PackageMempoolAcceptResult(package_state_final, std::move(results_final)); } } // anon namespace @@ -4519,8 +4589,8 @@ void ChainstateManager::LoadExternalBlockFile( int nLoaded = 0; try { - // This takes over fileIn and calls fclose() on it in the CBufferedFile destructor - CBufferedFile blkdat(fileIn, 2*MAX_BLOCK_SERIALIZED_SIZE, MAX_BLOCK_SERIALIZED_SIZE+8, SER_DISK, CLIENT_VERSION); + // This takes over fileIn and calls fclose() on it in the BufferedFile destructor + BufferedFile blkdat{fileIn, 2 * MAX_BLOCK_SERIALIZED_SIZE, MAX_BLOCK_SERIALIZED_SIZE + 8, CLIENT_VERSION}; // nRewind indicates where to resume scanning in case something goes wrong, // such as a block fails to deserialize. uint64_t nRewind = blkdat.GetPos(); @@ -4533,11 +4603,11 @@ void ChainstateManager::LoadExternalBlockFile( unsigned int nSize = 0; try { // locate a header - unsigned char buf[CMessageHeader::MESSAGE_START_SIZE]; + MessageStartChars buf; blkdat.FindByte(std::byte(params.MessageStart()[0])); nRewind = blkdat.GetPos() + 1; blkdat >> buf; - if (memcmp(buf, params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) { + if (buf != params.MessageStart()) { continue; } // read size diff --git a/src/validation.h b/src/validation.h index 1ff9aaa7a3..f1ff6bb671 100644 --- a/src/validation.h +++ b/src/validation.h @@ -210,21 +210,21 @@ private: */ struct PackageMempoolAcceptResult { - const PackageValidationState m_state; + PackageValidationState m_state; /** * Map from wtxid to finished MempoolAcceptResults. The client is responsible * for keeping track of the transaction objects themselves. If a result is not * present, it means validation was unfinished for that transaction. If there * was a package-wide error (see result in m_state), m_tx_results will be empty. */ - std::map<const uint256, const MempoolAcceptResult> m_tx_results; + std::map<uint256, MempoolAcceptResult> m_tx_results; explicit PackageMempoolAcceptResult(PackageValidationState state, - std::map<const uint256, const MempoolAcceptResult>&& results) + std::map<uint256, MempoolAcceptResult>&& results) : m_state{state}, m_tx_results(std::move(results)) {} explicit PackageMempoolAcceptResult(PackageValidationState state, CFeeRate feerate, - std::map<const uint256, const MempoolAcceptResult>&& results) + std::map<uint256, MempoolAcceptResult>&& results) : m_state{state}, m_tx_results(std::move(results)) {} /** Constructor to create a PackageMempoolAcceptResult from a single MempoolAcceptResult */ diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 0c24920516..0ee39d2b5a 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -136,7 +136,7 @@ bool IsSQLiteFile(const fs::path& path) } // Check the application id matches our network magic - return memcmp(Params().MessageStart(), app_id, 4) == 0; + return memcmp(Params().MessageStart().data(), app_id, 4) == 0; } void ReadDatabaseArgs(const ArgsManager& args, DatabaseOptions& options) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 0c2be26ddf..c4206e9897 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1566,6 +1566,7 @@ RPCHelpMan walletprocesspsbt() { {RPCResult::Type::STR, "psbt", "The base64-encoded partially signed transaction"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, + {RPCResult::Type::STR_HEX, "hex", /*optional=*/true, "The hex-encoded network transaction if complete"}, } }, RPCExamples{ @@ -1609,6 +1610,14 @@ RPCHelpMan walletprocesspsbt() ssTx << psbtx; result.pushKV("psbt", EncodeBase64(ssTx.str())); result.pushKV("complete", complete); + if (complete) { + CMutableTransaction mtx; + // Returns true if complete, which we already think it is. + CHECK_NONFATAL(FinalizeAndExtractPSBT(psbtx, mtx)); + CDataStream ssTx_final(SER_NETWORK, PROTOCOL_VERSION); + ssTx_final << mtx; + result.pushKV("hex", HexStr(ssTx_final)); + } return result; }, diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index ecd34bb96a..db9989163d 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -193,7 +193,7 @@ bool SQLiteDatabase::Verify(bilingual_str& error) auto read_result = ReadPragmaInteger(m_db, "application_id", "the application id", error); if (!read_result.has_value()) return false; uint32_t app_id = static_cast<uint32_t>(read_result.value()); - uint32_t net_magic = ReadBE32(Params().MessageStart()); + uint32_t net_magic = ReadBE32(Params().MessageStart().data()); if (app_id != net_magic) { error = strprintf(_("SQLiteDatabase: Unexpected application id. Expected %u, got %u"), net_magic, app_id); return false; @@ -324,7 +324,7 @@ void SQLiteDatabase::Open() } // Set the application id - uint32_t app_id = ReadBE32(Params().MessageStart()); + uint32_t app_id = ReadBE32(Params().MessageStart().data()); SetPragma(m_db, "application_id", strprintf("%d", static_cast<int32_t>(app_id)), "Failed to set the application id"); |