Skip to content

Don't punish peers for consensus-invalid txs #1513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Nov 26, 2025
Merged

Don't punish peers for consensus-invalid txs #1513

merged 2 commits into from
Nov 26, 2025
+59 −125

Conversation

@tomt1664
Copy link
Member

@tomt1664 tomt1664 commented Nov 21, 2025

Backport of bitcoin/bitcoin#33050

@tomt1664 tomt1664 requested a review from delta1 November 21, 2025 15:41
@delta1
Copy link
Member

delta1 commented Nov 26, 2025

After merging #1512, I'm seeing a test failure in p2p_segwit.py

File "/home/byron/code/elements-worktree/test/functional/test_framework/test_node.py", line 181, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['9d435db05b98f86cf89f6fd5328e4526036afaf0ce04994e58fb895e59bc68a5', 'was not accepted: non-mandatory-script-verify-flag (Witness program was passed an empty witness)']" does not partially match log:

 - 2025-11-26T09:41:33.640780Z [msghand] [net_processing.cpp:2588] [ProcessMessage] received: tx (176 bytes) peer=1
 - 2025-11-26T09:41:33.641222Z [msghand] [txmempool.cpp:805] [check] Checking mempool with 0 transactions and 0 inputs
 - 2025-11-26T09:41:33.641233Z [msghand] [net_processing.cpp:3522] [ProcessMessage] 9d435db05b98f86cf89f6fd5328e4526036afaf0ce04994e58fb895e59bc68a5 from peer=1 was not accepted: mempool-script-verify-flag-failed (Witness program was passed an empty witness)

@tomt1664
Copy link
Member Author

tomt1664 commented Nov 26, 2025

After merging #1512, I'm seeing a test failure in p2p_segwit.py

File "/home/byron/code/elements-worktree/test/functional/test_framework/test_node.py", line 181, in _raise_assertion_error
    raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['9d435db05b98f86cf89f6fd5328e4526036afaf0ce04994e58fb895e59bc68a5', 'was not accepted: non-mandatory-script-verify-flag (Witness program was passed an empty witness)']" does not partially match log:

 - 2025-11-26T09:41:33.640780Z [msghand] [net_processing.cpp:2588] [ProcessMessage] received: tx (176 bytes) peer=1
 - 2025-11-26T09:41:33.641222Z [msghand] [txmempool.cpp:805] [check] Checking mempool with 0 transactions and 0 inputs
 - 2025-11-26T09:41:33.641233Z [msghand] [net_processing.cpp:3522] [ProcessMessage] 9d435db05b98f86cf89f6fd5328e4526036afaf0ce04994e58fb895e59bc68a5 from peer=1 was not accepted: mempool-script-verify-flag-failed (Witness program was passed an empty witness)

#1512 changed the script-verify-flag message. Will push update.

achow101 and others added 2 commits November 26, 2025 10:22
@achow101
Merge bitcoin/bitcoin#33050: net, validation: don't punish peers for …
4b30166 
…consensus-invalid txs

876dbdfb4702410dfd4037614dc9298a0c09c63e tests: drop expect_disconnect behaviour for tx relay (Anthony Towns)
b29ae9efdfeeff774e32ee433ce67d8ed8ecd49f validation: only check input scripts once (Anthony Towns)
266dd0e10d08c0bfde63205db15d6c210a021b90 net_processing: drop MaybePunishNodeForTx (Anthony Towns)

Pull request description:

  Because we do not discourage nodes for transactions we consider non-standard, we don't get any DoS protection from this check in adversarial scenarios, so remove the check entirely both to simplify the code and reduce the risk of splitting the network due to changes in tx relay policy.

  Then, because we no longer make use of the distinction between consensus and standardness failures during script validation, don't re-validate each script with only-consensus rules, reducing the cost to us of transactions that we won't relay.

ACKs for top commit:
  achow101:
    ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e
  darosior:
    re-ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e
  sipa:
    re-ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e
  glozow:
    ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e

Tree-SHA512: 8bb0395766dde54fc48f7077b80b88e35581aa6e3054d6d65735965147abefffa7348f0850bb3d46f6c2541fd384ecd40a00a57fa653adabff8a35582e2d1811
updated script-verify-flag failure messagesin p2p_segwit.py
05bd984
delta1
delta1 approved these changes Nov 26, 2025
View reviewed changes
Copy link
Member

@delta1 delta1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 05bd984

@delta1 delta1 merged commit 7820602 into ElementsProject:elements-23.x Nov 26, 2025
5 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@delta1 delta1 delta1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tomt1664 @delta1 @achow101