net_processing: always disconnect for SENDTXRCNCL after VERACK #36752

1. 
   vasild commented at 9:00 AM on November 4, 2026: contributor

   As mandated by BIP 330:

   If "sendtxrcncl" was sent after "verack", the sender should be disconnected.

   So, always disconnect such a peer, regardless of the value of -txreconciliation. Previously the code would ignore and not disconnect such a peer if -txreconciliation=0.

2. 
   DrahtBot commented at 9:00 AM on November 4, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--006a51241073e994b41acfe9ec718e94-->

   External sites

   • Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36752
   • Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36752.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36752#author-ralph

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   ACK	maflcko, w0xlt

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #35502 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35502.svg">} (refactor: extract per-message helpers from ProcessMessage (move-only) by w0xlt)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

3. in src/net_processing.cpp:4272 in a1b6a7b450

   4274 |              pfrom.fDisconnect = true;
   4275 |              return;
   4276 |          }
   4277 |  
   4278 | +        if (!m_txreconciliation) {
   4279 | +            LogDebug(BCLog::NET, "sendtxrcncl from peer=%d ignored, as our node does not have txreconciliation enabled\n", pfrom.GetId());
   

   ---

   ---

   

   maflcko commented at 9:10 AM on November 4, 2026:

               LogDebug(BCLog::NET, "sendtxrcncl from peer=%d ignored, as our node does not have txreconciliation enabled", pfrom.GetId());
   

   nit: Remove to avoid having to touch this line yet again?

   ---

   

   vasild commented at 9:29 AM on November 4, 2026:

   Done, thanks!

4. maflcko approved
5. 
   maflcko commented at 9:12 AM on November 4, 2026: member

   The bip is in draft for ~7 years and this is basically dead code, but it lgtm to be more consistent here

6. net_processing: always disconnect for SENDTXRCNCL after VERACK

   As mandated by BIP 330:
   > If "sendtxrcncl" was sent after "verack", the sender should be disconnected.
   
   So, always disconnect such a peer, regardless of the value of
   `-txreconciliation`. Previously the code would ignore and not disconnect
   such a peer if `-txreconciliation=0`.

   87310779af
7. vasild force-pushed on Nov 4, 2026
8. 
   vasild commented at 9:29 AM on November 4, 2026: contributor

   a1b6a7b4502218368556e87029be26e6d7212380...87310779af88aec0233c704350df875284d40f34: do #36752 (review)

9. DrahtBot added the label CI failed on Nov 4, 2026
10. 
    maflcko commented at 9:39 AM on November 4, 2026: member

    The code is correct before and after this change, but it seems fine to change for negotiation consistency.

    I'd say one could even argue that BIP 330 should be changed to adopt the new feature msg type for even more consistency?

    review ACK 87310779af88aec0233c704350df875284d40f34 👈

    <details><summary>Show signature</summary>

    Signature:

    untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    trusted comment: review ACK 87310779af88aec0233c704350df875284d40f34 👈
    15Zc//6VLV5erocf79RGwDZHEp7b2+uxFPV097FvHSe98Sv2eNDduucvp5hYyI2IfZVHsz6qepSwdduRAQ5dDA==
    

    </details>

11. DrahtBot removed the label CI failed on Nov 4, 2026
12. 
    w0xlt commented at 12:53 AM on November 6, 2026: contributor

    ACK 87310779af88aec0233c704350df875284d40f34

13. 
    sedited commented at 10:15 AM on November 6, 2026: contributor

    Seems fine to me, but would also prefer feature negotiation to actually be done consistently in the future.

14. sedited merged this on Nov 6, 2026
15. sedited closed this on Nov 6, 2026

    ---

16. fanquake added this to the milestone 33.0 on Nov 6, 2026
17. vasild deleted the branch on Nov 9, 2026

Contributors

vasildDrahtBotmaflckow0xltsedited

Milestone
33.0