refactor: remove TxidFromString

TxidFromString was deprecated due to missing 64-character length-check
and hex-check, replace it with the more robust Txid::FromHex.
This commit is contained in:
stickies-v 2024-07-26 11:51:40 +01:00
parent 285ab50ace
commit f553e6d86f
No known key found for this signature in database
GPG key ID: 5CB1CE6E5E66A757
6 changed files with 28 additions and 46 deletions

View file

@ -174,22 +174,17 @@ void CoinControlDialog::showMenu(const QPoint &point)
contextMenuItem = item;
// disable some items (like Copy Transaction ID, lock, unlock) for tree roots in context menu
if (item->data(COLUMN_ADDRESS, TxHashRole).toString().length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode)
{
auto txid{Txid::FromHex(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString())};
if (txid) { // a valid txid means this is a child node, and not a parent node in tree mode
m_copy_transaction_outpoint_action->setEnabled(true);
if (model->wallet().isLockedCoin(COutPoint(TxidFromString(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), item->data(COLUMN_ADDRESS, VOutRole).toUInt())))
{
if (model->wallet().isLockedCoin(COutPoint(*txid, item->data(COLUMN_ADDRESS, VOutRole).toUInt()))) {
lockAction->setEnabled(false);
unlockAction->setEnabled(true);
}
else
{
} else {
lockAction->setEnabled(true);
unlockAction->setEnabled(false);
}
}
else // this means click on parent node in tree mode -> disable all
{
} else { // this means click on parent node in tree mode -> disable all
m_copy_transaction_outpoint_action->setEnabled(false);
lockAction->setEnabled(false);
unlockAction->setEnabled(false);
@ -240,7 +235,7 @@ void CoinControlDialog::lockCoin()
if (contextMenuItem->checkState(COLUMN_CHECKBOX) == Qt::Checked)
contextMenuItem->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked);
COutPoint outpt(TxidFromString(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toUInt());
COutPoint outpt(Txid::FromHex(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()).value(), contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toUInt());
model->wallet().lockCoin(outpt, /* write_to_db = */ true);
contextMenuItem->setDisabled(true);
contextMenuItem->setIcon(COLUMN_CHECKBOX, platformStyle->SingleColorIcon(":/icons/lock_closed"));
@ -250,7 +245,7 @@ void CoinControlDialog::lockCoin()
// context menu action: unlock coin
void CoinControlDialog::unlockCoin()
{
COutPoint outpt(TxidFromString(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toUInt());
COutPoint outpt(Txid::FromHex(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()).value(), contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toUInt());
model->wallet().unlockCoin(outpt);
contextMenuItem->setDisabled(false);
contextMenuItem->setIcon(COLUMN_CHECKBOX, QIcon());
@ -340,9 +335,10 @@ void CoinControlDialog::radioListMode(bool checked)
// checkbox clicked by user
void CoinControlDialog::viewItemChanged(QTreeWidgetItem* item, int column)
{
if (column == COLUMN_CHECKBOX && item->data(COLUMN_ADDRESS, TxHashRole).toString().length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode)
{
COutPoint outpt(TxidFromString(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), item->data(COLUMN_ADDRESS, VOutRole).toUInt());
if (column != COLUMN_CHECKBOX) return;
auto txid{Txid::FromHex(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString())};
if (txid) { // a valid txid means this is a child node, and not a parent node in tree mode
COutPoint outpt(*txid, item->data(COLUMN_ADDRESS, VOutRole).toUInt());
if (item->checkState(COLUMN_CHECKBOX) == Qt::Unchecked)
m_coin_control.UnSelect(outpt);

View file

@ -138,11 +138,11 @@ BOOST_AUTO_TEST_CASE(bloom_match)
BOOST_CHECK_MESSAGE(filter.IsRelevantAndUpdate(tx), "Simple Bloom filter didn't match output address");
filter = CBloomFilter(10, 0.000001, 0, BLOOM_UPDATE_ALL);
filter.insert(COutPoint(TxidFromString("0x90c122d70786e899529d71dbeba91ba216982fb6ba58f3bdaab65e73b7e9260b"), 0));
filter.insert(COutPoint(Txid::FromHex("90c122d70786e899529d71dbeba91ba216982fb6ba58f3bdaab65e73b7e9260b").value(), 0));
BOOST_CHECK_MESSAGE(filter.IsRelevantAndUpdate(tx), "Simple Bloom filter didn't match COutPoint");
filter = CBloomFilter(10, 0.000001, 0, BLOOM_UPDATE_ALL);
COutPoint prevOutPoint(TxidFromString("0x90c122d70786e899529d71dbeba91ba216982fb6ba58f3bdaab65e73b7e9260b"), 0);
COutPoint prevOutPoint(Txid::FromHex("90c122d70786e899529d71dbeba91ba216982fb6ba58f3bdaab65e73b7e9260b").value(), 0);
{
std::vector<unsigned char> data(32 + sizeof(unsigned int));
memcpy(data.data(), prevOutPoint.hash.begin(), 32);
@ -160,11 +160,11 @@ BOOST_AUTO_TEST_CASE(bloom_match)
BOOST_CHECK_MESSAGE(!filter.IsRelevantAndUpdate(tx), "Simple Bloom filter matched random address");
filter = CBloomFilter(10, 0.000001, 0, BLOOM_UPDATE_ALL);
filter.insert(COutPoint(TxidFromString("0x90c122d70786e899529d71dbeba91ba216982fb6ba58f3bdaab65e73b7e9260b"), 1));
filter.insert(COutPoint(Txid::FromHex("90c122d70786e899529d71dbeba91ba216982fb6ba58f3bdaab65e73b7e9260b").value(), 1));
BOOST_CHECK_MESSAGE(!filter.IsRelevantAndUpdate(tx), "Simple Bloom filter matched COutPoint for an output we didn't care about");
filter = CBloomFilter(10, 0.000001, 0, BLOOM_UPDATE_ALL);
filter.insert(COutPoint(TxidFromString("0x000000d70786e899529d71dbeba91ba216982fb6ba58f3bdaab65e73b7e9260b"), 0));
filter.insert(COutPoint(Txid::FromHex("000000d70786e899529d71dbeba91ba216982fb6ba58f3bdaab65e73b7e9260b").value(), 0));
BOOST_CHECK_MESSAGE(!filter.IsRelevantAndUpdate(tx), "Simple Bloom filter matched COutPoint for an output we didn't care about");
}
@ -426,9 +426,9 @@ BOOST_AUTO_TEST_CASE(merkle_block_4_test_p2pubkey_only)
BOOST_CHECK(merkleBlock.header.GetHash() == block.GetHash());
// We should match the generation outpoint
BOOST_CHECK(filter.contains(COutPoint(TxidFromString("0x147caa76786596590baa4e98f5d9f48b86c7765e489f7a6ff3360fe5c674360b"), 0)));
BOOST_CHECK(filter.contains(COutPoint(Txid::FromHex("147caa76786596590baa4e98f5d9f48b86c7765e489f7a6ff3360fe5c674360b").value(), 0)));
// ... but not the 4th transaction's output (its not pay-2-pubkey)
BOOST_CHECK(!filter.contains(COutPoint(TxidFromString("0x02981fa052f0481dbc5868f4fc2166035a10f27a03cfd2de67326471df5bc041"), 0)));
BOOST_CHECK(!filter.contains(COutPoint(Txid::FromHex("02981fa052f0481dbc5868f4fc2166035a10f27a03cfd2de67326471df5bc041").value(), 0)));
}
BOOST_AUTO_TEST_CASE(merkle_block_4_test_update_none)
@ -451,8 +451,8 @@ BOOST_AUTO_TEST_CASE(merkle_block_4_test_update_none)
BOOST_CHECK(merkleBlock.header.GetHash() == block.GetHash());
// We shouldn't match any outpoints (UPDATE_NONE)
BOOST_CHECK(!filter.contains(COutPoint(TxidFromString("0x147caa76786596590baa4e98f5d9f48b86c7765e489f7a6ff3360fe5c674360b"), 0)));
BOOST_CHECK(!filter.contains(COutPoint(TxidFromString("0x02981fa052f0481dbc5868f4fc2166035a10f27a03cfd2de67326471df5bc041"), 0)));
BOOST_CHECK(!filter.contains(COutPoint(Txid::FromHex("147caa76786596590baa4e98f5d9f48b86c7765e489f7a6ff3360fe5c674360b").value(), 0)));
BOOST_CHECK(!filter.contains(COutPoint(Txid::FromHex("02981fa052f0481dbc5868f4fc2166035a10f27a03cfd2de67326471df5bc041").value(), 0)));
}
static std::vector<unsigned char> RandomData()

View file

@ -24,10 +24,10 @@ BOOST_AUTO_TEST_CASE(merkleblock_construct_from_txids_found)
std::set<Txid> txids;
// Last txn in block.
Txid txhash1{TxidFromString("0x74d681e0e03bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20")};
Txid txhash1{Txid::FromHex("74d681e0e03bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20").value()};
// Second txn in block.
Txid txhash2{TxidFromString("0xf9fc751cb7dc372406a9f8d738d5e6f8f63bab71986a39cf36ee70ee17036d07")};
Txid txhash2{Txid::FromHex("f9fc751cb7dc372406a9f8d738d5e6f8f63bab71986a39cf36ee70ee17036d07").value()};
txids.insert(txhash1);
txids.insert(txhash2);
@ -63,7 +63,7 @@ BOOST_AUTO_TEST_CASE(merkleblock_construct_from_txids_not_found)
CBlock block = getBlock13b8a();
std::set<Txid> txids2;
txids2.insert(TxidFromString("0xc0ffee00003bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20"));
txids2.insert(Txid::FromHex("c0ffee00003bafa802c8aa084379aa98d9fcd632ddc2ed9782b586ec87451f20").value());
CMerkleBlock merkleBlock(block, txids2);
BOOST_CHECK_EQUAL(merkleBlock.header.GetHash().GetHex(), block.GetHash().GetHex());

View file

@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
fValid = false;
break;
}
COutPoint outpoint{TxidFromString(vinput[0].get_str()), uint32_t(vinput[1].getInt<int>())};
COutPoint outpoint{Txid::FromHex(vinput[0].get_str()).value(), uint32_t(vinput[1].getInt<int>())};
mapprevOutScriptPubKeys[outpoint] = ParseScript(vinput[2].get_str());
if (vinput.size() >= 4)
{
@ -311,7 +311,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
fValid = false;
break;
}
COutPoint outpoint{TxidFromString(vinput[0].get_str()), uint32_t(vinput[1].getInt<int>())};
COutPoint outpoint{Txid::FromHex(vinput[0].get_str()).value(), uint32_t(vinput[1].getInt<int>())};
mapprevOutScriptPubKeys[outpoint] = ParseScript(vinput[2].get_str());
if (vinput.size() >= 4)
{
@ -542,7 +542,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction)
// create a big transaction of 4500 inputs signed by the same key
for(uint32_t ij = 0; ij < 4500; ij++) {
uint32_t i = mtx.vin.size();
COutPoint outpoint(TxidFromString("0000000000000000000000000000000000000000000000000000000000000100"), i);
COutPoint outpoint(Txid::FromHex("0000000000000000000000000000000000000000000000000000000000000100").value(), i);
mtx.vin.resize(mtx.vin.size() + 1);
mtx.vin[i].prevout = outpoint;
@ -1028,12 +1028,4 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
}
}
BOOST_AUTO_TEST_CASE(test_TxidFromString)
{
// Make sure TxidFromString respects string_view length and stops reading at
// end of the substring.
BOOST_CHECK_EQUAL(TxidFromString(std::string_view("ABCD1234", 4)).ToString(),
"000000000000000000000000000000000000000000000000000000000000abcd");
}
BOOST_AUTO_TEST_SUITE_END()

View file

@ -78,9 +78,9 @@ BOOST_FIXTURE_TEST_CASE(package_hash_tests, TestChain100Setup)
BOOST_CHECK(wtxid_2.GetHex() < wtxid_3.GetHex());
// The txids are not (we want to test that sorting and hashing use wtxid, not txid):
Txid txid_1{TxidFromString("0xbd0f71c1d5e50589063e134fad22053cdae5ab2320db5bf5e540198b0b5a4e69")};
Txid txid_2{TxidFromString("0xb4749f017444b051c44dfd2720e88f314ff94f3dd6d56d40ef65854fcd7fff6b")};
Txid txid_3{TxidFromString("0xee707be5201160e32c4fc715bec227d1aeea5940fb4295605e7373edce3b1a93")};
Txid txid_1{Txid::FromHex("bd0f71c1d5e50589063e134fad22053cdae5ab2320db5bf5e540198b0b5a4e69").value()};
Txid txid_2{Txid::FromHex("b4749f017444b051c44dfd2720e88f314ff94f3dd6d56d40ef65854fcd7fff6b").value()};
Txid txid_3{Txid::FromHex("ee707be5201160e32c4fc715bec227d1aeea5940fb4295605e7373edce3b1a93").value()};
BOOST_CHECK_EQUAL(tx_1.GetHash(), txid_1);
BOOST_CHECK_EQUAL(tx_2.GetHash(), txid_2);
BOOST_CHECK_EQUAL(tx_3.GetHash(), txid_3);

View file

@ -72,10 +72,4 @@ using Txid = transaction_identifier<false>;
/** Wtxid commits to all transaction fields including the witness. */
using Wtxid = transaction_identifier<true>;
/** DEPRECATED due to missing length-check and hex-check, please use the safer FromHex, or FromUint256 */
inline Txid TxidFromString(std::string_view str)
{
return Txid::FromUint256(uint256S(str));
}
#endif // BITCOIN_UTIL_TRANSACTION_IDENTIFIER_H