Here is the CScriptNum
extension prototype (PR against master):
Highlights
- Re-enables the
OP_MUL
andOP_DIV
opcodes - Support 8 byte computation w/ arithmetic and comparison op codes.
- Doesn’t add any new opcodes, rather repurposes existing opcodes based on
SigVersion
. - Changes the underlying impl type in
CScriptNum
fromint64_t
→__int128_t
- Preserves behavior of the old
CScriptNum
(variable length encoding, allows overflow results, but no computation on overflowed results).
For those that may not be familiar with the existing behavior of CScriptNum
, I would suggest reading this comment in script.h
/**
* Numeric opcodes (OP_1ADD, etc) are restricted to operating on 4-byte integers.
* The semantics are subtle, though: operands must be in the range [-2^31 +1...2^31 -1],
* but results may overflow (and are valid as long as they are not used in a subsequent
* numeric operation). CScriptNum enforces those semantics by storing results as
* an int64 and allowing out-of-range values to be returned as a vector of bytes but
* throwing an exception if arithmetic is done or the result is interpreted as an integer.
*/
The intention of this branch is to retain this behavior, only extending the supported range of values to [-2^63 +1...2^63 -1]
.
Consensus risk
This PR changes the behavior of CScriptNum
. For instance, the constructor for CScriptNum
now takes a __int128_t
as a parameter rather than int64_t
. This constructor is called for all SigVersion
(not just SigVersion::TAPSCRIPT_64BIT
). This seems like it could lead to some consensus risk with old nodes if someone crafts a specific transaction using segwit v0 or tapscript that exceeds std::numeric_limits<int64_t>::max()
but is less than std::numeric_limits<__int128_t>::max()
.
More problems with __int128_t
I chose to use __in128_t
as it seemed like the logical thing to do to extend the existing behavior for support for overflow values.
I’m not an expert on c++, but it appears that __int128_t
is not supported properly on windows. I’m seeing CI failures on windows that look like this
D:\a\bitcoin\bitcoin\src\script\script.h(229,25): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [D:\a\bitcoin\bitcoin\build_msvc\libbitcoin_qt\libbitcoin_qt.vcxproj]
D:\a\bitcoin\bitcoin\src\script\script.h(229,41): error C2143: syntax error: missing ',' before '&' [D:\a\bitcoin\bitcoin\build_msvc\libbitcoin_qt\libbitcoin_qt.vcxproj]
D:\a\bitcoin\bitcoin\src\script\script.h(290,33): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [D:
Perhaps there is a better way to implement this while avoiding using __int128_t
. I’m open to ideas.