Page MenuHomePhabricator

Make CTransaction construction from CMutableTransaction explicit
ClosedPublic

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

Details

Summary

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
./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
T238_cast
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1884
Build 1928: Bitcoin ABC Buildbot (legacy)
Build 1927: arc lint + arc unit

Event Timeline

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

Accepted, but please fix the nits.

src/test/script_tests.cpp
1251 ↗(On Diff #2887)

TranactionSignatureChecker -> TransactionSignatureChecker

1372 ↗(On Diff #2887)

likestamp

src/test/txvalidationcache_tests.cpp
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.