RPC/Mining: Clean out pre-Segwit miner compatibility code #19391

1. 
   luke-jr commented at 9:24 pm on June 26, 2020: member

   This is dead code post-Segwit.
2. DrahtBot added the label Mining on Jun 26, 2020
3. DrahtBot added the label RPC/REST/ZMQ on Jun 26, 2020
4. 
   promag commented at 10:54 pm on July 26, 2020: member

   ACK
5. 
   MarcoFalke commented at 9:39 am on October 19, 2020: member

   Concept ACK on removing unused code
6. 
   DrahtBot commented at 11:08 am on March 10, 2021: contributor

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

   Conflicts

   No conflicts as of last run.

7. 
   jnewbery commented at 12:16 pm on March 10, 2021: member

   This is based on a 2018 base commit. Needs to be rebased to pick up all of the new tests/changes since then.
8. 
   jnewbery commented at 12:49 pm on March 10, 2021: member

   Code looks safe, but should be rebased on a recent base commit to ensure all new tests/checks run successfully.

   Should https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki#getblocktemplate-changes be updated to say that maxversion no longer has meaning?

9. luke-jr closed this on Mar 10, 2021

   ---

10. luke-jr reopened this on Mar 10, 2021

    ---

11. fanquake added the label Waiting for author on Apr 12, 2021
12. 
    fanquake commented at 9:02 am on April 12, 2021: member

    but should be rebased on a recent base commit to ensure all new tests/checks run successfully.

    Agree. This branch is currently based off (8a9ffec0a257da659ba54c5073bfc820986ae9c1).

    Should https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki#getblocktemplate-changes be updated to say that maxversion no longer has meaning?

    This also still needs a response.

    Added “Waiting for author”.

13. DrahtBot added the label Needs rebase on Apr 17, 2021
14. MarcoFalke removed the label Waiting for author on Apr 17, 2021
15. 
    luke-jr commented at 0:46 am on July 7, 2021: member

    Rebased due to conflicts, but note

    Code looks safe, but should be rebased on a recent base commit to ensure all new tests/checks run successfully.

    That’s not a reason to rebase. That’s a flaw in QA infra. It should be merging to master before running tests/checks.

16. luke-jr force-pushed on Jul 7, 2021
17. DrahtBot removed the label Needs rebase on Jul 7, 2021
18. 
    fanquake commented at 4:39 am on September 2, 2021: member

    Concept ACK

    That’s not a reason to rebase. That’s a flaw in QA infra.

    There’s a myriad of reasons to keep development branches rebased up and/or up to date.

    If I’m a dev wanting to just simply checkout and test this code, i.e git fetch origin pull/19391/head:19391 && git checkout 19391, not only should I not have to worry about merging it into master myself, but, having a commit here based on a branch from years ago means it’s going to bust up my caches when compiling, have missing tests, lack support for fuzzers & other tooling that’s been added in the interim, depends will be outdated, so if I’m building against that I’ll be testing against old / potentially broken dependencies. I also shouldn’t need to replicate the QA infra locally to test things fairly extensively.

19. 
    MarcoFalke commented at 6:49 am on September 2, 2021: member

    going to bust up my caches

    Personally I rebase/merge every change I test on master (even if it is just a single commit back) to rule out any unintended cache mishaps and at the same time check for silent merge conflict.

    If there is no reason to rebase, I think the approach @luke-jr is taking seems fine.

20. 
    glozow commented at 1:57 pm on March 8, 2022: member

    Concept ACK
21. fanquake added this to the milestone 24.0 on Mar 11, 2022
22. 
    fanquake commented at 2:51 pm on March 11, 2022: member

    Added to the 24.0 milestone.
23. DrahtBot added the label Needs rebase on May 19, 2022
24. 
    fanquake commented at 8:59 am on August 9, 2022: member

    @luke-jr do you want to rebase this, otherwise I can drop it back off the milestone.
25. RPC/Mining: Clean out pre-Segwit miner compatibility code 90a5dfa509
26. luke-jr force-pushed on Aug 9, 2022
27. 
    luke-jr commented at 10:16 pm on August 9, 2022: member

    #25153 should really have been reverted, but rebased anyway.
28. DrahtBot removed the label Needs rebase on Aug 9, 2022
29. 
    achow101 commented at 3:34 pm on August 15, 2022: member

    ACK 90a5dfa5098f142ba8b7b2f20eac31f4095ca583
30. fanquake merged this on Aug 17, 2022
31. fanquake closed this on Aug 17, 2022

    ---

32. sidhujag referenced this in commit 3c9d789e34 on Aug 17, 2022
33. bitcoin locked this on Aug 17, 2023

Contributors
luke-jr promag MarcoFalke DrahtBot jnewbery fanquake glozow achow101

Labels
RPC/REST/ZMQ Mining

Milestone
24.0