highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 14:38:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge #14906: refactor: Make explicit CMutableTransaction -> CTransaction conversion.

Browse Source 
b301950df3  Made expicit constructor CTransaction(const CMutableTransaction &tx). (lucash-dev)
faf29dd019  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
This commit is contained in:
Wladimir J. van der Laan
2019-01-21 20:12:19 +01:00
parent 6e6b3b944d b301950df3
commit f0c9e1c22b
7 changed files with 11 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/rpc/rawtransaction.cpp
View File

@@ -445,7 +445,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal
}
}
if (!rbf.isNull() && rawTx.vin.size() > 0 && rbfOptIn != SignalsOptInRBF(rawTx)) {
if (!rbf.isNull() && rawTx.vin.size() > 0 && rbfOptIn != SignalsOptInRBF(CTransaction(rawTx))) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter combination: Sequence number(s) contradict replaceable option");
}
@@ -517,7 +517,7 @@ static UniValue createrawtransaction(const JSONRPCRequest& request)
CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]);
return EncodeHexTx(rawTx);
return EncodeHexTx(CTransaction(rawTx));
}
static UniValue decoderawtransaction(const JSONRPCRequest& request)
@@ -773,7 +773,7 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request)
UpdateInput(txin, sigdata);
}
return EncodeHexTx(mergedTx);
return EncodeHexTx(CTransaction(mergedTx));
}
UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, const UniValue& prevTxsUnival, CBasicKeyStore *keystore, bool is_temp_keystore, const UniValue& hashType)
@@ -906,7 +906,7 @@ UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, con
bool fComplete = vErrors.empty();
UniValue result(UniValue::VOBJ);
result.pushKV("hex", EncodeHexTx(mtx));
result.pushKV("hex", EncodeHexTx(CTransaction(mtx)));
result.pushKV("complete", fComplete);
if (!vErrors.empty()) {
result.pushKV("errors", vErrors);