[qa] don't pad transactions during make_conform_to_ctor
ClosedPublic
Actions

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

Details

Reviewers

jasonbcox
deadalnix
Fabien

Group Reviewers

Restricted Project

Commits

rSTAGINGa335fc174906: [qa] don't pad transactions during make_conform_to_ctor
rABCa335fc174906: [qa] don't pad transactions during make_conform_to_ctor

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 created this revision.May 22 2019, 23:02

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 22 2019, 23:02

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B5945: Diff 8813!May 22 2019, 23:15

markblundeberg planned changes to this revision.May 22 2019, 23:24

fix broken test

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B5949: Diff 8817.May 23 2019, 01:09

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

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!

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.

deadalnix accepted this revision.May 23 2019, 23:05

jasonbcox accepted this revision.May 24 2019, 00:49

Fabien accepted this revision.May 24 2019, 06:48

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

Closed by commit rABCa335fc174906: [qa] don't pad transactions during make_conform_to_ctor (authored by Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com>). · Explain WhyMay 24 2019, 16:47

This revision was automatically updated to reflect the committed changes.

Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rABCa335fc174906: [qa] don't pad transactions during make_conform_to_ctor.

Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rSTAGINGa335fc174906: [qa] don't pad transactions during make_conform_to_ctor.Sep 18 2019, 23:44