mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-01-31 10:41:08 +00:00
Merge bitcoin/bitcoin#34397: doc: fix arg name hints so bugprone can validate them
a73a3ec5532ddc05c1b013d868d9994f2889c9cf doc: fix invalid arg name hints for bugprone validation (Lőrinc) Pull request description: The extra leading `=` or missing trailing `=` prevented clang-tidy's `bugprone-argument-comment` check from validating the parameter name, as it only matches comments formatted strictly as `/*parameter_name=*/` (see https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html). I have considered doing a scripted diff, but the values I found aren't so numerous and can easily be reviewed manually. ACKs for top commit: b-l-u-e: ACK a73a3ec tested and saw that argument comments now use the strict "/*param=*/" format required by bugprone-argument-comment Sjors: utACK a73a3ec5532ddc05c1b013d868d9994f2889c9cf maflcko: review ACK a73a3ec5532ddc05c1b013d868d9994f2889c9cf 🍦 Tree-SHA512: 31177934d645116f381668a0f945028d7e04fab1fc6185dd0e3b7451aab71f89f1e4dd07246db667d1c4734eea3e5d73433c8b0e09181b3ece47dacc8677401e
This commit is contained in:
commit
34a5ecadd7
@ -106,7 +106,7 @@ static void MemPoolAddTransactions(benchmark::Bench& bench)
|
||||
std::vector<CTransactionRef> transactions;
|
||||
// Create 1000 clusters of 100 transactions each
|
||||
for (int i=0; i<100; i++) {
|
||||
auto new_txs = CreateCoinCluster(det_rand, childTxs, /*min_ancestors*/ 1);
|
||||
auto new_txs = CreateCoinCluster(det_rand, childTxs, /*min_ancestors=*/ 1);
|
||||
transactions.insert(transactions.end(), new_txs.begin(), new_txs.end());
|
||||
}
|
||||
|
||||
@ -156,7 +156,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
|
||||
// in the same state at the end of the function, so we benchmark both
|
||||
// mining a block and reorging the block's contents back into the mempool.
|
||||
bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
|
||||
pool.removeForBlock(tx_remove_for_block, /*nBlockHeight*/100);
|
||||
pool.removeForBlock(tx_remove_for_block, /*nBlockHeight=*/100);
|
||||
for (auto& tx: tx_remove_for_block) {
|
||||
AddTx(tx, pool, det_rand);
|
||||
}
|
||||
|
||||
@ -980,7 +980,7 @@ public:
|
||||
bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) override
|
||||
{
|
||||
LOCK(chainman().GetMutex());
|
||||
BlockValidationState state{TestBlockValidity(chainman().ActiveChainstate(), block, /*check_pow=*/options.check_pow, /*=check_merkle_root=*/options.check_merkle_root)};
|
||||
BlockValidationState state{TestBlockValidity(chainman().ActiveChainstate(), block, /*check_pow=*/options.check_pow, /*check_merkle_root=*/options.check_merkle_root)};
|
||||
reason = state.GetRejectReason();
|
||||
debug = state.GetDebugMessage();
|
||||
return state.IsValid();
|
||||
|
||||
@ -1084,7 +1084,7 @@ static RPCHelpMan getorphantxs()
|
||||
PeerManager& peerman = EnsurePeerman(node);
|
||||
std::vector<node::TxOrphanage::OrphanInfo> orphanage = peerman.GetOrphanTransactions();
|
||||
|
||||
int verbosity{ParseVerbosity(request.params[0], /*default_verbosity=*/0, /*allow_bool*/false)};
|
||||
int verbosity{ParseVerbosity(request.params[0], /*default_verbosity=*/0, /*allow_bool=*/false)};
|
||||
|
||||
UniValue ret(UniValue::VARR);
|
||||
|
||||
|
||||
@ -153,7 +153,7 @@ BOOST_AUTO_TEST_CASE(sneaky_redownload)
|
||||
// Pretend the message is still "full", so we don't abort.
|
||||
CHECK_RESULT(hss.ProcessNextHeaders({{first_chain.front()}}, /*full_headers_message=*/true),
|
||||
hss, /*exp_state=*/State::PRESYNC,
|
||||
/*exp_success*/true, /*exp_request_more=*/true,
|
||||
/*exp_success=*/true, /*exp_request_more=*/true,
|
||||
/*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
|
||||
/*exp_locator_hash=*/first_chain.front().GetHash());
|
||||
|
||||
@ -161,7 +161,7 @@ BOOST_AUTO_TEST_CASE(sneaky_redownload)
|
||||
// requirement during PRESYNC and transitioned to REDOWNLOAD.
|
||||
CHECK_RESULT(hss.ProcessNextHeaders(std::span{first_chain}.subspan(1), true),
|
||||
hss, /*exp_state=*/State::REDOWNLOAD,
|
||||
/*exp_success*/true, /*exp_request_more=*/true,
|
||||
/*exp_success=*/true, /*exp_request_more=*/true,
|
||||
/*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
|
||||
/*exp_locator_hash=*/genesis.GetHash());
|
||||
|
||||
@ -173,7 +173,7 @@ BOOST_AUTO_TEST_CASE(sneaky_redownload)
|
||||
// Try to sneakily feed back the second chain during REDOWNLOAD.
|
||||
CHECK_RESULT(hss.ProcessNextHeaders(second_chain, true),
|
||||
hss, /*exp_state=*/State::FINAL,
|
||||
/*exp_success*/false, // Foiled! We detected mismatching headers.
|
||||
/*exp_success=*/false, // Foiled! We detected mismatching headers.
|
||||
/*exp_request_more=*/false,
|
||||
/*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
|
||||
/*exp_locator_hash=*/std::nullopt);
|
||||
@ -192,7 +192,7 @@ BOOST_AUTO_TEST_CASE(happy_path)
|
||||
const auto genesis_hash{genesis.GetHash()};
|
||||
CHECK_RESULT(hss.ProcessNextHeaders(first_chain, full_headers_message),
|
||||
hss, /*exp_state=*/State::REDOWNLOAD,
|
||||
/*exp_success*/true, /*exp_request_more=*/true,
|
||||
/*exp_success=*/true, /*exp_request_more=*/true,
|
||||
/*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
|
||||
/*exp_locator_hash=*/genesis_hash);
|
||||
|
||||
@ -200,14 +200,14 @@ BOOST_AUTO_TEST_CASE(happy_path)
|
||||
// validated headers shouldn't be returned yet:
|
||||
CHECK_RESULT(hss.ProcessNextHeaders({first_chain.begin(), REDOWNLOAD_BUFFER_SIZE}, true),
|
||||
hss, /*exp_state=*/State::REDOWNLOAD,
|
||||
/*exp_success*/true, /*exp_request_more=*/true,
|
||||
/*exp_success=*/true, /*exp_request_more=*/true,
|
||||
/*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
|
||||
/*exp_locator_hash=*/first_chain[REDOWNLOAD_BUFFER_SIZE - 1].GetHash());
|
||||
|
||||
// We start receiving headers for permanent storage before completing:
|
||||
CHECK_RESULT(hss.ProcessNextHeaders({{first_chain[REDOWNLOAD_BUFFER_SIZE]}}, true),
|
||||
hss, /*exp_state=*/State::REDOWNLOAD,
|
||||
/*exp_success*/true, /*exp_request_more=*/true,
|
||||
/*exp_success=*/true, /*exp_request_more=*/true,
|
||||
/*exp_headers_size=*/1, /*exp_pow_validated_prev=*/genesis_hash,
|
||||
/*exp_locator_hash=*/first_chain[REDOWNLOAD_BUFFER_SIZE].GetHash());
|
||||
|
||||
@ -215,7 +215,7 @@ BOOST_AUTO_TEST_CASE(happy_path)
|
||||
// completing the REDOWNLOAD phase:
|
||||
CHECK_RESULT(hss.ProcessNextHeaders({first_chain.begin() + REDOWNLOAD_BUFFER_SIZE + 1, first_chain.end()}, full_headers_message),
|
||||
hss, /*exp_state=*/State::FINAL,
|
||||
/*exp_success*/true, /*exp_request_more=*/false,
|
||||
/*exp_success=*/true, /*exp_request_more=*/false,
|
||||
// All headers except the one already returned above:
|
||||
/*exp_headers_size=*/first_chain.size() - 1, /*exp_pow_validated_prev=*/first_chain.front().GetHash(),
|
||||
/*exp_locator_hash=*/std::nullopt);
|
||||
@ -234,7 +234,7 @@ BOOST_AUTO_TEST_CASE(too_little_work)
|
||||
// Pretend just the first message is "full", so we don't abort.
|
||||
CHECK_RESULT(hss.ProcessNextHeaders({{second_chain.front()}}, true),
|
||||
hss, /*exp_state=*/State::PRESYNC,
|
||||
/*exp_success*/true, /*exp_request_more=*/true,
|
||||
/*exp_success=*/true, /*exp_request_more=*/true,
|
||||
/*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
|
||||
/*exp_locator_hash=*/second_chain.front().GetHash());
|
||||
|
||||
@ -245,7 +245,7 @@ BOOST_AUTO_TEST_CASE(too_little_work)
|
||||
hss, /*exp_state=*/State::FINAL,
|
||||
// Nevertheless, no validation errors should have been detected with the
|
||||
// chain:
|
||||
/*exp_success*/true,
|
||||
/*exp_success=*/true,
|
||||
/*exp_request_more=*/false,
|
||||
/*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
|
||||
/*exp_locator_hash=*/std::nullopt);
|
||||
|
||||
@ -221,7 +221,7 @@ BOOST_FIXTURE_TEST_CASE(rbf_conflicts_calculator, TestChain100Setup)
|
||||
dummy.clear();
|
||||
|
||||
// If we mine the parent_tx's, then the clusters split (102 clusters).
|
||||
pool.removeForBlock({parent_tx_1, parent_tx_2}, /* dummy */ 1);
|
||||
pool.removeForBlock({parent_tx_1, parent_tx_2}, /*nBlockHeight=*/ 1);
|
||||
|
||||
// Add some descendants now to each of the direct children (we can do this now that the clusters have split).
|
||||
for (const auto& child : direct_children) {
|
||||
|
||||
@ -484,63 +484,63 @@ public:
|
||||
static ATMPArgs SingleAccept(const CChainParams& chainparams, int64_t accept_time,
|
||||
bool bypass_limits, std::vector<COutPoint>& coins_to_uncache,
|
||||
bool test_accept) {
|
||||
return ATMPArgs{/* m_chainparams */ chainparams,
|
||||
/* m_accept_time */ accept_time,
|
||||
/* m_bypass_limits */ bypass_limits,
|
||||
/* m_coins_to_uncache */ coins_to_uncache,
|
||||
/* m_test_accept */ test_accept,
|
||||
/* m_allow_replacement */ true,
|
||||
/* m_allow_sibling_eviction */ true,
|
||||
/* m_package_submission */ false,
|
||||
/* m_package_feerates */ false,
|
||||
/* m_client_maxfeerate */ {}, // checked by caller
|
||||
return ATMPArgs{/*chainparams=*/ chainparams,
|
||||
/*accept_time=*/ accept_time,
|
||||
/*bypass_limits=*/ bypass_limits,
|
||||
/*coins_to_uncache=*/ coins_to_uncache,
|
||||
/*test_accept=*/ test_accept,
|
||||
/*allow_replacement=*/ true,
|
||||
/*allow_sibling_eviction=*/ true,
|
||||
/*package_submission=*/ false,
|
||||
/*package_feerates=*/ false,
|
||||
/*client_maxfeerate=*/ {}, // checked by caller
|
||||
};
|
||||
}
|
||||
|
||||
/** Parameters for test package mempool validation through testmempoolaccept. */
|
||||
static ATMPArgs PackageTestAccept(const CChainParams& chainparams, int64_t accept_time,
|
||||
std::vector<COutPoint>& coins_to_uncache) {
|
||||
return ATMPArgs{/* m_chainparams */ chainparams,
|
||||
/* m_accept_time */ accept_time,
|
||||
/* m_bypass_limits */ false,
|
||||
/* m_coins_to_uncache */ coins_to_uncache,
|
||||
/* m_test_accept */ true,
|
||||
/* m_allow_replacement */ false,
|
||||
/* m_allow_sibling_eviction */ false,
|
||||
/* m_package_submission */ false, // not submitting to mempool
|
||||
/* m_package_feerates */ false,
|
||||
/* m_client_maxfeerate */ {}, // checked by caller
|
||||
return ATMPArgs{/*chainparams=*/ chainparams,
|
||||
/*accept_time=*/ accept_time,
|
||||
/*bypass_limits=*/ false,
|
||||
/*coins_to_uncache=*/ coins_to_uncache,
|
||||
/*test_accept=*/ true,
|
||||
/*allow_replacement=*/ false,
|
||||
/*allow_sibling_eviction=*/ false,
|
||||
/*package_submission=*/ false, // not submitting to mempool
|
||||
/*package_feerates=*/ false,
|
||||
/*client_maxfeerate=*/ {}, // checked by caller
|
||||
};
|
||||
}
|
||||
|
||||
/** Parameters for child-with-parents package validation. */
|
||||
static ATMPArgs PackageChildWithParents(const CChainParams& chainparams, int64_t accept_time,
|
||||
std::vector<COutPoint>& coins_to_uncache, const std::optional<CFeeRate>& client_maxfeerate) {
|
||||
return ATMPArgs{/* m_chainparams */ chainparams,
|
||||
/* m_accept_time */ accept_time,
|
||||
/* m_bypass_limits */ false,
|
||||
/* m_coins_to_uncache */ coins_to_uncache,
|
||||
/* m_test_accept */ false,
|
||||
/* m_allow_replacement */ true,
|
||||
/* m_allow_sibling_eviction */ false,
|
||||
/* m_package_submission */ true,
|
||||
/* m_package_feerates */ true,
|
||||
/* m_client_maxfeerate */ client_maxfeerate,
|
||||
return ATMPArgs{/*chainparams=*/ chainparams,
|
||||
/*accept_time=*/ accept_time,
|
||||
/*bypass_limits=*/ false,
|
||||
/*coins_to_uncache=*/ coins_to_uncache,
|
||||
/*test_accept=*/ false,
|
||||
/*allow_replacement=*/ true,
|
||||
/*allow_sibling_eviction=*/ false,
|
||||
/*package_submission=*/ true,
|
||||
/*package_feerates=*/ true,
|
||||
/*client_maxfeerate=*/ client_maxfeerate,
|
||||
};
|
||||
}
|
||||
|
||||
/** Parameters for a single transaction within a package. */
|
||||
static ATMPArgs SingleInPackageAccept(const ATMPArgs& package_args) {
|
||||
return ATMPArgs{/* m_chainparams */ package_args.m_chainparams,
|
||||
/* m_accept_time */ package_args.m_accept_time,
|
||||
/* m_bypass_limits */ false,
|
||||
/* m_coins_to_uncache */ package_args.m_coins_to_uncache,
|
||||
/* m_test_accept */ package_args.m_test_accept,
|
||||
/* m_allow_replacement */ true,
|
||||
/* m_allow_sibling_eviction */ true,
|
||||
/* m_package_submission */ true, // trim at the end of AcceptPackage()
|
||||
/* m_package_feerates */ false, // only 1 transaction
|
||||
/* m_client_maxfeerate */ package_args.m_client_maxfeerate,
|
||||
return ATMPArgs{/*chainparams=*/ package_args.m_chainparams,
|
||||
/*accept_time=*/ package_args.m_accept_time,
|
||||
/*bypass_limits=*/ false,
|
||||
/*coins_to_uncache=*/ package_args.m_coins_to_uncache,
|
||||
/*test_accept=*/ package_args.m_test_accept,
|
||||
/*allow_replacement=*/ true,
|
||||
/*allow_sibling_eviction=*/ true,
|
||||
/*package_submission=*/ true, // trim at the end of AcceptPackage()
|
||||
/*package_feerates=*/ false, // only 1 transaction
|
||||
/*client_maxfeerate=*/ package_args.m_client_maxfeerate,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@ -530,7 +530,7 @@ RPCHelpMan simulaterawtransaction()
|
||||
|
||||
for (size_t i = 0; i < txs.size(); ++i) {
|
||||
CMutableTransaction mtx;
|
||||
if (!DecodeHexTx(mtx, txs[i].get_str(), /* try_no_witness */ true, /* try_witness */ true)) {
|
||||
if (!DecodeHexTx(mtx, txs[i].get_str(), /*try_no_witness=*/ true, /*try_witness=*/ true)) {
|
||||
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Transaction hex string decoding failure.");
|
||||
}
|
||||
|
||||
|
||||
@ -903,7 +903,7 @@ util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coin
|
||||
// Try with unsafe inputs if they are allowed. This may spend unconfirmed outputs
|
||||
// received from other wallets.
|
||||
if (coin_selection_params.m_include_unsafe_inputs) {
|
||||
ordered_filters.push_back({CoinEligibilityFilter(/*conf_mine=*/0, /*conf_theirs*/0, max_ancestors-1, max_descendants-1, /*include_partial=*/true)});
|
||||
ordered_filters.push_back({CoinEligibilityFilter(/*conf_mine=*/0, /*conf_theirs=*/0, max_ancestors-1, max_descendants-1, /*include_partial=*/true)});
|
||||
}
|
||||
// Try with unlimited ancestors/descendants. The transaction will still need to meet
|
||||
// mempool ancestor/descendant policy to be accepted to mempool and broadcasted, but
|
||||
|
||||
@ -24,7 +24,7 @@ static const int P2WPKH_OUTPUT_VSIZE = 31;
|
||||
static CoinSelectionParams init_default_params()
|
||||
{
|
||||
CoinSelectionParams dcsp{
|
||||
/*rng_fast*/default_rand,
|
||||
/*rng_fast=*/default_rand,
|
||||
/*change_output_size=*/P2WPKH_OUTPUT_VSIZE,
|
||||
/*change_spend_size=*/P2WPKH_INPUT_VSIZE,
|
||||
/*min_change_target=*/50'000,
|
||||
|
||||
@ -277,14 +277,14 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
|
||||
add_coin(available_coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
|
||||
|
||||
CAmount selection_target = 16 * CENT;
|
||||
const auto& no_res = SelectCoinsBnB(GroupCoins(available_coins.All(), /*subtract_fee_outputs*/true),
|
||||
const auto& no_res = SelectCoinsBnB(GroupCoins(available_coins.All(), /*subtract_fee_outputs=*/true),
|
||||
selection_target, /*cost_of_change=*/0, MAX_STANDARD_TX_WEIGHT);
|
||||
BOOST_REQUIRE(!no_res);
|
||||
BOOST_CHECK(util::ErrorString(no_res).original.find("The inputs size exceeds the maximum weight") != std::string::npos);
|
||||
|
||||
// Now add same coin value with a good size and check that it gets selected
|
||||
add_coin(available_coins, *wallet, 5 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
|
||||
const auto& res = SelectCoinsBnB(GroupCoins(available_coins.All(), /*subtract_fee_outputs*/true), selection_target, /*cost_of_change=*/0);
|
||||
const auto& res = SelectCoinsBnB(GroupCoins(available_coins.All(), /*subtract_fee_outputs=*/true), selection_target, /*cost_of_change=*/0);
|
||||
|
||||
expected_result.Clear();
|
||||
add_coin(8 * CENT, 2, expected_result);
|
||||
|
||||
@ -332,7 +332,7 @@ BOOST_FIXTURE_TEST_CASE(LoadReceiveRequests, TestingSetup)
|
||||
auto requests = wallet->GetAddressReceiveRequests();
|
||||
auto erequests = {"val_rr11", "val_rr20"};
|
||||
BOOST_CHECK_EQUAL_COLLECTIONS(requests.begin(), requests.end(), std::begin(erequests), std::end(erequests));
|
||||
RunWithinTxn(wallet->GetDatabase(), /*process_desc*/"test", [](WalletBatch& batch){
|
||||
RunWithinTxn(wallet->GetDatabase(), /*process_desc=*/"test", [](WalletBatch& batch){
|
||||
BOOST_CHECK(batch.WriteAddressPreviouslySpent(PKHash(), false));
|
||||
BOOST_CHECK(batch.EraseAddressData(ScriptHash()));
|
||||
return true;
|
||||
|
||||
@ -4169,7 +4169,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error,
|
||||
// Parse the descriptor
|
||||
FlatSigningProvider keys;
|
||||
std::string parse_err;
|
||||
std::vector<std::unique_ptr<Descriptor>> descs = Parse(desc_str, keys, parse_err, /* require_checksum */ true);
|
||||
std::vector<std::unique_ptr<Descriptor>> descs = Parse(desc_str, keys, parse_err, /*require_checksum=*/ true);
|
||||
assert(descs.size() == 1); // It shouldn't be possible to have the LegacyScriptPubKeyMan make an invalid descriptor or a multipath descriptors
|
||||
assert(!descs.at(0)->IsRange()); // It shouldn't be possible to have LegacyScriptPubKeyMan make a ranged watchonly descriptor
|
||||
|
||||
@ -4208,7 +4208,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error,
|
||||
// Parse the descriptor
|
||||
FlatSigningProvider keys;
|
||||
std::string parse_err;
|
||||
std::vector<std::unique_ptr<Descriptor>> descs = Parse(desc_str, keys, parse_err, /* require_checksum */ true);
|
||||
std::vector<std::unique_ptr<Descriptor>> descs = Parse(desc_str, keys, parse_err, /*require_checksum=*/ true);
|
||||
assert(descs.size() == 1); // It shouldn't be possible to have the LegacyScriptPubKeyMan make an invalid descriptor or a multipath descriptors
|
||||
assert(!descs.at(0)->IsRange()); // It shouldn't be possible to have LegacyScriptPubKeyMan make a ranged watchonly descriptor
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user