Page MenuHomePhabricator

Make CTransaction construction from CMutableTransaction explicit

Authored by matra774 on Feb 16 2018, 02:51.



Most of the changes are in the test code. The tests first build CMutableTransaction and then used t implicit conversion to call functions that require CTransaction.

I decided to use CTransaction(x) for conversion. Let me know if you prefer static_cast<CTransaction>.

In many cases, I was tempted to create an overload that takes a CTransaction (for example for IsStandardTx(), that is heavily used in transaction_tests.cpp), but later decided to use explicit conversion, otherwise the result will be very similar to using implicit cast operator, that I was trying to get rid of. An alternative solution would be to create a IsStandardTxFromMutable(), that would be local to test code.

I had quite some fun with test combineSigs, because I’ve overlooked the fact that it modifies transaction by directly manipulating references to CMutableTransaction’s members.

Refs T238

Test Plan

make check

Diff Detail

rABC Bitcoin ABC
Lint Not Applicable
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Feb 16 2018, 02:51

Accepted, but please fix the nits.

1251 ↗(On Diff #2887)

TranactionSignatureChecker -> TransactionSignatureChecker

1372 ↗(On Diff #2887)


331 ↗(On Diff #2887)

This comment is unnecessary. Please remove.

This revision is now accepted and ready to land.Feb 16 2018, 05:57

Addressed review comments

Rebasing on master (arc land failed)

Rebasing again after doing git fetch origin master

This revision was automatically updated to reflect the committed changes.