Small cleanups for clarity

I'm doing a larger bit of work to enable logicsig cost pooling and I
found myself making these changes along the way. It's easier to review
by extracting them into their own PR.  This does three things.

Introduces and uses:
```
func (msig MultisigSig) Signatures() int
```
for the sevral places we need to examine a MultiSig to count the
number of sub signatures present.

Uses the pre-existing `func (s *Signature) Blank() bool` in many
places.

Renames `EvalContext.runModeFlags` to simply `EvalContext.runMode`
because it takes on a single value, it can't be, for example: `ModeSig |
ModeApp`, it must be one or the other.

crypto/multisig.go

@@ -75,6 +75,18 @@ func (msig MultisigSig) Preimage() (version, threshold uint8, pks []PublicKey) {
 	return msig.Version, msig.Threshold, pks
 }
 
+// Signatures returns the actually number of signatures included in the
+// multisig. That is, the number of subsigs that are not blank.
+func (msig MultisigSig) Signatures() int {
+	sigs := 0
+	for i := range msig.Subsigs {
+		if !msig.Subsigs[i].Sig.Blank() {
+			sigs++
+		}
+	}
+	return sigs
+}
+
 const multiSigString = "MultisigAddr"
 const maxMultisig = 255
 
@@ -207,7 +219,7 @@ func MultisigAssemble(unisig []MultisigSig) (msig MultisigSig, err error) {
 	}
 	for i := 0; i < len(unisig); i++ {
 		for j := 0; j < len(unisig[0].Subsigs); j++ {
-			if (unisig[i].Subsigs[j].Sig != Signature{}) {
+			if !unisig[i].Subsigs[j].Sig.Blank() {
 				msig.Subsigs[j].Sig = unisig[i].Subsigs[j].Sig
 			}
 		}
@@ -228,7 +240,7 @@ func MultisigVerify(msg Hashable, addr Digest, sig MultisigSig) (err error) {
 
 // MultisigBatchPrep performs checks on the assembled MultisigSig and adds to the batch.
 // The caller must call batchVerifier.verify() to verify it.
-func MultisigBatchPrep(msg Hashable, addr Digest, sig MultisigSig, batchVerifier *BatchVerifier) (err error) {
+func MultisigBatchPrep(msg Hashable, addr Digest, sig MultisigSig, batchVerifier *BatchVerifier) error {
 	// short circuit: if msig doesn't have subsigs or if Subsigs are empty
 	// then terminate (the upper layer should now verify the unisig)
 	if (len(sig.Subsigs) == 0 || sig.Subsigs[0] == MultisigSubsig{}) {
@@ -238,7 +250,7 @@ func MultisigBatchPrep(msg Hashable, addr Digest, sig MultisigSig, batchVerifier
 	// check the address is correct
 	addrnew, err := MultisigAddrGenWithSubsigs(sig.Version, sig.Threshold, sig.Subsigs)
 	if err != nil {
-		return
+		return err
 	}
 	if addr != addrnew {
 		return errInvalidAddress
@@ -249,37 +261,18 @@ func MultisigBatchPrep(msg Hashable, addr Digest, sig MultisigSig, batchVerifier
 		return errInvalidNumberOfSignature
 	}
 
-	// check that we don't have too few multisig subsigs
-	if len(sig.Subsigs) < int(sig.Threshold) {
-		return errInvalidNumberOfSignature
-	}
-
 	// checks the number of non-blank signatures is no less than threshold
-	var counter uint8
-	for _, subsigi := range sig.Subsigs {
-		if (subsigi.Sig != Signature{}) {
-			counter++
-		}
-	}
-	if counter < sig.Threshold {
+	if sig.Signatures() < int(sig.Threshold) {
 		return errInvalidNumberOfSignature
 	}
 
-	// checks individual signature verifies
-	var sigCount int
+	// queues individual signature verifies
 	for _, subsigi := range sig.Subsigs {
-		if (subsigi.Sig != Signature{}) {
+		if !subsigi.Sig.Blank() {
 			batchVerifier.EnqueueSignature(subsigi.Key, msg, subsigi.Sig)
-			sigCount++
 		}
 	}
-
-	// sanity check. if we get here then every non-blank subsig should have
-	// been verified successfully, and we should have had enough of them
-	if sigCount < int(sig.Threshold) {
-		return errInvalidNumberOfSignature
-	}
-	return
+	return nil
 }
 
 // MultisigAdd adds unisig to an existing msig
@@ -315,8 +308,8 @@ func MultisigAdd(unisig []MultisigSig, msig *MultisigSig) (err error) {
 	// update the msig
 	for i := 0; i < len(unisig); i++ {
 		for j := 0; j < len(msig.Subsigs); j++ {
-			if (unisig[i].Subsigs[j].Sig != Signature{}) {
-				if (msig.Subsigs[j].Sig == Signature{}) {
+			if !unisig[i].Subsigs[j].Sig.Blank() {
+				if msig.Subsigs[j].Sig.Blank() {
 					// add the signature
 					msig.Subsigs[j].Sig = unisig[i].Subsigs[j].Sig
 				} else if msig.Subsigs[j].Sig != unisig[i].Subsigs[j].Sig {
@@ -354,13 +347,13 @@ func MultisigMerge(msig1 MultisigSig, msig2 MultisigSig) (msigt MultisigSig, err
 	msigt.Subsigs = make([]MultisigSubsig, len(msig1.Subsigs))
 	for i := 0; i < len(msigt.Subsigs); i++ {
 		msigt.Subsigs[i].Key = msig1.Subsigs[i].Key
-		if (msig1.Subsigs[i].Sig == Signature{}) {
-			if (msig2.Subsigs[i].Sig != Signature{}) {
+		if msig1.Subsigs[i].Sig.Blank() {
+			if !msig2.Subsigs[i].Sig.Blank() {
 				// update signature with msig2's signature
 				msigt.Subsigs[i].Sig = msig2.Subsigs[i].Sig
 			}
-		} else if (msig2.Subsigs[i].Sig == Signature{} || // msig2's sig is empty
-			msig2.Subsigs[i].Sig == msig1.Subsigs[i].Sig) { // valid duplicates
+		} else if msig2.Subsigs[i].Sig.Blank() || // msig2's sig is empty
+			msig2.Subsigs[i].Sig == msig1.Subsigs[i].Sig { // valid duplicates
 			// update signature with msig1's signature
 			msigt.Subsigs[i].Sig = msig1.Subsigs[i].Sig
 		} else {

data/transactions/logic/backwardCompat_test.go

@@ -368,7 +368,6 @@ func TestBackwardCompatGlobalFields(t *testing.T) {
 		ops := testProg(t, text, AssemblerMaxVersion)
 
 		ep, _, _ := makeSampleEnvWithVersion(1)
-		ep.TxnGroup[0].Txn.RekeyTo = basics.Address{} // avoid min version issues
 		ep.TxnGroup[0].Lsig.Logic = ops.Program
 		_, err := EvalSignature(0, ep)
 		require.Error(t, err)
@@ -431,11 +430,7 @@ func TestBackwardCompatTxnFields(t *testing.T) {
 				require.NoError(t, err)
 			}
 
-			ep, tx, _ := makeSampleEnvWithVersion(1)
-			// We'll reject too early if we have a nonzero RekeyTo, because that
-			// field must be zero for every txn in the group if this is an old
-			// AVM version
-			tx.RekeyTo = basics.Address{}
+			ep, _, _ := makeSampleEnvWithVersion(1)
 			ep.TxnGroup[0].Lsig.Logic = ops.Program
 
 			// check failure with version check

data/transactions/logic/debugger.go

@@ -177,7 +177,7 @@ func makeDebugState(cx *EvalContext) *DebugState {
 	globals := make([]basics.TealValue, len(globalFieldSpecs))
 	for _, fs := range globalFieldSpecs {
 		// Don't try to grab app only fields when evaluating a signature
-		if (cx.runModeFlags&ModeSig) != 0 && fs.mode == ModeApp {
+		if cx.runMode == ModeSig && fs.mode == ModeApp {
 			continue
 		}
 		sv, err := cx.globalFieldToValue(fs)
@@ -188,7 +188,7 @@ func makeDebugState(cx *EvalContext) *DebugState {
 	}
 	ds.Globals = globals
 
-	if (cx.runModeFlags & ModeApp) != 0 {
+	if (cx.runMode & ModeApp) != 0 {
 		ds.EvalDelta = cx.txn.EvalDelta
 	}
 
@@ -300,7 +300,7 @@ func (a *debuggerEvalTracerAdaptor) refreshDebugState(cx *EvalContext, evalError
 	ds.OpcodeBudget = cx.remainingBudget()
 	ds.CallStack = ds.parseCallstack(cx.callstack)
 
-	if (cx.runModeFlags & ModeApp) != 0 {
+	if cx.runMode == ModeApp {
 		ds.EvalDelta = cx.txn.EvalDelta
 	}
 

data/transactions/logic/eval.go

@@ -570,8 +570,8 @@ type scratchSpace [256]stackValue
 type EvalContext struct {
 	*EvalParams
 
-	// determines eval mode: runModeSignature or runModeApplication
-	runModeFlags RunMode
+	// determines eval mode: ModeSig or ModeApp
+	runMode RunMode
 
 	// the index of the transaction being evaluated
 	groupIndex int
@@ -624,7 +624,7 @@ func (cx *EvalContext) GroupIndex() int {
 
 // RunMode returns the evaluation context's mode (signature or application)
 func (cx *EvalContext) RunMode() RunMode {
-	return cx.runModeFlags
+	return cx.runMode
 }
 
 // PC returns the program counter of the current application being evaluated
@@ -911,11 +911,11 @@ func EvalContract(program []byte, gi int, aid basics.AppIndex, params *EvalParam
 		return false, nil, errors.New("0 appId in contract eval")
 	}
 	cx := EvalContext{
-		EvalParams:   params,
-		runModeFlags: ModeApp,
-		groupIndex:   gi,
-		txn:          &params.TxnGroup[gi],
-		appID:        aid,
+		EvalParams: params,
+		runMode:    ModeApp,
+		groupIndex: gi,
+		txn:        &params.TxnGroup[gi],
+		appID:      aid,
 	}
 
 	if cx.Proto.IsolateClearState && cx.txn.Txn.OnCompletion == transactions.ClearStateOC {
@@ -1014,10 +1014,10 @@ func EvalSignatureFull(gi int, params *EvalParams) (bool, *EvalContext, error) {
 		return false, nil, errors.New("no sig ledger in signature eval")
 	}
 	cx := EvalContext{
-		EvalParams:   params,
-		runModeFlags: ModeSig,
-		groupIndex:   gi,
-		txn:          &params.TxnGroup[gi],
+		EvalParams: params,
+		runMode:    ModeSig,
+		groupIndex: gi,
+		txn:        &params.TxnGroup[gi],
 	}
 	pass, err := eval(cx.txn.Lsig.Logic, &cx)
 
@@ -1176,7 +1176,7 @@ func check(program []byte, params *EvalParams, mode RunMode) (err error) {
 	cx.version = version
 	cx.pc = vlen
 	cx.EvalParams = params
-	cx.runModeFlags = mode
+	cx.runMode = mode
 	cx.program = program
 	cx.branchTargets = make([]bool, len(program)+1) // teal v2 allowed jumping to the end of the prog
 	cx.instructionStarts = make([]bool, len(program)+1)
@@ -1262,7 +1262,7 @@ func (cx *EvalContext) AppID() basics.AppIndex {
 }
 
 func (cx *EvalContext) remainingBudget() int {
-	if cx.runModeFlags == ModeSig {
+	if cx.runMode == ModeSig {
 		return int(cx.Proto.LogicSigMaxCost) - cx.cost
 	}
 
@@ -1300,7 +1300,7 @@ func (cx *EvalContext) step() error {
 	if spec.op == nil {
 		return fmt.Errorf("%3d illegal opcode 0x%02x", cx.pc, opcode)
 	}
-	if (cx.runModeFlags & spec.Modes) == 0 {
+	if (cx.runMode & spec.Modes) == 0 {
 		return fmt.Errorf("%s not allowed in current mode", spec.Name)
 	}
 
@@ -1449,7 +1449,7 @@ func (cx *EvalContext) checkStep() (int, error) {
 	if spec.op == nil {
 		return 0, fmt.Errorf("illegal opcode 0x%02x", opcode)
 	}
-	if (cx.runModeFlags & spec.Modes) == 0 {
+	if (cx.runMode & spec.Modes) == 0 {
 		return 0, fmt.Errorf("%s not allowed in current mode", spec.Name)
 	}
 	deets := spec.OpDetails
@@ -2871,7 +2871,7 @@ func (cx *EvalContext) getTxID(txn *transactions.Transaction, groupIndex int, in
 
 func (cx *EvalContext) txnFieldToStack(stxn *transactions.SignedTxnWithAD, fs *txnFieldSpec, arrayFieldIdx uint64, groupIndex int, inner bool) (sv stackValue, err error) {
 	if fs.effects {
-		if cx.runModeFlags == ModeSig {
+		if cx.runMode == ModeSig {
 			return sv, fmt.Errorf("txn[%s] not allowed in current mode", fs.field)
 		}
 		if cx.version < txnEffectsVersion && !inner {
@@ -3139,7 +3139,7 @@ func (cx *EvalContext) opTxnImpl(gi uint64, src txnSource, field TxnField, ai ui
 	case srcGroup:
 		if fs.effects && gi >= uint64(cx.groupIndex) {
 			// Test mode so that error is clearer
-			if cx.runModeFlags == ModeSig {
+			if cx.runMode == ModeSig {
 				return sv, fmt.Errorf("txn[%s] not allowed in current mode", fs.field)
 			}
 			return sv, fmt.Errorf("txn effects can only be read from past txns %d %d", gi, cx.groupIndex)
@@ -3557,7 +3557,7 @@ func opGlobal(cx *EvalContext) error {
 	if !ok || fs.version > cx.version {
 		return fmt.Errorf("invalid global field %s", globalField)
 	}
-	if (cx.runModeFlags & fs.mode) == 0 {
+	if (cx.runMode & fs.mode) == 0 {
 		return fmt.Errorf("global[%s] not allowed in current mode", globalField)
 	}
 

data/transactions/logic/evalStateful_test.go

@@ -57,6 +57,10 @@ func makeSampleEnvWithVersion(version uint64) (*EvalParams, *transactions.Transa
 	if version >= appsEnabledVersion {
 		firstTxn.Txn.Type = protocol.ApplicationCallTx
 	}
+	// avoid putting in a RekeyTo field if version < rekeyingEnabledVersion
+	if version < rekeyingEnabledVersion {
+		firstTxn.Txn.RekeyTo = basics.Address{}
+	}
 	ep := defaultEvalParamsWithVersion(version, makeSampleTxnGroup(firstTxn)...)
 	ledger := NewLedger(nil)
 	ep.SigLedger = ledger
@@ -2845,11 +2849,11 @@ func TestReturnTypes(t *testing.T) {
 				ep.ioBudget = 50
 
 				cx := EvalContext{
-					EvalParams:   ep,
-					runModeFlags: m,
-					groupIndex:   1,
-					txn:          &ep.TxnGroup[1],
-					appID:        300,
+					EvalParams: ep,
+					runMode:    m,
+					groupIndex: 1,
+					txn:        &ep.TxnGroup[1],
+					appID:      300,
 				}
 
 				// These set conditions for some ops that examine the group.

data/transactions/verify/txn.go

@@ -252,7 +252,7 @@ const stateProofTxn sigOrTxnType = 4
 // checkTxnSigTypeCounts checks the number of signature types and reports an error in case of a violation
 func checkTxnSigTypeCounts(s *transactions.SignedTxn, groupIndex int) (sigType sigOrTxnType, err *TxGroupError) {
 	numSigCategories := 0
-	if s.Sig != (crypto.Signature{}) {
+	if !s.Sig.Blank() {
 		numSigCategories++
 		sigType = regularSig
 	}
@@ -295,15 +295,10 @@ func stxnCoreChecks(s *transactions.SignedTxn, groupIndex int, groupCtx *GroupCo
 		if err := crypto.MultisigBatchPrep(s.Txn, crypto.Digest(s.Authorizer()), s.Msig, batchVerifier); err != nil {
 			return &TxGroupError{err: fmt.Errorf("multisig validation failed: %w", err), GroupIndex: groupIndex, Reason: TxGroupErrorReasonMsigNotWellFormed}
 		}
-		counter := 0
-		for _, subsigi := range s.Msig.Subsigs {
-			if (subsigi.Sig != crypto.Signature{}) {
-				counter++
-			}
-		}
-		if counter <= 4 {
+		sigs := s.Msig.Signatures()
+		if sigs <= 4 {
 			msigLessOrEqual4.Inc(nil)
-		} else if counter <= 10 {
+		} else if sigs <= 10 {
 			msigLessOrEqual10.Inc(nil)
 		} else {
 			msigMore10.Inc(nil)
@@ -375,7 +370,7 @@ func logicSigSanityCheckBatchPrep(txn *transactions.SignedTxn, groupIndex int, g
 
 	hasMsig := false
 	numSigs := 0
-	if lsig.Sig != (crypto.Signature{}) {
+	if !lsig.Sig.Blank() {
 		numSigs++
 	}
 	if !lsig.Msig.Blank() {
@@ -403,15 +398,10 @@ func logicSigSanityCheckBatchPrep(txn *transactions.SignedTxn, groupIndex int, g
 		if err := crypto.MultisigBatchPrep(&program, crypto.Digest(txn.Authorizer()), lsig.Msig, batchVerifier); err != nil {
 			return fmt.Errorf("logic multisig validation failed: %w", err)
 		}
-		counter := 0
-		for _, subsigi := range lsig.Msig.Subsigs {
-			if (subsigi.Sig != crypto.Signature{}) {
-				counter++
-			}
-		}
-		if counter <= 4 {
+		sigs := lsig.Msig.Signatures()
+		if sigs <= 4 {
 			msigLsigLessOrEqual4.Inc(nil)
-		} else if counter <= 10 {
+		} else if sigs <= 10 {
 			msigLsigLessOrEqual10.Inc(nil)
 		} else {
 			msigLsigMore10.Inc(nil)

data/transactions/verify/txnBatch.go

@@ -212,14 +212,7 @@ func getNumberOfBatchableSigsInTxn(stx *transactions.SignedTxn, groupIndex int)
 	case regularSig:
 		return 1, nil
 	case multiSig:
-		sig := stx.Msig
-		batchSigs := uint64(0)
-		for _, subsigi := range sig.Subsigs {
-			if (subsigi.Sig != crypto.Signature{}) {
-				batchSigs++
-			}
-		}
-		return batchSigs, nil
+		return uint64(stx.Msig.Signatures()), nil
 	case logicSig:
 		// Currently the sigs in here are not batched. Something to consider later.
 		return 0, nil