Directly operate with CMutableTransaction #13359
Conversation
|
How about using something like Example use here: https://github.com/bitcoin/bitcoin/pull/13076/files#diff-4a0cb5ad5e93d187a1b065a99bbae143 |
|
Could remove the following in a separate commit? int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts) |
|
Is this mining bitcoin |
|
@pawan17kumar yes we are hard at work swinging pick-axes into keyboards. |
|
utACK faadf51
if only mining was proof-of-open-source-development... |
faadf51 to
fa7ae60
Compare
fa7ae60 to
fa1b4e9
Compare
fa1b4e9 to
fa25e2e
Compare
Note to reviewers: This pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
| { | ||
| static_assert(std::is_same<T, CTransaction>::value || std::is_same<T, CMutableTransaction>::value, "no need to allow other types"); |
There was a problem hiding this comment.
it might be nicer to have this function not participate in overload resolution (i.e. enable_if) when these conditions are not met, rather than being triggering a static assert.
There was a problem hiding this comment.
Downside of using enable_if is that the resulting compiler error message will probably be more ambiguous and harder to understand.
There was a problem hiding this comment.
Either way, I think there should be a standard logic for checking whether a type is a transaction type -- provided by transaction.h, thus preserving encapsulation.
| @@ -4,15 +4,18 @@ | |||
|
|
|||
| #include <policy/rbf.h> | |||
|
|
|||
| bool SignalsOptInRBF(const CTransaction &tx) | |||
| template <typename T> | |||
| bool SignalsOptInRBF(const T& tx) | |||
There was a problem hiding this comment.
this template should probably constrain T as well
| Needs rebase |
| { | ||
| static_assert(std::is_same<T, CTransaction>::value || std::is_same<T, CMutableTransaction>::value, "no need to allow other types"); |
There was a problem hiding this comment.
Downside of using enable_if is that the resulting compiler error message will probably be more ambiguous and harder to understand.
|
Why was this closed? |
It needs benchmarks, to justify the use of C++11, which is less common in this project (enable_if or static_assert) |
When serializing a transaction for the wallet or rpc, it shouldn't matter if the type is
CTransactionorCMutableTransaction. (See the code forSerializeTransaction, which is a template onTxType)Thus, we can avoid a call to a
CTransactionconstructor by templating various helper methods similar to #13309 (Directly operate with CMutableTransaction in SignSignature)