Repository: bitcoin/bitcoin
File: src/wallet/feebumper.cpp (C++)

Code before:
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <common/system.h>
#include <consensus/validation.h>
#include <interfaces/chain.h>
#include <node/types.h>
#include <policy/fees.h>
#include <policy/policy.h>
#include <util/moneystr.h>
#include <util/rbf.h>
#include <util/translation.h>
#include <wallet/coincontrol.h>
#include <wallet/feebumper.h>
#include <wallet/fees.h>
#include <wallet/receive.h>
#include <wallet/spend.h>
#include <wallet/wallet.h>

namespace wallet {
//! Check whether transaction has descendant in wallet or mempool, or has been
//! mined, or conflicts with a mined transaction. Return a feebumper::Result.
static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, bool require_mine, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
    if (wallet.HasWalletSpend(wtx.tx)) {
        errors.push_back(Untranslated("Transaction has descendants in the wallet"));
        return feebumper::Result::INVALID_PARAMETER;
    }

    {
        if (wallet.chain().hasDescendantsInMempool(wtx.GetHash())) {
            errors.push_back(Untranslated("Transaction has descendants in the mempool"));
            return feebumper::Result::INVALID_PARAMETER;
        }
    }

    if (wallet.GetTxDepthInMainChain(wtx) != 0) {
        errors.push_back(Untranslated("Transaction has been mined, or is conflicted with a mined transaction"));
        return feebumper::Result::WALLET_ERROR;
    }

    if (!SignalsOptInRBF(*wtx.tx)) {
        errors.push_back(Untranslated("Transaction is not BIP 125 replaceable"));
        return feebumper::Result::WALLET_ERROR;
    }

    if (wtx.mapValue.count("replaced_by_txid")) {
        errors.push_back(strprintf(Untranslated("Cannot bump transaction %s which was already bumped by transaction %s"), wtx.GetHash().ToString(), wtx.mapValue.at("replaced_by_txid")));
        return feebumper::Result::WALLET_ERROR;
    }


Review comment (bug):
I tried reproducing the tidy warnings with
```bash
$ clang++ --version
Homebrew clang version 19.1.3

$ cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
&& cmake --build build -j$(nproc) \
&& run-clang-tidy -p build -j$(nproc) -checks='-*,modernize-use-emplace' src/wallet/feebumper.cpp
```

But it doesn't give me the mentioned warnings.
The change itself does make sense (forwarding the bilingual_str params directly), I just can't seem to reproduce the warning.
Based on https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-emplace.html the check targets cases where a temporary object is explicitly constructed in the argument to push_back.
Was this different in the original PR, where the construction happened inside of push_back (or related to any of the warnings below) or am I running something incorrectly?

---

Enabling all warnings for this file I do see a few other ones, some of them may be related to translations, e.g.
```
bitcoin/src/wallet/feebumper.cpp:27:26: error: 'Untranslated' must resolve to a function declared within the namespace defined by the 'LIBC_NAMESPACE' macro [llvmlibc-callee-namespace,-warnings-as-errors]
   27 |         errors.push_back(Untranslated("Transaction has descendants in the wallet"));
      |                          ^
bitcoin/src/util/translation.h:51:22: note: resolves to this declaration
   51 | inline bilingual_str Untranslated(std::string original) { return {original, original}; }
      |                      ^
```
and
```
bitcoin/src/wallet/feebumper.cpp:49:26: error: 'format<std::string, std::string>' must resolve to a function declared within the namespace defined by the 'LIBC_NAMESPACE' macro [llvmlibc-callee-namespace,-warnings-as-errors]
   49 |         errors.push_back(strprintf(Untranslated("Cannot bump transaction %s which was already bumped by transaction %s"), wtx.GetHash().ToString(), wtx.mapValue.at("replaced_by_txid")));
      |                          ^
bitcoin/src/tinyformat.h:1165:19: note: expanded from macro 'strprintf'
 1165 | #define strprintf tfm::format
      |                   ^
bitcoin/src/util/translation.h:56:15: note: resolves to this declaration
   56 | bilingual_str format(const bilingual_str& fmt, const Args&... args)
      |               ^
```

---

nit, shouldn't `Untranslated` pass by const reference instead (or nobody cares since it's inline anyway), i.e. 
```C++
inline bilingual_str Untranslated(const std::string& original) { return {original, original}; }
```
?

Code after:
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <common/system.h>
#include <consensus/validation.h>
#include <interfaces/chain.h>
#include <node/types.h>
#include <policy/fees.h>
#include <policy/policy.h>
#include <util/moneystr.h>
#include <util/rbf.h>
#include <util/translation.h>
#include <wallet/coincontrol.h>
#include <wallet/feebumper.h>
#include <wallet/fees.h>
#include <wallet/receive.h>
#include <wallet/spend.h>
#include <wallet/wallet.h>

namespace wallet {
//! Check whether transaction has descendant in wallet or mempool, or has been
//! mined, or conflicts with a mined transaction. Return a feebumper::Result.
static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, bool require_mine, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
    if (wallet.HasWalletSpend(wtx.tx)) {
        errors.emplace_back(Untranslated("Transaction has descendants in the wallet"));
        return feebumper::Result::INVALID_PARAMETER;
    }

    {
        if (wallet.chain().hasDescendantsInMempool(wtx.GetHash())) {
            errors.emplace_back(Untranslated("Transaction has descendants in the mempool"));
            return feebumper::Result::INVALID_PARAMETER;
        }
    }

    if (wallet.GetTxDepthInMainChain(wtx) != 0) {
        errors.emplace_back(Untranslated("Transaction has been mined, or is conflicted with a mined transaction"));
        return feebumper::Result::WALLET_ERROR;
    }

    if (!SignalsOptInRBF(*wtx.tx)) {
        errors.emplace_back(Untranslated("Transaction is not BIP 125 replaceable"));
        return feebumper::Result::WALLET_ERROR;
    }

    if (wtx.mapValue.count("replaced_by_txid")) {
        errors.push_back(strprintf(Untranslated("Cannot bump transaction %s which was already bumped by transaction %s"), wtx.GetHash().ToString(), wtx.mapValue.at("replaced_by_txid")));
        return feebumper::Result::WALLET_ERROR;
    }