Page MenuHomePhabricator

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

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

Details

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Mon, Aug 12, 23:08
jasonbcox edited the summary of this revision. (Show Details)Mon, Aug 12, 23:19
Fabien requested changes to this revision.Tue, Aug 13, 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.Tue, Aug 13, 07:38
jasonbcox added inline comments.Tue, Aug 13, 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.Tue, Aug 13, 22:06

Addressed feedback

jasonbcox updated this revision to Diff 10776.Wed, Aug 14, 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)Wed, Aug 14, 01:38
Fabien accepted this revision.Wed, Aug 14, 08:15
This revision is now accepted and ready to land.Wed, Aug 14, 08:15