Page MenuHomePhabricator

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

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



Partial backport of Core PR14119

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 --extended

Diff Detail

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

Event Timeline

jasonbcox created this revision.Aug 12 2019, 23:08
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.
135 ↗(On Diff #10747)

Nit: move named argument for consistency

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 ?

620 ↗(On Diff #10747)

received => 'logged'

657 ↗(On Diff #10747)


This revision now requires changes to proceed.Aug 13 2019, 07:38
jasonbcox added inline comments.Aug 13 2019, 22:05
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

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
Fabien accepted this revision.Aug 14 2019, 08:15
This revision is now accepted and ready to land.Aug 14 2019, 08:15