-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Binary Maths #82
Conversation
Codecov Report
@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 83.94% 84.53% +0.58%
==========================================
Files 35 35
Lines 3370 3504 +134
==========================================
+ Hits 2829 2962 +133
- Misses 394 395 +1
Partials 147 147
Continue to review full report at Codecov.
|
// interpreting it as an integer, it provides the required behaviour. | ||
type scriptNumber struct { | ||
val *big.Int | ||
afterGenesis bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it only utxos which are after genesis that are treated this way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah every utxo, tho the ones before genesis are capped within the limits of pre-genesis
@@ -241,6 +241,7 @@ func (t *thread) CheckErrorCondition(finalScript bool) error { | |||
if finalScript { | |||
t.afterSuccess() | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my fav change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
Currently, arithmeitc that overflows
math.MaxInt64
and underflowsmath.MinInt64
just causes integer overflows within the script, meaning maths that should be compatible with large numbers just does not work on our interpreter.Also,
OP_NUM2BIN
andOP_BIN2NUM
are implemented incorrectly.To fix this, I've fixed the implementations of the two aforementioned opcodes, and changed the
scriptNum
data type to wrapbig.Int
, implementing the correct math functions and byte conversion both to and from consensus, to and from standard big endian.