Skip to content

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

Merged
delta1 merged 2 commits into
ElementsProject:elements-23.xfrom
tomt1664:nopunishtx
Nov 26, 2025
Merged

Don't punish peers for consensus-invalid txs#1513
delta1 merged 2 commits into
ElementsProject:elements-23.xfrom
tomt1664:nopunishtx

Conversation

@tomt1664

Copy link
Copy Markdown
Member

Backport of bitcoin/bitcoin#33050

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

delta1 commented Nov 26, 2025

Copy link
Copy Markdown
Member

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
Copy Markdown
Member Author

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
…consensus-invalid txs

876dbdf tests: drop expect_disconnect behaviour for tx relay (Anthony Towns)
b29ae9e validation: only check input scripts once (Anthony Towns)
266dd0e 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 876dbdf
  darosior:
    re-ACK 876dbdf
  sipa:
    re-ACK 876dbdf
  glozow:
    ACK 876dbdf

Tree-SHA512: 8bb0395766dde54fc48f7077b80b88e35581aa6e3054d6d65735965147abefffa7348f0850bb3d46f6c2541fd384ecd40a00a57fa653adabff8a35582e2d1811

@delta1 delta1 left a comment

Copy link
Copy Markdown
Member

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants