txnbuild: remove redundant ok from internal verify functions (#2065)

Remove the `ok` bool value that the `verifyTxSignature` function returns alongside its `error` value.

The `bool` is redundant.

The `verifyTxSignature` function returns both a `bool` and an `error` but both are communicating the same result. The bool is only ever true when there is no error, and only ever false when there is an error. I'm refactoring some of this code and plan to use this function in a new function and it makes that reuse more difficult if this function returns bool and error.

In my new function I won't be passing on the bool to the caller, and so the question remains in my new function should I handle the bool and error separately so that if bool ever returns false without an error I generate some type of new error. I'd like to not have to solve this problem since it isn't important and instead remove the redundant bool.

txnbuild/transaction.go

@@ -477,42 +477,48 @@ func VerifyChallengeTx(challengeTx, serverAccountID, network string) (bool, erro
 	}
 
 	// verify signature from operation source
-	ok, err = verifyTxSignature(tx, op.SourceAccount.GetAccountID())
+	err = verifyTxSignature(tx, op.SourceAccount.GetAccountID())
 	if err != nil {
-		return ok, err
+		return false, err
 	}
 
 	// verify signature from server signing key
-	return verifyTxSignature(tx, serverAccountID)
+	err = verifyTxSignature(tx, serverAccountID)
+	if err != nil {
+		return false, err
+	}
+
+	return true, nil
 }
 
 // verifyTxSignature checks if a transaction has been signed by the provided Stellar account.
-func verifyTxSignature(tx Transaction, accountID string) (bool, error) {
-	signerFound := false
+func verifyTxSignature(tx Transaction, accountID string) error {
 	txHash, err := tx.Hash()
 	if err != nil {
-		return signerFound, err
+		return err
 	}
 
 	kp, err := keypair.Parse(accountID)
 	if err != nil {
-		return signerFound, err
+		return err
 	}
 
 	// find and verify signatures
 	if tx.xdrEnvelope == nil {
-		return signerFound, errors.New("transaction has no signatures")
+		return errors.New("transaction has no signatures")
 	}
+
+	signerFound := false
 	for _, s := range tx.xdrEnvelope.Signatures {
 		e := kp.Verify(txHash[:], s.Signature)
 		if e == nil {
 			signerFound = true
 			break
 		}
 	}
-
 	if !signerFound {
-		return signerFound, errors.Errorf("transaction not signed by %s", accountID)
+		return errors.Errorf("transaction not signed by %s", accountID)
 	}
-	return signerFound, nil
+
+	return nil
 }

txnbuild/transaction_test.go

@@ -1183,11 +1183,10 @@ func TestVerifyTxSignatureUnsignedTx(t *testing.T) {
 	// verify unsigned tx
 	err := tx.Build()
 	assert.NoError(t, err)
-	ok, err := verifyTxSignature(tx, kp0.Address())
+	err = verifyTxSignature(tx, kp0.Address())
 	if assert.Error(t, err) {
 		assert.Contains(t, err.Error(), "transaction not signed by GDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKTL3")
 	}
-	assert.Equal(t, false, ok)
 }
 
 func TestVerifyTxSignatureSingle(t *testing.T) {
@@ -1211,9 +1210,8 @@ func TestVerifyTxSignatureSingle(t *testing.T) {
 	assert.NoError(t, err)
 	err = tx.Sign(kp0)
 	assert.NoError(t, err)
-	ok, err := verifyTxSignature(tx, kp0.Address())
+	err = verifyTxSignature(tx, kp0.Address())
 	assert.NoError(t, err)
-	assert.Equal(t, true, ok)
 }
 
 func TestVerifyTxSignatureMultiple(t *testing.T) {
@@ -1237,12 +1235,10 @@ func TestVerifyTxSignatureMultiple(t *testing.T) {
 	assert.NoError(t, err)
 	err = tx.Sign(kp0, kp1)
 	assert.NoError(t, err)
-	ok, err := verifyTxSignature(tx, kp0.Address())
+	err = verifyTxSignature(tx, kp0.Address())
 	assert.NoError(t, err)
-	assert.Equal(t, true, ok)
-	ok, err = verifyTxSignature(tx, kp1.Address())
+	err = verifyTxSignature(tx, kp1.Address())
 	assert.NoError(t, err)
-	assert.Equal(t, true, ok)
 }
 func TestVerifyTxSignatureInvalid(t *testing.T) {
 	kp0 := newKeypair0()
@@ -1265,11 +1261,10 @@ func TestVerifyTxSignatureInvalid(t *testing.T) {
 	assert.NoError(t, err)
 	err = tx.Sign(kp0, kp1)
 	assert.NoError(t, err)
-	ok, err := verifyTxSignature(tx, "GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY")
+	err = verifyTxSignature(tx, "GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY")
 	if assert.Error(t, err) {
 		assert.Contains(t, err.Error(), "transaction not signed by GATBMIXTHXYKSUZSZUEJKACZ2OS2IYUWP2AIF3CA32PIDLJ67CH6Y5UY")
 	}
-	assert.Equal(t, false, ok)
 }
 
 func TestVerifyChallengeTxInvalid(t *testing.T) {