diff options
author | Wladimir J. van der Laan <laanwj@gmail.com> | 2019-01-21 20:12:19 +0100 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2019-01-21 20:28:56 +0100 |
commit | f0c9e1c22b8a043983f3ba90ad910b67cf981e36 (patch) | |
tree | e2a126ad18a535c9219cde63d7b6e443ede53805 /src/wallet | |
parent | 6e6b3b944d12a252a0fd9a1d68fec9843dd5b4f8 (diff) | |
parent | b301950df32443e358bc22ca22c6f9ac09d18219 (diff) | |
download | bitcoin-f0c9e1c22b8a043983f3ba90ad910b67cf981e36.tar.xz |
Merge #14906: refactor: Make explicit CMutableTransaction -> CTransaction conversion.
b301950df32443e358bc22ca22c6f9ac09d18219 Made expicit constructor CTransaction(const CMutableTransaction &tx). (lucash-dev)
faf29dd019efef4b05e8e78885926764134d9c04 Minimal changes to comply with explicit CMutableTransaction -> CTranaction conversion. (lucash-dev)
Pull request description:
This PR is re-submission of #14156, which was automatically closed by github (glitch?)
Original description:
This PR makes explicit the now implicit conversion constructor `CTransaction(const CMutableTransaction&)` in `transaction.h`.
Minimal changes were made elsewhere to make the code compilable. I'll follow up with other PRs to address individually refactoring functions that should have a `CMutableTransaction` version, or where a `CTransaction` should be reused.
The rationale for this change is:
- Conversion constructors should not be explicit unless there's a strong reason for it (in the opinion of, for example, https://google.github.io/styleguide/cppguide.html, and https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Ro-conversion. Let me know your take on this).
- This particular conversion is very costly -- it implies a serialization plus hash of the transaction.
- Even though `CTransaction` and `CMutableTransaction` represent the same data, they have very different use cases and performance properties.
- Making it explicit allows for easier reasoning of performance trade-offs.
- There has been previous performance issues caused by unneeded use of this implicit conversion.
- This PR creates a map for places to look for possible refactoring and performance gains (this benefit still holds if the PR is not merged).
Tree-SHA512: 2427462e7211b5ffc7299dae17339d27f8c43266e0895690fda49a83c72751bd2489d4471b3993075a18f3fef25d741243e5010b2f49aeef4a9688b30b6d0631
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/rpcwallet.cpp | 2 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 4 |
2 files changed, 3 insertions, 3 deletions
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index cb08112761..5e036eb5df 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3062,7 +3062,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) FundTransaction(pwallet, tx, fee, change_position, request.params[1]); UniValue result(UniValue::VOBJ); - result.pushKV("hex", EncodeHexTx(tx)); + result.pushKV("hex", EncodeHexTx(CTransaction(tx))); result.pushKV("fee", ValueFromAmount(fee)); result.pushKV("changepos", change_position); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 098055673b..74deb2dddc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1510,7 +1510,7 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall // implies that we can sign for every input. return -1; } - return GetVirtualTransactionSize(txNew); + return GetVirtualTransactionSize(CTransaction(txNew)); } int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, bool use_max_sig) @@ -2850,7 +2850,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std txNew.vin.push_back(CTxIn(coin.outpoint,CScript())); } - nBytes = CalculateMaximumSignedTxSize(txNew, this, coin_control.fAllowWatchOnly); + nBytes = CalculateMaximumSignedTxSize(CTransaction(txNew), this, coin_control.fAllowWatchOnly); if (nBytes < 0) { strFailReason = _("Signing transaction failed"); return false; |