refactor: Appropriate re-naming of MAX_OPCODE after tapscript #30953

1. 
   itornaza commented at 5:51 pm on September 23, 2024: contributor

   In src/script/script.h:216 we define MAX_OPCODE to denote the maximum opcode that can be used in script.

   However, after the addition of the OP_CHECKSIGADD with taproot in the opcode set, the MAX_OPCODE naming looks somehow misleading.

   Renaming the variable to MAX_SCRIPT_OPCODE clearly denotes that the variable is intended for and referring exclusively to script (and not tapscript).

2. refactor: Appropriate re-naming of MAX_OPCODE after tapscript 30e78d7c3d
3. 
   DrahtBot commented at 5:51 pm on September 23, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept NACK	darosior

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

4. DrahtBot added the label Refactoring on Sep 23, 2024
5. 
   darosior commented at 7:53 am on October 1, 2024: member

   Concept NACK. Not that this change makes things worse, but i don’t think it meets the bar for refactorings.
6. 
   itornaza commented at 9:04 am on October 1, 2024: contributor

   Concept NACK. Not that this change makes things worse, but i don’t think it meets the bar for refactorings.

   Thank you for the clarification @darosior, and I will make sure to aim higher in the future to match the standards. However, the name of this variable after the introduction of tapscript is misleading and especially to new contributors like myself that read through the code and try to understand it. With the assumption that is very difficult to differentiate tapscript opcodes from script opcodes in a more structured way, this renaming despite being humble as the refactoring bar is concerned, made sense to me.

7. 
   darosior commented at 12:32 pm on October 1, 2024: member

   I understand your motivation. I’m not making a value judgement on whether this change is good or bad. It is just my personal opinion that the limited resources of the project are better allocated to something else than reviewing a bikeshed-prone (inbefore someone suggests renaming the variable to MAX_OPCODE_BEFORE_WITNESS_V1 instead) refactoring of a complicated and consensus critical part of the software.
8. 
   itornaza commented at 9:59 am on October 2, 2024: contributor

   @darosior, highly respecting your opinion and taking note of your suggestions, I am close this pr.
9. itornaza closed this on Oct 2, 2024

   ---

Contributors
itornaza DrahtBot darosior

Labels
Refactoring