script: remove unused bitwise CScriptNum operators #27096

1. 
   theStack commented at 10:53 am on February 14, 2023: contributor

   These overloaded operators were introduced almost seven years ago in the implementation of OP_CHECKSEQUENCEVERIFY (BIP112, PR #7524, commit 53e53a33c939949665f60d5eeb82abbb21f97128), but have never been used since outside of fuzzing. One could argue to keep those around solely for consistency reasons, but I think its preferable to get rid of methods that are likely never used. Even the so-far only use of the & operator (checking the OP_CSV argument for the SEQUENCE_LOCKTIME_DISABLE_FLAG), could be removed by simply doing the check on the returned GetInt64(), but fiddling in the consensus code is probably not worth it.
2. 
   DrahtBot commented at 10:53 am on February 14, 2023: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	w0xlt

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. DrahtBot added the label Consensus on Feb 14, 2023
4. script: remove unused bitwise `CScriptNum` operators

   These overloaded operators were introduced almost seven years ago in the
   implementation of OP_CHECKSEQUENCEVERIFY (BIP112, PR #7524, commit
   53e53a33c939949665f60d5eeb82abbb21f97128), but never used since outside
   of fuzzing.

   e6e51178d6
5. theStack force-pushed on Feb 14, 2023
6. w0xlt approved
7. 
   w0xlt commented at 7:01 pm on February 14, 2023: contributor

   utACK https://github.com/bitcoin/bitcoin/pull/27096/commits/e6e51178d6adb3edcc5049e2aadc511c1ca2f644
8. fanquake requested review from instagibbs on Mar 16, 2023
9. achow101 requested review from maflcko on Apr 25, 2023
10. 
    maflcko commented at 2:17 pm on April 26, 2023: member

    Idk, this is probably too hard to review to be worth it. You are removing an overload, so reviewers need to check how this interacts with overload resolution. Maybe just close and reopen/mention it the next time the code is touched an reviewers are primed on it?
11. 
    theStack commented at 8:57 am on April 29, 2023: contributor

    Idk, this is probably too hard to review to be worth it. You are removing an overload, so reviewers need to check how this interacts with overload resolution. Maybe just close and reopen/mention it the next time the code is touched an reviewers are primed on it?

    Agree that reviewing this is more cumbersome than it should be (this is just another example of how overloading leads to obfuscation and makes maintaining code harder). Closing for now.

12. theStack closed this on Apr 29, 2023

    ---

13. theStack deleted the branch on Apr 29, 2023
14. bitcoin locked this on Apr 28, 2024

Contributors
theStack DrahtBot w0xlt maflcko

Review Requested
instagibbs maflcko

Labels
Consensus