From b8251d286351fb8f0bf74c313859c07c624b0ec9 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Mon, 9 Aug 2021 14:21:56 +1200 Subject: [PATCH] Merge bitcoin/bitcoin#22337: wallet: Use bilingual_str for errors 92993aa5cf37995e65e68dfd6f129ecaf418e01c Change SignTransaction's input_errors to use bilingual_str (Andrew Chow) 171366e89b828a557f8262d9dc14ff7a03f813f7 Use bilingual_str for address fetching functions (Andrew Chow) 9571c69b51115454c6a699be9492024f7b46c2b4 Add bilingual_str::clear() (Andrew Chow) Pull request description: In a couple of places in the wallet, errors are `std::string`. In order for these errors to be translated, change them to use `bilingual_str`. ACKs for top commit: hebasto: re-ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c, only rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22337#pullrequestreview-694542729) review, verified with klementtan: Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c meshcollider: Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c Tree-SHA512: 5400e419dd87db8c49b67ed0964de2d44b58010a566ca246f2f0760ed9ef6a9b6f6df7a6adcb211b315b74c727bfe8c7d07eb5690b5922fa5828ceef4c83461f --- src/coinjoin/client.cpp | 6 +++--- src/interfaces/chain.h | 2 +- src/node/interfaces.cpp | 2 +- src/node/transaction.cpp | 7 ++++--- src/node/transaction.h | 4 ++-- src/qt/psbtoperationsdialog.cpp | 2 +- src/rpc/rawtransaction.cpp | 5 +++-- src/rpc/rawtransaction_util.cpp | 9 +++++---- src/rpc/rawtransaction_util.h | 3 ++- src/script/sign.cpp | 11 ++++++----- src/script/sign.h | 3 ++- src/test/fuzz/script_sign.cpp | 3 ++- src/test/util/setup_common.cpp | 2 +- src/test/util/wallet.cpp | 3 ++- src/util/translation.h | 6 ++++++ src/wallet/interfaces.cpp | 3 ++- src/wallet/rpcwallet.cpp | 10 +++++----- src/wallet/scriptpubkeyman.cpp | 18 +++++++++--------- src/wallet/scriptpubkeyman.h | 12 ++++++------ src/wallet/test/coinselector_tests.cpp | 3 ++- src/wallet/test/wallet_tests.cpp | 8 ++++---- src/wallet/wallet.cpp | 24 ++++++++++++------------ src/wallet/wallet.h | 8 ++++---- 23 files changed, 85 insertions(+), 69 deletions(-) diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index e260ca21abeb8e..a0de82d350056b 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -638,14 +638,14 @@ bool CCoinJoinClientSession::SignFinalTransaction(CNode& peer, CChainState& acti // fill values for found outpoints m_wallet.chain().findCoins(coins); - std::map signing_errors; + std::map signing_errors; m_wallet.SignTransaction(finalMutableTransaction, coins, SIGHASH_ALL | SIGHASH_ANYONECANPAY, signing_errors); for (const auto& [input_index, error_string] : signing_errors) { // NOTE: this is a partial signing so it's expected for SignTransaction to return // "Input not found or already spent" errors for inputs that aren't ours - if (error_string != "Input not found or already spent") { - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- signing input %d failed: %s!\n", __func__, input_index, error_string); + if (error_string.original != "Input not found or already spent") { + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- signing input %d failed: %s!\n", __func__, input_index, error_string.original); UnlockCoins(); keyHolderStorage.ReturnAll(); SetNull(); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index dcc399a29de9d8..d84688dcc0b247 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -198,7 +198,7 @@ class Chain virtual bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, bool relay, - std::string& err_string) = 0; + bilingual_str& err_string) = 0; //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 7c51e7255ac232..1817ee42032622 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -878,7 +878,7 @@ class ChainImpl : public Chain auto it = m_node.mempool->GetIter(txid); return it && (*it)->GetCountWithDescendants() > 1; } - bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, bool relay, std::string& err_string) override + bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, bool relay, bilingual_str& err_string) override { const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback*/ false); // Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures. diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 76264ffb1cfda1..9c039c1a6e0bac 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -28,7 +29,7 @@ static TransactionError HandleATMPError(const TxValidationState& state, std::str } } -TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback, bool bypass_limits) +TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, bilingual_str& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback, bool bypass_limits) { // BroadcastTransaction can be called by either sendrawtransaction RPC or wallet RPCs. // node.peerman is assigned both before chain clients and before RPC server is accepting calls, @@ -59,7 +60,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, bypass_limits, true /* test_accept */); if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { - return HandleATMPError(result.m_state, err_string); + return HandleATMPError(result.m_state, err_string.original); } else if (result.m_base_fees.value() > max_tx_fee) { return TransactionError::MAX_FEE_EXCEEDED; } @@ -68,7 +69,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, bypass_limits, false /* test_accept */); if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { - return HandleATMPError(result.m_state, err_string); + return HandleATMPError(result.m_state, err_string.original); } // Transaction was accepted to the mempool. diff --git a/src/node/transaction.h b/src/node/transaction.h index 3867401586fa09..511a252817b5dc 100644 --- a/src/node/transaction.h +++ b/src/node/transaction.h @@ -35,12 +35,12 @@ static const CFeeRate DEFAULT_MAX_RAW_TX_FEE_RATE{COIN / 10}; * * @param[in] node reference to node context * @param[in] tx the transaction to broadcast - * @param[out] err_string reference to std::string to fill with error string if available + * @param[out] err_string reference to bilingual_str to fill with error string if available * @param[in] relay flag if both mempool insertion and p2p relay are requested * @param[in] wait_callback wait until callbacks have been processed to avoid stale result due to a sequentially RPC. * return error */ -[[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, std::string& err_string, const CAmount& highfee, bool relay, bool wait_callback, bool bypass_limits = false); +[[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, bilingual_str& err_string, const CAmount& highfee, bool relay, bool wait_callback, bool bypass_limits = false); /** * Return transaction with a given hash. diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index b6229531926ba6..c31ca27ef6e9ee 100644 --- a/src/qt/psbtoperationsdialog.cpp +++ b/src/qt/psbtoperationsdialog.cpp @@ -100,7 +100,7 @@ void PSBTOperationsDialog::broadcastTransaction() } CTransactionRef tx = MakeTransactionRef(mtx); - std::string err_string; + bilingual_str err_string; TransactionError error = BroadcastTransaction( *m_client_model->node().context(), tx, err_string, DEFAULT_MAX_RAW_TX_FEE_RATE.GetFeePerK(), /* relay */ true, /* await_callback */ false); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 6c7111ccd5da90..eb9ae5a40b9249 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -1164,12 +1165,12 @@ RPCHelpMan sendrawtransaction() bool bypass_limits = false; if (!request.params[3].isNull()) bypass_limits = request.params[3].get_bool(); - std::string err_string; + bilingual_str err_string; AssertLockNotHeld(cs_main); NodeContext& node = EnsureAnyNodeContext(request.context); const TransactionError err = BroadcastTransaction(node, tx, err_string, max_raw_tx_fee, /* relay */ true, /* wait_callback */ true, bypass_limits); if (TransactionError::OK != err) { - throw JSONRPCTransactionError(err, err_string); + throw JSONRPCTransactionError(err, err_string.original); } return tx->GetHash().GetHex(); diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 11df0c274ef665..6453c14622f74e 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -18,6 +18,7 @@ #include #include #include +#include CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime) { @@ -227,22 +228,22 @@ void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, int nHashType = ParseSighashString(hashType); // Script verification errors - std::map input_errors; + std::map input_errors; bool complete = SignTransaction(mtx, keystore, coins, nHashType, input_errors); SignTransactionResultToJSON(mtx, complete, coins, input_errors, result); } -void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, UniValue& result) +void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, UniValue& result) { // Make errors UniValue UniValue vErrors(UniValue::VARR); for (const auto& err_pair : input_errors) { - if (err_pair.second == "Missing amount") { + if (err_pair.second.original == "Missing amount") { // This particular error needs to be an exception for some reason throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Missing amount for %s", coins.at(mtx.vin.at(err_pair.first).prevout).out.ToString())); } - TxInErrorToJSON(mtx.vin.at(err_pair.first), vErrors, err_pair.second); + TxInErrorToJSON(mtx.vin.at(err_pair.first), vErrors, err_pair.second.original); } result.pushKV("hex", EncodeHexTx(CTransaction(mtx))); diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index c8865588bc07e3..6ee04e77e1b941 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -8,6 +8,7 @@ #include #include +struct bilingual_str; class FillableSigningProvider; class UniValue; struct CMutableTransaction; @@ -25,7 +26,7 @@ class SigningProvider; * @param result JSON object where signed transaction results accumulate */ void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map& coins, const UniValue& hashType, UniValue& result); -void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, UniValue& result); +void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, UniValue& result); /** * Parse a prevtxs UniValue array and get the map of coins from it diff --git a/src/script/sign.cpp b/src/script/sign.cpp index bb120d784c29c7..c9491f65f57d93 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -11,6 +11,7 @@ #include