Page MenuHomePhabricator
Differential D3862

qa: Read reject reasons from debug log, not p2p messages
ClosedPublic
Actions

Authored by jasonbcox on Aug 12 2019, 23:08.

Details

Reviewers
deadalnix
Fabien
Group Reviewers
Restricted Project
Commits
rSTAGINGea63c14f0d04: qa: Read reject reasons from debug log, not p2p messages
rABCea63c14f0d04: qa: Read reject reasons from debug log, not p2p messages
Summary

Partial backport of Core PR14119
https://github.com/bitcoin/bitcoin/pull/14119/files

Rationale for partially completing this out-of-order backport:

  • We get the benefits of this change today with minimal negative impact on future backporting efforts.
  • The original PR is a bit large and could do with splitting up the review. Only two additional lines of code are necessary to make this backport digestible in chunks (and then all TODOs can be removed once completed).
  • Improves robustness for D3863

Depends on D3860, D3861

Test Plan
test_runner.py --extended

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jasonbcox created this revision.Aug 12 2019, 23:08
Herald added a reviewer: deadalnix. · View Herald TranscriptAug 12 2019, 23:08
teamcity added a comment.Aug 12 2019, 23:09

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Aug 12 2019, 23:15

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7128: Diff 10747.Aug 12 2019, 23:15
jasonbcox added a child revision: D3863: Migrate abc-invalid-chains off of the ComparisonTestFramework.Aug 12 2019, 23:18
jasonbcox edited the summary of this revision. (Show Details)Aug 12 2019, 23:19
Fabien requested changes to this revision.Aug 13 2019, 07:38
Fabien added a subscriber: Fabien.
Fabien added inline comments.
test/functional/abc-schnorr.py
135 ↗(On Diff #10747)

Nit: move named argument for consistency

test/functional/p2p_invalid_tx.py
168 ↗(On Diff #10747)

I'm not sure why this is removed in the PR, I guess this is related to core disabling bip61 per default.
Does this test really need to be removed for us ?

test/functional/test_framework/mininode.py
620 ↗(On Diff #10747)

received => 'logged'

657 ↗(On Diff #10747)

Dito

This revision now requires changes to proceed.Aug 13 2019, 07:38
jasonbcox added inline comments.Aug 13 2019, 22:05
test/functional/p2p_invalid_tx.py
168 ↗(On Diff #10747)

Talked about this offline. We agree that removing it while BIP61 is on by default does not make sense. I'll be leaving it in.

jasonbcox updated this revision to Diff 10772.Aug 13 2019, 22:06

Addressed feedback

teamcity added a comment.Aug 13 2019, 22:06

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Aug 13 2019, 22:13

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7136: Diff 10772.Aug 13 2019, 22:13
jasonbcox updated this revision to Diff 10776.Aug 14 2019, 01:37

Fix in feature_block that was not passing (feature_block is part of extended tests)

jasonbcox edited the test plan for this revision. (Show Details)Aug 14 2019, 01:38
teamcity added a comment.Aug 14 2019, 01:38

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Aug 14 2019, 01:44

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7138: Diff 10776.Aug 14 2019, 01:44
Fabien accepted this revision.Aug 14 2019, 08:15
This revision is now accepted and ready to land.Aug 14 2019, 08:15
Closed by commit rABCea63c14f0d04: qa: Read reject reasons from debug log, not p2p messages (authored by jasonbcox). · Explain WhyAug 14 2019, 16:39
This revision was automatically updated to reflect the committed changes.
jasonbcox added a commit: rABCea63c14f0d04: qa: Read reject reasons from debug log, not p2p messages.
jasonbcox added a child revision: D3873: Cleanup reject_code in feature_block.py.Aug 14 2019, 16:52
jasonbcox added a child revision: D3875: Read more reject messages from debug logs in feature_dersig.py.Aug 14 2019, 18:23
jasonbcox added a child revision: D3878: Read more reject messages from debug logs in feature_cltv.py.Aug 14 2019, 19:09
jasonbcox added a child revision: D3895: Cleanup reject_code in abc-schnorrmultisig-activation.Aug 16 2019, 23:21
jasonbcox added a child revision: D3896: Cleanup reject_code in abc-minimaldata-activation.Aug 16 2019, 23:26
jasonbcox added a commit: rSTAGINGea63c14f0d04: qa: Read reject reasons from debug log, not p2p messages.Sep 19 2019, 01:49

Revision Contents
Changeset List

Diff 10785

View Options

test/functional/abc-minimaldata-activation.py

Loading...
View Options

test/functional/abc-schnorr.py

Loading...
View Options

test/functional/abc-schnorrmultisig-activation.py

Loading...
View Options

test/functional/abc-segwit-recovery.py

Loading...
View Options

test/functional/feature_block.py

Loading...
View Options

test/functional/p2p_invalid_tx.py

Loading...
View Options

test/functional/test_framework/mininode.py

Loading...