Page MenuHomePhabricator

[tests] Fix checkinputs_test to work under magnetic anomaly
ClosedPublic

Authored by schancel on Mon, Nov 19, 01:34.

Details

Summary

As per title. Add CTOR ordering to test block construction. Use NULLDUMMY
instead of CLEANSTACK for checking consensus vs non-consensus script cache behavior.

Test Plan
make -j10 VERBOSE=1 check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

schancel updated this revision to Diff 5908.Mon, Nov 19, 01:34
schancel created this revision.

Re-remove useless header

schancel updated this revision to Diff 5909.Mon, Nov 19, 01:36

Fix legacy comment

schancel edited the summary of this revision. (Show Details)Mon, Nov 19, 01:38
deadalnix requested changes to this revision.Mon, Nov 19, 01:40
deadalnix added a subscriber: deadalnix.

I'm sure I missed some of the feedback provided on D2089 .

src/test/txvalidationcache_tests.cpp
22 ↗(On Diff #5907)

Remove

182 ↗(On Diff #5907)

Seems like a temporary would be beneficial here

186 ↗(On Diff #5907)

Why is that called p2sh if it is not p2sh ?

194 ↗(On Diff #5907)

The whole block should be scoped

196 ↗(On Diff #5907)

Fix comment

This revision now requires changes to proceed.Mon, Nov 19, 01:40
jasonbcox added inline comments.
src/test/txvalidationcache_tests.cpp
249 ↗(On Diff #5907)

DERSIG -> NULLDUMMY

jasonbcox requested changes to this revision.Mon, Nov 19, 01:45
schancel updated this revision to Diff 5910.Mon, Nov 19, 01:49

Fix feedback

jasonbcox requested changes to this revision.Mon, Nov 19, 01:56
jasonbcox added inline comments.
src/test/txvalidationcache_tests.cpp
187 ↗(On Diff #5910)

nulldummySigHas -> nulldummySigHash (missing an 'h')

196 ↗(On Diff #5910)

Comment should be // Mine the funding transaction into a block

This revision now requires changes to proceed.Mon, Nov 19, 01:56
jasonbcox added inline comments.Mon, Nov 19, 01:57
src/test/txvalidationcache_tests.cpp
170 ↗(On Diff #5910)

double space after Create

schancel updated this revision to Diff 5911.Mon, Nov 19, 01:58

Fix comments

jasonbcox added inline comments.Mon, Nov 19, 01:58
src/test/txvalidationcache_tests.cpp
180 ↗(On Diff #5910)

Scope should start before this line imo. There's no reason to have the mutableFunding_tx stuff inside this scope, right?

deadalnix added inline comments.Mon, Nov 19, 02:00
src/test/txvalidationcache_tests.cpp
180 ↗(On Diff #5910)

It doesn't change anything. But it groups things logically. Ehter way it is fine.

jasonbcox accepted this revision.Mon, Nov 19, 02:01
deadalnix accepted this revision.Mon, Nov 19, 02:01
This revision is now accepted and ready to land.Mon, Nov 19, 02:01
This revision was automatically updated to reflect the committed changes.