feat(sighash): implement sighash range [part 3/7]#913
feat(sighash): implement sighash range [part 3/7]#913glevco wants to merge 1 commit intotests/sighash/bitmaskfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## tests/sighash/bitmask #913 +/- ##
========================================================
Coverage ? 85.43%
========================================================
Files ? 289
Lines ? 22584
Branches ? 3410
========================================================
Hits ? 19295
Misses ? 2614
Partials ? 675 ☔ View full report in Codecov by Sentry. |
glevco
force-pushed
the
tests/sighash/bitmask
branch
from
78b027b to
1b44d48
Compare
glevco
force-pushed
the
feat/sighash/range
branch
from
c3467a9 to
479cb8a
Compare
glevco
force-pushed
the
tests/sighash/bitmask
branch
from
1b44d48 to
fc82733
Compare
glevco
force-pushed
the
feat/sighash/range
branch
from
479cb8a to
b815b65
Compare
glevco
force-pushed
the
tests/sighash/bitmask
branch
from
2bf7549 to
ad1def9
Compare
glevco
force-pushed
the
feat/sighash/range
branch
from
b815b65 to
e2a9b75
Compare
hathor/transaction/scripts/opcode.py
Outdated
| output_start=bytes_to_int(output_start), | ||
| output_end=bytes_to_int(output_end), | ||
| ) | ||
| except Exception as e: |
There was a problem hiding this comment.
Why are you capturing Exception instead of the specific exception types?
There was a problem hiding this comment.
I changed it as explained in #911 (comment)
|
|
||
| SighashType: TypeAlias = SighashAll | SighashBitmask | ||
| class SighashRange(CustomSighash): | ||
| """A model representing the sighash range type config. Range ends are not inclusive.""" |
There was a problem hiding this comment.
Shouldn't range ends be included? I feel it's more intuitive than not including them.
There was a problem hiding this comment.
I think it's intuitive like this when you include the whole range, which would be from 0 to 255, which is the max number of inputs/outputs. If we do it inclusive, the user would have to set 254 to include the whole range. I can change it if you want.
glevco
force-pushed
the
tests/sighash/bitmask
branch
from
ad1def9 to
890c630
Compare
glevco
force-pushed
the
feat/sighash/range
branch
from
e2a9b75 to
d7aa643
Compare
glevco
force-pushed
the
feat/sighash/range
branch
from
d7aa643 to
887a8a7
Compare
2 participants
Depends on #912
Motivation
Implement the new sighash range opcode.
Acceptance Criteria
SighashRangemodel.op_sighash_range()function.