Page MenuHomePhabricator

[qa] don't pad transactions during make_conform_to_ctor
ClosedPublic

Authored by markblundeberg on May 22 2019, 23:02.

Details

Summary

This is a bad idea, as if it takes effect then it will change txids and
thus can break chains of transactions. Also, it simply does not belong in
this function.

(Introduced in D2086)

Also, fixes test that relied on this behaviour.

Test Plan

test_runner.py --extended

Diff Detail

Repository
rABC Bitcoin ABC
Branch
nopad
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5949
Build 9958: Bitcoin ABC Buildbot (legacy)
Build 9957: arc lint + arc unit

Event Timeline

markblundeberg edited the summary of this revision. (Show Details)

fix broken test

Fabien requested changes to this revision.May 23 2019, 06:19
Fabien added a subscriber: Fabien.

test_runner.py does not run all the tests, there is a cutoff based on their expected duration. To run them all you need test_runner.py --extended.
For example, abc-p2p-compactblocks.py uses make_conform_to_ctor, but will not be tested without the --extended flag.
Did you also check these tests ? Anyway you should update the test plan.

This revision now requires changes to proceed.May 23 2019, 06:19
In D3094#73113, @Fabien wrote:

test_runner.py does not run all the tests, there is a cutoff based on their expected duration. To run them all you need test_runner.py --extended.
For example, abc-p2p-compactblocks.py uses make_conform_to_ctor, but will not be tested without the --extended flag.
Did you also check these tests ? Anyway you should update the test plan.

I did not know this!

markblundeberg edited the test plan for this revision. (Show Details)

Updated test plan; ran a couple times with --extended. Only observed (once) an apparently unrelated failure in mining_getblocktemplate_longpoll.py (some kind of threading error), that I could not even reproduce in individual testing.

This revision is now accepted and ready to land.May 24 2019, 06:48