[64-bit ints] Refactor `CScriptNum` to require setting `nMaxNumSize`
Needs RevisionPublic
Actions

Authored by tobias_ruck on Oct 25 2024, 00:21.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Summary

Currently, CScriptNum's constructor and IsMinimallyEncoded have a default for nMaxNumSize, which defaults to 4 (eCash's maximum integer size).

For upgrading Script integers from 32-bit to 64-bit, having to make the integer size explicit allows us to upgrade them based on a script flag.

This has no change in behavior.

Test Plan

ninja check

Repository

rABC Bitcoin ABC

Branch

interpreter-refactor-script-num-size

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 30835
Build 61183: Build Diff	build-without-wallet · lint-circular-dependencies · build-clang-tidy · build-clang · build-debug · build-diff
Build 61182: arc lint + arc unit

rebase, also do the same change with ScriptNum10

rebase, add const

tobias_ruck published this revision for review.Fri, Dec 6, 10:27

Fabien requested changes to this revision.Fri, Dec 6, 13:43

Fabien added inline comments.

src/script/script.h
227 ↗	(On Diff #51430)	This should probably no longer be a CScriptNum member. Why would you pass a static member to the constructor ? When going from 4 to 8 this variable will need to be moved out anyway.

This revision now requires changes to proceed.Fri, Dec 6, 13:43

src/script/script.h
227 ↗	(On Diff #51430)	well we can just reference CScriptNum::MAXIMUM_ELEMENT_SIZE, no? but happy to move it, keep it inside script.h ? keep in mind that there will be two variables during transition, MAXIMUM_ELEMENT_SIZE_32_BIT and MAXIMUM_ELEMENT_SIZE_64_BIT