diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/Makefile.test.include | 1 | ||||
| -rw-r--r-- | src/net.cpp | 73 | ||||
| -rw-r--r-- | src/net.h | 41 | ||||
| -rw-r--r-- | src/node/blockstorage.cpp | 40 | ||||
| -rw-r--r-- | src/node/blockstorage.h | 11 | ||||
| -rw-r--r-- | src/rpc/client.cpp | 4 | ||||
| -rw-r--r-- | src/test/fuzz/package_eval.cpp | 294 | ||||
| -rw-r--r-- | src/test/net_tests.cpp | 58 | ||||
| -rw-r--r-- | src/validation.cpp | 6 | ||||
| -rw-r--r-- | src/wallet/feebumper.cpp | 14 | ||||
| -rw-r--r-- | src/wallet/feebumper.h | 4 | ||||
| -rw-r--r-- | src/wallet/rpc/spend.cpp | 18 |
12 files changed, 433 insertions, 131 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 5dc20d4fab..d66f5bf53a 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -300,6 +300,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/netbase_dns_lookup.cpp \ test/fuzz/node_eviction.cpp \ test/fuzz/p2p_transport_serialization.cpp \ + test/fuzz/package_eval.cpp \ test/fuzz/parse_hd_keypath.cpp \ test/fuzz/parse_numbers.cpp \ test/fuzz/parse_script.cpp \ diff --git a/src/net.cpp b/src/net.cpp index 0c6dac572d..c540b2f7ae 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1002,8 +1002,7 @@ void V2Transport::StartSendingHandshake() noexcept m_send_buffer.resize(EllSwiftPubKey::size() + m_send_garbage.size()); std::copy(std::begin(m_cipher.GetOurPubKey()), std::end(m_cipher.GetOurPubKey()), MakeWritableByteSpan(m_send_buffer).begin()); std::copy(m_send_garbage.begin(), m_send_garbage.end(), m_send_buffer.begin() + EllSwiftPubKey::size()); - // We cannot wipe m_send_garbage as it will still be used to construct the garbage - // authentication packet. + // We cannot wipe m_send_garbage as it will still be used as AAD later in the handshake. } V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span<const std::byte> ent32, std::vector<uint8_t> garbage) noexcept : @@ -1037,9 +1036,6 @@ void V2Transport::SetReceiveState(RecvState recv_state) noexcept Assume(recv_state == RecvState::GARB_GARBTERM); break; case RecvState::GARB_GARBTERM: - Assume(recv_state == RecvState::GARBAUTH); - break; - case RecvState::GARBAUTH: Assume(recv_state == RecvState::VERSION); break; case RecvState::VERSION: @@ -1171,24 +1167,15 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept m_cipher.GetSendGarbageTerminator().end(), MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::GARBAGE_TERMINATOR_LEN).begin()); - // Construct garbage authentication packet in the send buffer (using the garbage data which - // is still there). - m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION); - m_cipher.Encrypt( - /*contents=*/{}, - /*aad=*/MakeByteSpan(m_send_garbage), - /*ignore=*/false, - /*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION)); - // We no longer need the garbage. - ClearShrink(m_send_garbage); - - // Construct version packet in the send buffer. + // Construct version packet in the send buffer, with the sent garbage data as AAD. m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION + VERSION_CONTENTS.size()); m_cipher.Encrypt( /*contents=*/VERSION_CONTENTS, - /*aad=*/{}, + /*aad=*/MakeByteSpan(m_send_garbage), /*ignore=*/false, /*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION + VERSION_CONTENTS.size())); + // We no longer need the garbage. + ClearShrink(m_send_garbage); } else { // We still have to receive more key bytes. } @@ -1202,11 +1189,11 @@ bool V2Transport::ProcessReceivedGarbageBytes() noexcept Assume(m_recv_buffer.size() <= MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN); if (m_recv_buffer.size() >= BIP324Cipher::GARBAGE_TERMINATOR_LEN) { if (MakeByteSpan(m_recv_buffer).last(BIP324Cipher::GARBAGE_TERMINATOR_LEN) == m_cipher.GetReceiveGarbageTerminator()) { - // Garbage terminator received. Switch to receiving garbage authentication packet. - m_recv_garbage = std::move(m_recv_buffer); - m_recv_garbage.resize(m_recv_garbage.size() - BIP324Cipher::GARBAGE_TERMINATOR_LEN); + // Garbage terminator received. Store garbage to authenticate it as AAD later. + m_recv_aad = std::move(m_recv_buffer); + m_recv_aad.resize(m_recv_aad.size() - BIP324Cipher::GARBAGE_TERMINATOR_LEN); m_recv_buffer.clear(); - SetReceiveState(RecvState::GARBAUTH); + SetReceiveState(RecvState::VERSION); } else if (m_recv_buffer.size() == MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN) { // We've reached the maximum length for garbage + garbage terminator, and the // terminator still does not match. Abort. @@ -1225,8 +1212,7 @@ bool V2Transport::ProcessReceivedGarbageBytes() noexcept bool V2Transport::ProcessReceivedPacketBytes() noexcept { AssertLockHeld(m_recv_mutex); - Assume(m_recv_state == RecvState::GARBAUTH || m_recv_state == RecvState::VERSION || - m_recv_state == RecvState::APP); + Assume(m_recv_state == RecvState::VERSION || m_recv_state == RecvState::APP); // The maximum permitted contents length for a packet, consisting of: // - 0x00 byte: indicating long message type encoding @@ -1249,45 +1235,37 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept // as GetMaxBytesToProcess only allows up to LENGTH_LEN into the buffer before that point. m_recv_decode_buffer.resize(m_recv_len); bool ignore{false}; - Span<const std::byte> aad; - if (m_recv_state == RecvState::GARBAUTH) aad = MakeByteSpan(m_recv_garbage); bool ret = m_cipher.Decrypt( /*input=*/MakeByteSpan(m_recv_buffer).subspan(BIP324Cipher::LENGTH_LEN), - /*aad=*/aad, + /*aad=*/MakeByteSpan(m_recv_aad), /*ignore=*/ignore, /*contents=*/MakeWritableByteSpan(m_recv_decode_buffer)); if (!ret) { LogPrint(BCLog::NET, "V2 transport error: packet decryption failure (%u bytes), peer=%d\n", m_recv_len, m_nodeid); return false; } + // We have decrypted a valid packet with the AAD we expected, so clear the expected AAD. + ClearShrink(m_recv_aad); // Feed the last 4 bytes of the Poly1305 authentication tag (and its timing) into our RNG. RandAddEvent(ReadLE32(m_recv_buffer.data() + m_recv_buffer.size() - 4)); - // At this point we have a valid packet decrypted into m_recv_decode_buffer. Depending on - // the current state, decide what to do with it. - switch (m_recv_state) { - case RecvState::GARBAUTH: - // Ignore flag does not matter for garbage authentication. Any valid packet functions - // as authentication. Receive and process the version packet next. - SetReceiveState(RecvState::VERSION); - ClearShrink(m_recv_garbage); - break; - case RecvState::VERSION: - if (!ignore) { + // At this point we have a valid packet decrypted into m_recv_decode_buffer. If it's not a + // decoy, which we simply ignore, use the current state to decide what to do with it. + if (!ignore) { + switch (m_recv_state) { + case RecvState::VERSION: // Version message received; transition to application phase. The contents is // ignored, but can be used for future extensions. SetReceiveState(RecvState::APP); - } - break; - case RecvState::APP: - if (!ignore) { + break; + case RecvState::APP: // Application message decrypted correctly. It can be extracted using GetMessage(). SetReceiveState(RecvState::APP_READY); + break; + default: + // Any other state is invalid (this function should not have been called). + Assume(false); } - break; - default: - // Any other state is invalid (this function should not have been called). - Assume(false); } // Wipe the receive buffer where the next packet will be received into. ClearShrink(m_recv_buffer); @@ -1323,7 +1301,6 @@ size_t V2Transport::GetMaxBytesToProcess() noexcept case RecvState::GARB_GARBTERM: // Process garbage bytes one by one (because terminator may appear anywhere). return 1; - case RecvState::GARBAUTH: case RecvState::VERSION: case RecvState::APP: // These three states all involve decoding a packet. Process the length descriptor first, @@ -1377,7 +1354,6 @@ bool V2Transport::ReceivedBytes(Span<const uint8_t>& msg_bytes) noexcept // bytes). m_recv_buffer.reserve(MAX_GARBAGE_LEN + BIP324Cipher::GARBAGE_TERMINATOR_LEN); break; - case RecvState::GARBAUTH: case RecvState::VERSION: case RecvState::APP: { // During states where a packet is being received, as much as is expected but never @@ -1421,7 +1397,6 @@ bool V2Transport::ReceivedBytes(Span<const uint8_t>& msg_bytes) noexcept if (!ProcessReceivedGarbageBytes()) return false; break; - case RecvState::GARBAUTH: case RecvState::VERSION: case RecvState::APP: if (!ProcessReceivedPacketBytes()) return false; @@ -460,10 +460,10 @@ private: * * start(responder) * | - * | start(initiator) /---------\ - * | | | | - * v v v | - * KEY_MAYBE_V1 -> KEY -> GARB_GARBTERM -> GARBAUTH -> VERSION -> APP -> APP_READY + * | start(initiator) /---------\ + * | | | | + * v v v | + * KEY_MAYBE_V1 -> KEY -> GARB_GARBTERM -> VERSION -> APP -> APP_READY * | * \-------> V1 */ @@ -485,24 +485,19 @@ private: /** Garbage and garbage terminator. * * Whenever a byte is received, the last 16 bytes are compared with the expected garbage - * terminator. When that happens, the state becomes GARBAUTH. If no matching terminator is + * terminator. When that happens, the state becomes VERSION. If no matching terminator is * received in 4111 bytes (4095 for the maximum garbage length, and 16 bytes for the * terminator), the connection aborts. */ GARB_GARBTERM, - /** Garbage authentication packet. - * - * A packet is received, and decrypted/verified with AAD set to the garbage received during - * the GARB_GARBTERM state. If that succeeds, the state becomes VERSION. If it fails the - * connection aborts. */ - GARBAUTH, - /** Version packet. * - * A packet is received, and decrypted/verified. If that succeeds, the state becomes APP, - * and the decrypted contents is interpreted as version negotiation (currently, that means - * ignoring it, but it can be used for negotiating future extensions). If it fails, the - * connection aborts. */ + * A packet is received, and decrypted/verified. If that fails, the connection aborts. The + * first received packet in this state (whether it's a decoy or not) is expected to + * authenticate the garbage received during the GARB_GARBTERM state as associated + * authenticated data (AAD). The first non-decoy packet in this state is interpreted as + * version negotiation (currently, that means ignoring the contents, but it can be used for + * negotiating future extensions), and afterwards the state becomes APP. */ VERSION, /** Application packet. @@ -556,9 +551,9 @@ private: /** Normal sending state. * * In this state, the ciphers are initialized, so packets can be sent. When this state is - * entered, the garbage terminator, garbage authentication packet, and version - * packet are appended to the send buffer (in addition to the key and garbage which may - * still be there). In this state a message can be provided if the send buffer is empty. */ + * entered, the garbage terminator and version packet are appended to the send buffer (in + * addition to the key and garbage which may still be there). In this state a message can be + * provided if the send buffer is empty. */ READY, /** This transport is using v1 fallback. @@ -578,13 +573,13 @@ private: /** Lock for receiver-side fields. */ mutable Mutex m_recv_mutex ACQUIRED_BEFORE(m_send_mutex); - /** In {GARBAUTH, VERSION, APP}, the decrypted packet length, if m_recv_buffer.size() >= + /** In {VERSION, APP}, the decrypted packet length, if m_recv_buffer.size() >= * BIP324Cipher::LENGTH_LEN. Unspecified otherwise. */ uint32_t m_recv_len GUARDED_BY(m_recv_mutex) {0}; /** Receive buffer; meaning is determined by m_recv_state. */ std::vector<uint8_t> m_recv_buffer GUARDED_BY(m_recv_mutex); - /** During GARBAUTH, the garbage received during GARB_GARBTERM. */ - std::vector<uint8_t> m_recv_garbage GUARDED_BY(m_recv_mutex); + /** AAD expected in next received packet (currently used only for garbage). */ + std::vector<uint8_t> m_recv_aad GUARDED_BY(m_recv_mutex); /** Buffer to put decrypted contents in, for converting to CNetMessage. */ std::vector<uint8_t> m_recv_decode_buffer GUARDED_BY(m_recv_mutex); /** Deserialization type. */ @@ -624,7 +619,7 @@ private: bool ProcessReceivedKeyBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex, !m_send_mutex); /** Process bytes in m_recv_buffer, while in GARB_GARBTERM state. */ bool ProcessReceivedGarbageBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex); - /** Process bytes in m_recv_buffer, while in GARBAUTH/VERSION/APP state. */ + /** Process bytes in m_recv_buffer, while in VERSION/APP state. */ bool ProcessReceivedPacketBytes() noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex); public: diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index ae63d12ef7..5c3b7f958e 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -651,16 +651,19 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in return true; } -void BlockManager::FlushUndoFile(int block_file, bool finalize) +bool BlockManager::FlushUndoFile(int block_file, bool finalize) { FlatFilePos undo_pos_old(block_file, m_blockfile_info[block_file].nUndoSize); if (!UndoFileSeq().Flush(undo_pos_old, finalize)) { m_opts.notifications.flushError("Flushing undo file to disk failed. This is likely the result of an I/O error."); + return false; } + return true; } -void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo) +bool BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo) { + bool success = true; LOCK(cs_LastBlockFile); if (m_blockfile_info.size() < 1) { @@ -668,17 +671,23 @@ void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo) // chainstate init, when we call ChainstateManager::MaybeRebalanceCaches() (which // then calls FlushStateToDisk()), resulting in a call to this function before we // have populated `m_blockfile_info` via LoadBlockIndexDB(). - return; + return true; } assert(static_cast<int>(m_blockfile_info.size()) > m_last_blockfile); FlatFilePos block_pos_old(m_last_blockfile, m_blockfile_info[m_last_blockfile].nSize); if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) { m_opts.notifications.flushError("Flushing block file to disk failed. This is likely the result of an I/O error."); + success = false; } // we do not always flush the undo file, as the chain tip may be lagging behind the incoming blocks, // e.g. during IBD or a sync after a node going offline - if (!fFinalize || finalize_undo) FlushUndoFile(m_last_blockfile, finalize_undo); + if (!fFinalize || finalize_undo) { + if (!FlushUndoFile(m_last_blockfile, finalize_undo)) { + success = false; + } + } + return success; } uint64_t BlockManager::CalculateCurrentUsage() @@ -771,7 +780,19 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne if (!fKnown) { LogPrint(BCLog::BLOCKSTORAGE, "Leaving block file %i: %s\n", m_last_blockfile, m_blockfile_info[m_last_blockfile].ToString()); } - FlushBlockFile(!fKnown, finalize_undo); + + // Do not propagate the return code. The flush concerns a previous block + // and undo file that has already been written to. If a flush fails + // here, and we crash, there is no expected additional block data + // inconsistency arising from the flush failure here. However, the undo + // data may be inconsistent after a crash if the flush is called during + // a reindex. A flush error might also leave some of the data files + // untrimmed. + if (!FlushBlockFile(!fKnown, finalize_undo)) { + LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, + "Failed to flush previous block file %05i (finalize=%i, finalize_undo=%i) before opening new block file %05i\n", + m_last_blockfile, !fKnown, finalize_undo, nFile); + } m_last_blockfile = nFile; m_undo_height_in_last_blockfile = 0; // No undo data yet in the new file, so reset our undo-height tracking. } @@ -862,7 +883,14 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid // with the block writes (usually when a synced up node is getting newly mined blocks) -- this case is caught in // the FindBlockPos function if (_pos.nFile < m_last_blockfile && static_cast<uint32_t>(block.nHeight) == m_blockfile_info[_pos.nFile].nHeightLast) { - FlushUndoFile(_pos.nFile, true); + // Do not propagate the return code, a failed flush here should not + // be an indication for a failed write. If it were propagated here, + // the caller would assume the undo data not to be written, when in + // fact it is. Note though, that a failed flush might leave the data + // file untrimmed. + if (!FlushUndoFile(_pos.nFile, true)) { + LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, "Failed to flush undo file %05i\n", _pos.nFile); + } } else if (_pos.nFile == m_last_blockfile && static_cast<uint32_t>(block.nHeight) > m_undo_height_in_last_blockfile) { m_undo_height_in_last_blockfile = block.nHeight; } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index a89fa7f76e..9a1d44cc75 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -119,9 +119,14 @@ private: */ bool LoadBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void FlushBlockFile(bool fFinalize = false, bool finalize_undo = false); - void FlushUndoFile(int block_file, bool finalize = false); - bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, uint64_t nTime, bool fKnown); + + /** Return false if block file or undo file flushing fails. */ + [[nodiscard]] bool FlushBlockFile(bool fFinalize = false, bool finalize_undo = false); + + /** Return false if undo file flushing fails. */ + [[nodiscard]] bool FlushUndoFile(int block_file, bool finalize = false); + + [[nodiscard]] bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, uint64_t nTime, bool fKnown); bool FindUndoPos(BlockValidationState& state, int nFile, FlatFilePos& pos, unsigned int nAddSize); FlatFileSeq BlockFileSeq() const; diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 0ee3f27761..1e5e231cef 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -263,13 +263,13 @@ static const CRPCConvertParam vRPCConvertParams[] = { "bumpfee", 1, "fee_rate"}, { "bumpfee", 1, "replaceable"}, { "bumpfee", 1, "outputs"}, - { "bumpfee", 1, "reduce_output"}, + { "bumpfee", 1, "original_change_index"}, { "psbtbumpfee", 1, "options" }, { "psbtbumpfee", 1, "conf_target"}, { "psbtbumpfee", 1, "fee_rate"}, { "psbtbumpfee", 1, "replaceable"}, { "psbtbumpfee", 1, "outputs"}, - { "psbtbumpfee", 1, "reduce_output"}, + { "psbtbumpfee", 1, "original_change_index"}, { "logging", 0, "include" }, { "logging", 1, "exclude" }, { "disconnectnode", 1, "nodeid" }, diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp new file mode 100644 index 0000000000..4c81c0b679 --- /dev/null +++ b/src/test/fuzz/package_eval.cpp @@ -0,0 +1,294 @@ +// 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. + +#include <consensus/validation.h> +#include <node/context.h> +#include <node/mempool_args.h> +#include <node/miner.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/fuzz/util/mempool.h> +#include <test/util/mining.h> +#include <test/util/script.h> +#include <test/util/setup_common.h> +#include <test/util/txmempool.h> +#include <util/rbf.h> +#include <validation.h> +#include <validationinterface.h> + +using node::NodeContext; + +namespace { + +const TestingSetup* g_setup; +std::vector<COutPoint> g_outpoints_coinbase_init_mature; + +struct MockedTxPool : public CTxMemPool { + void RollingFeeUpdate() EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + LOCK(cs); + lastRollingFeeUpdate = GetTime(); + blockSinceLastRollingFeeBump = true; + } +}; + +void initialize_tx_pool() +{ + static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(); + g_setup = testing_setup.get(); + + for (int i = 0; i < 2 * COINBASE_MATURITY; ++i) { + COutPoint prevout{MineBlock(g_setup->m_node, P2WSH_OP_TRUE)}; + if (i < COINBASE_MATURITY) { + // Remember the txids to avoid expensive disk access later on + g_outpoints_coinbase_init_mature.push_back(prevout); + } + } + SyncWithValidationInterfaceQueue(); +} + +struct OutpointsUpdater final : public CValidationInterface { + std::set<COutPoint>& m_mempool_outpoints; + + explicit OutpointsUpdater(std::set<COutPoint>& r) + : m_mempool_outpoints{r} {} + + void TransactionAddedToMempool(const CTransactionRef& tx, uint64_t /* mempool_sequence */) override + { + // for coins spent we always want to be able to rbf so they're not removed + + // outputs from this tx can now be spent + for (uint32_t index{0}; index < tx->vout.size(); ++index) { + m_mempool_outpoints.insert(COutPoint{tx->GetHash(), index}); + } + } + + void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t /* mempool_sequence */) override + { + // outpoints spent by this tx are now available + for (const auto& input : tx->vin) { + // Could already exist if this was a replacement + m_mempool_outpoints.insert(input.prevout); + } + // outpoints created by this tx no longer exist + for (uint32_t index{0}; index < tx->vout.size(); ++index) { + m_mempool_outpoints.erase(COutPoint{tx->GetHash(), index}); + } + } +}; + +struct TransactionsDelta final : public CValidationInterface { + std::set<CTransactionRef>& m_added; + + explicit TransactionsDelta(std::set<CTransactionRef>& a) + : m_added{a} {} + + void TransactionAddedToMempool(const CTransactionRef& tx, uint64_t /* mempool_sequence */) override + { + // Transactions may be entered and booted any number of times + m_added.insert(tx); + } + + void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t /* mempool_sequence */) override + { + // Transactions may be entered and booted any number of times + m_added.erase(tx); + } +}; + +void MockTime(FuzzedDataProvider& fuzzed_data_provider, const Chainstate& chainstate) +{ + const auto time = ConsumeTime(fuzzed_data_provider, + chainstate.m_chain.Tip()->GetMedianTimePast() + 1, + std::numeric_limits<decltype(chainstate.m_chain.Tip()->nTime)>::max()); + SetMockTime(time); +} + +CTxMemPool MakeMempool(FuzzedDataProvider& fuzzed_data_provider, const NodeContext& node) +{ + // Take the default options for tests... + CTxMemPool::Options mempool_opts{MemPoolOptionsForTest(node)}; + + + // ...override specific options for this specific fuzz suite + mempool_opts.limits.ancestor_count = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 50); + mempool_opts.limits.ancestor_size_vbytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 202) * 1'000; + mempool_opts.limits.descendant_count = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 50); + mempool_opts.limits.descendant_size_vbytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 202) * 1'000; + mempool_opts.max_size_bytes = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 200) * 1'000'000; + mempool_opts.expiry = std::chrono::hours{fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(0, 999)}; + nBytesPerSigOp = fuzzed_data_provider.ConsumeIntegralInRange<unsigned>(1, 999); + + mempool_opts.estimator = nullptr; + mempool_opts.check_ratio = 1; + mempool_opts.require_standard = fuzzed_data_provider.ConsumeBool(); + + // ...and construct a CTxMemPool from it + return CTxMemPool{mempool_opts}; +} + +FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const auto& node = g_setup->m_node; + auto& chainstate{static_cast<DummyChainState&>(node.chainman->ActiveChainstate())}; + + MockTime(fuzzed_data_provider, chainstate); + + // All RBF-spendable outpoints outside of the unsubmitted package + std::set<COutPoint> mempool_outpoints; + std::map<COutPoint, CAmount> outpoints_value; + for (const auto& outpoint : g_outpoints_coinbase_init_mature) { + Assert(mempool_outpoints.insert(outpoint).second); + outpoints_value[outpoint] = 50 * COIN; + } + + auto outpoints_updater = std::make_shared<OutpointsUpdater>(mempool_outpoints); + RegisterSharedValidationInterface(outpoints_updater); + + CTxMemPool tx_pool_{MakeMempool(fuzzed_data_provider, node)}; + MockedTxPool& tx_pool = *static_cast<MockedTxPool*>(&tx_pool_); + + chainstate.SetMempool(&tx_pool); + + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) + { + Assert(!mempool_outpoints.empty()); + + std::vector<CTransactionRef> txs; + + // Make packages of 1-to-26 transactions + const auto num_txs = (size_t) fuzzed_data_provider.ConsumeIntegralInRange<int>(1, 26); + std::set<COutPoint> package_outpoints; + while (txs.size() < num_txs) { + + // Last transaction in a package needs to be a child of parents to get further in validation + // so the last transaction to be generated(in a >1 package) must spend all package-made outputs + // Note that this test currently only spends package outputs in last transaction. + bool last_tx = num_txs > 1 && txs.size() == num_txs - 1; + + // Create transaction to add to the mempool + const CTransactionRef tx = [&] { + CMutableTransaction tx_mut; + tx_mut.nVersion = CTransaction::CURRENT_VERSION; + tx_mut.nLockTime = fuzzed_data_provider.ConsumeBool() ? 0 : fuzzed_data_provider.ConsumeIntegral<uint32_t>(); + // Last tx will sweep all outpoints in package + const auto num_in = last_tx ? package_outpoints.size() : fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size()); + const auto num_out = fuzzed_data_provider.ConsumeIntegralInRange<int>(1, mempool_outpoints.size() * 2); + + auto& outpoints = last_tx ? package_outpoints : mempool_outpoints; + + Assert(!outpoints.empty()); + + CAmount amount_in{0}; + for (size_t i = 0; i < num_in; ++i) { + // Pop random outpoint + auto pop = outpoints.begin(); + std::advance(pop, fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, outpoints.size() - 1)); + const auto outpoint = *pop; + outpoints.erase(pop); + // no need to update or erase from outpoints_value + amount_in += outpoints_value.at(outpoint); + + // Create input + const auto sequence = ConsumeSequence(fuzzed_data_provider); + const auto script_sig = CScript{}; + const auto script_wit_stack = std::vector<std::vector<uint8_t>>{WITNESS_STACK_ELEM_OP_TRUE}; + CTxIn in; + in.prevout = outpoint; + in.nSequence = sequence; + in.scriptSig = script_sig; + in.scriptWitness.stack = script_wit_stack; + + tx_mut.vin.push_back(in); + } + const auto amount_fee = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(0, amount_in); + const auto amount_out = (amount_in - amount_fee) / num_out; + for (int i = 0; i < num_out; ++i) { + tx_mut.vout.emplace_back(amount_out, P2WSH_OP_TRUE); + } + // TODO vary transaction sizes to catch size-related issues + auto tx = MakeTransactionRef(tx_mut); + // Restore previously removed outpoints, except in-package outpoints + if (!last_tx) { + for (const auto& in : tx->vin) { + Assert(outpoints.insert(in.prevout).second); + } + // Cache the in-package outpoints being made + for (size_t i = 0; i < tx->vout.size(); ++i) { + package_outpoints.emplace(tx->GetHash(), i); + } + } + // We need newly-created values for the duration of this run + for (size_t i = 0; i < tx->vout.size(); ++i) { + outpoints_value[COutPoint(tx->GetHash(), i)] = tx->vout[i].nValue; + } + return tx; + }(); + txs.push_back(tx); + } + + if (fuzzed_data_provider.ConsumeBool()) { + MockTime(fuzzed_data_provider, chainstate); + } + if (fuzzed_data_provider.ConsumeBool()) { + tx_pool.RollingFeeUpdate(); + } + if (fuzzed_data_provider.ConsumeBool()) { + const auto& txid = fuzzed_data_provider.ConsumeBool() ? + txs.back()->GetHash() : + PickValue(fuzzed_data_provider, mempool_outpoints).hash; + const auto delta = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-50 * COIN, +50 * COIN); + tx_pool.PrioritiseTransaction(txid, delta); + } + + // Remember all added transactions + std::set<CTransactionRef> added; + auto txr = std::make_shared<TransactionsDelta>(added); + RegisterSharedValidationInterface(txr); + const bool bypass_limits = fuzzed_data_provider.ConsumeBool(); + + // When there are multiple transactions in the package, we call ProcessNewPackage(txs, test_accept=false) + // and AcceptToMemoryPool(txs.back(), test_accept=true). When there is only 1 transaction, we might flip it + // (the package is a test accept and ATMP is a submission). + auto single_submit = txs.size() == 1 && fuzzed_data_provider.ConsumeBool(); + + const auto result_package = WITH_LOCK(::cs_main, + return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit)); + // If something went wrong due to a package-specific policy, it might not return a + // validation result for the transaction. + if (result_package.m_state.GetResult() != PackageValidationResult::PCKG_POLICY) { + auto it = result_package.m_tx_results.find(txs.back()->GetWitnessHash()); + Assert(it != result_package.m_tx_results.end()); + Assert(it->second.m_result_type == MempoolAcceptResult::ResultType::VALID || + it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID || + it->second.m_result_type == MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); + } + + const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, txs.back(), GetTime(), bypass_limits, /*test_accept=*/!single_submit)); + const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID; + + SyncWithValidationInterfaceQueue(); + UnregisterSharedValidationInterface(txr); + + // There is only 1 transaction in the package. We did a test-package-accept and a ATMP + if (single_submit) { + Assert(accepted != added.empty()); + Assert(accepted == res.m_state.IsValid()); + if (accepted) { + Assert(added.size() == 1); + Assert(txs.back() == *added.begin()); + } + } else { + // This is empty if it fails early checks, or "full" if transactions are looked at deeper + Assert(result_package.m_tx_results.size() == txs.size() || result_package.m_tx_results.empty()); + } + } + + UnregisterSharedValidationInterface(outpoints_updater); + + WITH_LOCK(::cs_main, tx_pool.check(chainstate.CoinsTip(), chainstate.m_chain.Height() + 1)); +} +} // namespace diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 1e9c9d923b..1df08127ad 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1031,9 +1031,11 @@ class V2TransportTester bool m_test_initiator; //!< Whether m_transport is the initiator (true) or responder (false) std::vector<uint8_t> m_sent_garbage; //!< The garbage we've sent to m_transport. + std::vector<uint8_t> m_recv_garbage; //!< The garbage we've received from m_transport. std::vector<uint8_t> m_to_send; //!< Bytes we have queued up to send to m_transport. std::vector<uint8_t> m_received; //!< Bytes we have received from m_transport. std::deque<CSerializedNetMsg> m_msg_to_send; //!< Messages to be sent *by* m_transport to us. + bool m_sent_aad{false}; public: /** Construct a tester object. test_initiator: whether the tested transport is initiator. */ @@ -1131,8 +1133,7 @@ public: /** Schedule specified garbage to be sent to the transport. */ void SendGarbage(Span<const uint8_t> garbage) { - // Remember the specified garbage (so we can use it for constructing the garbage - // authentication packet). + // Remember the specified garbage (so we can use it as AAD). m_sent_garbage.assign(garbage.begin(), garbage.end()); // Schedule it for sending. Send(m_sent_garbage); @@ -1191,27 +1192,27 @@ public: Send(ciphertext); } - /** Schedule garbage terminator and authentication packet to be sent to the transport (only - * after ReceiveKey). */ - void SendGarbageTermAuth(size_t garb_auth_data_len = 0, bool garb_auth_ignore = false) + /** Schedule garbage terminator to be sent to the transport (only after ReceiveKey). */ + void SendGarbageTerm() { - // Generate random data to include in the garbage authentication packet (ignored by peer). - auto garb_auth_data = g_insecure_rand_ctx.randbytes<uint8_t>(garb_auth_data_len); // Schedule the garbage terminator to be sent. Send(m_cipher.GetSendGarbageTerminator()); - // Schedule the garbage authentication packet to be sent. - SendPacket(/*content=*/garb_auth_data, /*aad=*/m_sent_garbage, /*ignore=*/garb_auth_ignore); } /** Schedule version packet to be sent to the transport (only after ReceiveKey). */ void SendVersion(Span<const uint8_t> version_data = {}, bool vers_ignore = false) { - SendPacket(/*content=*/version_data, /*aad=*/{}, /*ignore=*/vers_ignore); + Span<const std::uint8_t> aad; + // Set AAD to garbage only for first packet. + if (!m_sent_aad) aad = m_sent_garbage; + SendPacket(/*content=*/version_data, /*aad=*/aad, /*ignore=*/vers_ignore); + m_sent_aad = true; } /** Expect a packet to have been received from transport, process it, and return its contents - * (only after ReceiveKey). By default, decoys are skipped. */ - std::vector<uint8_t> ReceivePacket(Span<const std::byte> aad = {}, bool skip_decoy = true) + * (only after ReceiveKey). Decoys are skipped. Optional associated authenticated data (AAD) is + * expected in the first received packet, no matter if that is a decoy or not. */ + std::vector<uint8_t> ReceivePacket(Span<const std::byte> aad = {}) { std::vector<uint8_t> contents; // Loop as long as there are ignored packets that are to be skipped. @@ -1232,16 +1233,18 @@ public: /*ignore=*/ignore, /*contents=*/MakeWritableByteSpan(contents)); BOOST_CHECK(ret); + // Don't expect AAD in further packets. + aad = {}; // Strip the processed packet's bytes off the front of the receive buffer. m_received.erase(m_received.begin(), m_received.begin() + size + BIP324Cipher::EXPANSION); - // Stop if the ignore bit is not set on this packet, or if we choose to not honor it. - if (!ignore || !skip_decoy) break; + // Stop if the ignore bit is not set on this packet. + if (!ignore) break; } return contents; } - /** Expect garbage, garbage terminator, and garbage auth packet to have been received, and - * process them (only after ReceiveKey). */ + /** Expect garbage and garbage terminator to have been received, and process them (only after + * ReceiveKey). */ void ReceiveGarbage() { // Figure out the garbage length. @@ -1252,18 +1255,15 @@ public: if (term_span == m_cipher.GetReceiveGarbageTerminator()) break; } // Copy the garbage to a buffer. - std::vector<uint8_t> garbage(m_received.begin(), m_received.begin() + garblen); + m_recv_garbage.assign(m_received.begin(), m_received.begin() + garblen); // Strip garbage + garbage terminator off the front of the receive buffer. m_received.erase(m_received.begin(), m_received.begin() + garblen + BIP324Cipher::GARBAGE_TERMINATOR_LEN); - // Process the expected garbage authentication packet. Such a packet still functions as one - // even when its ignore bit is set to true, so we do not skip decoy packets here. - ReceivePacket(/*aad=*/MakeByteSpan(garbage), /*skip_decoy=*/false); } /** Expect version packet to have been received, and process it (only after ReceiveKey). */ void ReceiveVersion() { - auto contents = ReceivePacket(); + auto contents = ReceivePacket(/*aad=*/MakeByteSpan(m_recv_garbage)); // Version packets from real BIP324 peers are expected to be empty, despite the fact that // this class supports *sending* non-empty version packets (to test that BIP324 peers // correctly ignore version packet contents). @@ -1340,7 +1340,7 @@ BOOST_AUTO_TEST_CASE(v2transport_test) tester.SendKey(); tester.SendGarbage(); tester.ReceiveKey(); - tester.SendGarbageTermAuth(); + tester.SendGarbageTerm(); tester.SendVersion(); ret = tester.Interact(); BOOST_REQUIRE(ret && ret->empty()); @@ -1380,7 +1380,7 @@ BOOST_AUTO_TEST_CASE(v2transport_test) auto ret = tester.Interact(); BOOST_REQUIRE(ret && ret->empty()); tester.ReceiveKey(); - tester.SendGarbageTermAuth(); + tester.SendGarbageTerm(); tester.SendVersion(); ret = tester.Interact(); BOOST_REQUIRE(ret && ret->empty()); @@ -1408,10 +1408,6 @@ BOOST_AUTO_TEST_CASE(v2transport_test) bool initiator = InsecureRandBool(); /** Use either 0 bytes or the maximum possible (4095 bytes) garbage length. */ size_t garb_len = InsecureRandBool() ? 0 : V2Transport::MAX_GARBAGE_LEN; - /** Sometimes, use non-empty contents in the garbage authentication packet (which is to be ignored). */ - size_t garb_auth_data_len = InsecureRandBool() ? 0 : InsecureRandRange(100000); - /** Whether to set the ignore bit on the garbage authentication packet (it still functions as garbage authentication). */ - bool garb_ignore = InsecureRandBool(); /** How many decoy packets to send before the version packet. */ unsigned num_ignore_version = InsecureRandRange(10); /** What data to send in the version packet (ignored by BIP324 peers, but reserved for future extensions). */ @@ -1432,7 +1428,7 @@ BOOST_AUTO_TEST_CASE(v2transport_test) tester.SendGarbage(garb_len); } tester.ReceiveKey(); - tester.SendGarbageTermAuth(garb_auth_data_len, garb_ignore); + tester.SendGarbageTerm(); for (unsigned v = 0; v < num_ignore_version; ++v) { size_t ver_ign_data_len = InsecureRandBool() ? 0 : InsecureRandRange(1000); auto ver_ign_data = g_insecure_rand_ctx.randbytes<uint8_t>(ver_ign_data_len); @@ -1476,7 +1472,7 @@ BOOST_AUTO_TEST_CASE(v2transport_test) tester.SendKey(); tester.SendGarbage(V2Transport::MAX_GARBAGE_LEN + 1); tester.ReceiveKey(); - tester.SendGarbageTermAuth(); + tester.SendGarbageTerm(); ret = tester.Interact(); BOOST_CHECK(!ret); } @@ -1489,7 +1485,7 @@ BOOST_AUTO_TEST_CASE(v2transport_test) auto ret = tester.Interact(); BOOST_REQUIRE(ret && ret->empty()); tester.ReceiveKey(); - tester.SendGarbageTermAuth(); + tester.SendGarbageTerm(); ret = tester.Interact(); BOOST_CHECK(!ret); } @@ -1514,7 +1510,7 @@ BOOST_AUTO_TEST_CASE(v2transport_test) // the first 15 of them match. garbage[len_before + 15] ^= (uint8_t(1) << InsecureRandRange(8)); tester.SendGarbage(garbage); - tester.SendGarbageTermAuth(); + tester.SendGarbageTerm(); tester.SendVersion(); ret = tester.Interact(); BOOST_REQUIRE(ret && ret->empty()); diff --git a/src/validation.cpp b/src/validation.cpp index 4acd5c7cb0..357b4d422d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2594,7 +2594,11 @@ bool Chainstate::FlushStateToDisk( LOG_TIME_MILLIS_WITH_CATEGORY("write block and undo data to disk", BCLog::BENCH); // First make sure all block and undo data is flushed to disk. - m_blockman.FlushBlockFile(); + // TODO: Handle return error, or add detailed comment why it is + // safe to not return an error upon failure. + if (!m_blockman.FlushBlockFile()) { + LogPrintLevel(BCLog::VALIDATION, BCLog::Level::Warning, "%s: Failed to flush block file.\n", __func__); + } } // Then update all block file information (which may refer to block and undo files). diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 512a011dfc..f4cb4bbd66 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -162,11 +162,11 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid) } Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors, - CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine, const std::vector<CTxOut>& outputs, std::optional<uint32_t> reduce_output) + CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine, const std::vector<CTxOut>& outputs, std::optional<uint32_t> original_change_index) { - // Cannot both specify new outputs and an output to reduce - if (!outputs.empty() && reduce_output.has_value()) { - errors.push_back(Untranslated("Cannot specify both new outputs to use and an output index to reduce")); + // For now, cannot specify both new outputs to use and an output index to send change + if (!outputs.empty() && original_change_index.has_value()) { + errors.push_back(Untranslated("The options 'outputs' and 'original_change_index' are incompatible. You can only either specify a new set of outputs, or designate a change output to be recycled.")); return Result::INVALID_PARAMETER; } @@ -182,8 +182,8 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo } const CWalletTx& wtx = it->second; - // Make sure that reduce_output is valid - if (reduce_output.has_value() && reduce_output.value() >= wtx.tx->vout.size()) { + // Make sure that original_change_index is valid + if (original_change_index.has_value() && original_change_index.value() >= wtx.tx->vout.size()) { errors.push_back(Untranslated("Change position is out of range")); return Result::INVALID_PARAMETER; } @@ -259,7 +259,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo const CTxOut& output = txouts.at(i); CTxDestination dest; ExtractDestination(output.scriptPubKey, dest); - if (reduce_output.has_value() ? reduce_output.value() == i : OutputIsChange(wallet, output)) { + if (original_change_index.has_value() ? original_change_index.value() == i : OutputIsChange(wallet, output)) { new_coin_control.destChange = dest; } else { CRecipient recipient = {dest, output.nValue, false}; diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h index f00bf15730..d3d43861ef 100644 --- a/src/wallet/feebumper.h +++ b/src/wallet/feebumper.h @@ -44,7 +44,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid); * @param[out] mtx The bump transaction itself * @param[in] require_mine Whether the original transaction must consist of inputs that can be spent by the wallet * @param[in] outputs Vector of new outputs to replace the bumped transaction's outputs - * @param[in] reduce_output The position of the change output to deduct the fee from in the transaction being bumped + * @param[in] original_change_index The position of the change output to deduct the fee from in the transaction being bumped */ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, @@ -55,7 +55,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, CMutableTransaction& mtx, bool require_mine, const std::vector<CTxOut>& outputs, - std::optional<uint32_t> reduce_output = std::nullopt); + std::optional<uint32_t> original_change_index = std::nullopt); //! Sign the new transaction, //! @return false if the tx couldn't be found or if it was diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index c2f4321a60..6b96fc4e49 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1016,10 +1016,14 @@ static RPCHelpMan bumpfee_helper(std::string method_name) {"outputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "The outputs specified as key-value pairs.\n" "Each key may only appear once, i.e. there can only be one 'data' output, and no address may be duplicated.\n" "At least one output of either type must be specified.\n" - "Cannot be provided if 'reduce_output' is specified.", + "Cannot be provided if 'original_change_index' is specified.", OutputsDoc(), RPCArgOptions{.skip_type_check = true}}, - {"reduce_output", RPCArg::Type::NUM, RPCArg::DefaultHint{"not set, detect change automatically"}, "The 0-based index of the output from which the additional fees will be deducted. In general, this should be the position of change output. Cannot be provided if 'outputs' is specified."}, + {"original_change_index", RPCArg::Type::NUM, RPCArg::DefaultHint{"not set, detect change automatically"}, "The 0-based index of the change output on the original transaction. " + "The indicated output will be recycled into the new change output on the bumped transaction. " + "The remainder after paying the recipients and fees will be sent to the output script of the " + "original change output. The change output’s amount can increase if bumping the transaction " + "adds new inputs, otherwise it will decrease. Cannot be used in combination with the 'outputs' option."}, }, RPCArgOptions{.oneline_description="options"}}, }, @@ -1058,7 +1062,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) coin_control.m_signal_bip125_rbf = true; std::vector<CTxOut> outputs; - std::optional<uint32_t> reduce_output; + std::optional<uint32_t> original_change_index; if (!request.params[1].isNull()) { UniValue options = request.params[1]; @@ -1070,7 +1074,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) {"replaceable", UniValueType(UniValue::VBOOL)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, {"outputs", UniValueType()}, // will be checked by AddOutputs() - {"reduce_output", UniValueType(UniValue::VNUM)}, + {"original_change_index", UniValueType(UniValue::VNUM)}, }, true, true); @@ -1095,8 +1099,8 @@ static RPCHelpMan bumpfee_helper(std::string method_name) outputs = tempTx.vout; } - if (options.exists("reduce_output")) { - reduce_output = options["reduce_output"].getInt<uint32_t>(); + if (options.exists("original_change_index")) { + original_change_index = options["original_change_index"].getInt<uint32_t>(); } } @@ -1115,7 +1119,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) CMutableTransaction mtx; feebumper::Result res; // Targeting feerate bump. - res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt, outputs, reduce_output); + res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt, outputs, original_change_index); if (res != feebumper::Result::OK) { switch(res) { case feebumper::Result::INVALID_ADDRESS_OR_KEY: |