Should this validate the parsed txid?
Txid::FromHex(value) returns std::nullopt for malformed input, so assigning it directly makes an invalid stored replaces_txid / replaced_by_txid indistinguishable from a missing key. If the wallet is later rewritten, that metadata is silently lost.
diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h
index 3697da8e35..ee3a5a2001 100644
--- a/src/wallet/transaction.h
+++ b/src/wallet/transaction.h
@@ -322,9 +322,15 @@ public:
else if (key == "message") m_message = value;
else if (key == "comment") m_comment = value;
else if (key == "to") m_comment_to = value;
- else if (key == "replaces_txid") m_replaces_txid = Txid::FromHex(value);
- else if (key == "replaced_by_txid") m_replaced_by_txid = Txid::FromHex(value);
- else {
+ else if (key == "replaces_txid") {
+ const auto txid{Txid::FromHex(value)};
+ if (!txid) throw std::runtime_error("Invalid replaces_txid value in CWalletTx string value map");
+ m_replaces_txid = *txid;
+ } else if (key == "replaced_by_txid") {
+ const auto txid{Txid::FromHex(value)};
+ if (!txid) throw std::runtime_error("Invalid replaced_by_txid value in CWalletTx string value map");
+ m_replaced_by_txid = *txid;
+ } else {
throw std::runtime_error("Unexpected value in CWalletTx strings value map");
}
}
Unit test (as an example):
<details>
<summary>diff</summary>
diff --git a/src/wallet/test/wallet_transaction_tests.cpp b/src/wallet/test/wallet_transaction_tests.cpp
index 38be52f41f..da3e4d1d3d 100644
--- a/src/wallet/test/wallet_transaction_tests.cpp
+++ b/src/wallet/test/wallet_transaction_tests.cpp
@@ -4,6 +4,7 @@
#include <wallet/transaction.h>
+#include <streams.h>
#include <test/util/common.h>
#include <wallet/test/wallet_test_fixture.h>
@@ -23,5 +24,33 @@ BOOST_AUTO_TEST_CASE(roundtrip)
}
}
+BOOST_AUTO_TEST_CASE(malformed_replacement_txids_throw)
+{
+ auto check_invalid_txid = [](const std::string& key) {
+ const CTransactionRef tx{MakeTransactionRef(CMutableTransaction{})};
+ const TxState state{TxStateInactive{}};
+ const std::map<std::string, std::string> string_values{{key, "not-a-txid"}};
+
+ DataStream stream{};
+ stream << TX_WITH_WITNESS(tx)
+ << TxStateSerializedBlockHash(state)
+ << std::vector<uint8_t>{}
+ << TxStateSerializedIndex(state)
+ << std::vector<uint8_t>{}
+ << string_values
+ << std::vector<std::pair<std::string, std::string>>{}
+ << uint32_t{0}
+ << 0U
+ << false
+ << false;
+
+ CWalletTx wtx{nullptr, TxStateInactive{}};
+ BOOST_CHECK_THROW(stream >> wtx, std::runtime_error);
+ };
+
+ check_invalid_txid("replaces_txid");
+ check_invalid_txid("replaced_by_txid");
+}
+
BOOST_AUTO_TEST_SUITE_END()
} // namespace wallet
</details>
