Chore: remove a redundant argument to maintain single source of truth #5530
Conversation
jannotti
changed the title
Codecov Report
@@ Coverage Diff @@
## master #5530 +/- ##
==========================================
+ Coverage 55.78% 55.87% +0.08%
==========================================
Files 446 446
Lines 63262 63263 +1
==========================================
+ Hits 35293 35350 +57
+ Misses 25590 25547 -43
+ Partials 2379 2366 -13
... and 17 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| if groupCtx.consensusParams.LogicSigVersion == 0 { | ||
| return errors.New("LogicSig not enabled") | ||
| } | ||
|
|
||
| if gi < 0 { |
There was a problem hiding this comment.
Why do you care to check for negative values, and not for values greater than the group size?
There was a problem hiding this comment.
I just retained the existing check (pulled up from lines 356-358, because I needed to use the index earlier).
I think maybe the negative check is a bit too much paranoia, but didn't want to make a decision like that in this PR. In all the cases I know of, these gi indexes come from a range operator on the transactions, so I probably would not check less than zero or greater than length.
This eliminates an initial *SIgnedTransaction argument that is redundant with the later combination of group index and groupCtx argument. That way, callers can't mess it up.