diff --git a/crypto/batchverifier.go b/crypto/batchverifier.go index cce8e06d7e..db8ffe6426 100644 --- a/crypto/batchverifier.go +++ b/crypto/batchverifier.go @@ -53,7 +53,6 @@ const minBatchVerifierAlloc = 16 // Batch verifications errors var ( ErrBatchVerificationFailed = errors.New("At least one signature didn't pass verification") - ErrZeroTransactionInBatch = errors.New("Could not validate empty signature set") ) //export ed25519_randombytes_unsafe @@ -104,19 +103,19 @@ func (b *BatchVerifier) expand() { b.signatures = signatures } -// GetNumberOfEnqueuedSignatures returns the number of signatures current enqueue onto the bacth verifier object -func (b *BatchVerifier) GetNumberOfEnqueuedSignatures() int { +// getNumberOfEnqueuedSignatures returns the number of signatures current enqueue onto the bacth verifier object +func (b *BatchVerifier) getNumberOfEnqueuedSignatures() int { return len(b.messages) } // Verify verifies that all the signatures are valid. in that case nil is returned // if the batch is zero an appropriate error is return. func (b *BatchVerifier) Verify() error { - if b.GetNumberOfEnqueuedSignatures() == 0 { - return ErrZeroTransactionInBatch + if b.getNumberOfEnqueuedSignatures() == 0 { + return nil } - var messages = make([][]byte, b.GetNumberOfEnqueuedSignatures()) + var messages = make([][]byte, b.getNumberOfEnqueuedSignatures()) for i, m := range b.messages { messages[i] = HashRep(m) } diff --git a/crypto/batchverifier_test.go b/crypto/batchverifier_test.go index 781a80e174..4469da400b 100644 --- a/crypto/batchverifier_test.go +++ b/crypto/batchverifier_test.go @@ -62,7 +62,7 @@ func TestBatchVerifierBulk(t *testing.T) { sig := sigSecrets.Sign(msg) bv.EnqueueSignature(sigSecrets.SignatureVerifier, msg, sig) } - require.Equal(t, n, bv.GetNumberOfEnqueuedSignatures()) + require.Equal(t, n, bv.getNumberOfEnqueuedSignatures()) require.NoError(t, bv.Verify()) } @@ -122,5 +122,5 @@ func BenchmarkBatchVerifier(b *testing.B) { func TestEmpty(t *testing.T) { partitiontest.PartitionTest(t) bv := MakeBatchVerifier() - require.Error(t, bv.Verify()) + require.NoError(t, bv.Verify()) } diff --git a/crypto/multisig.go b/crypto/multisig.go index 53386ebc97..62ec187a2b 100644 --- a/crypto/multisig.go +++ b/crypto/multisig.go @@ -216,33 +216,23 @@ func MultisigAssemble(unisig []MultisigSig) (msig MultisigSig, err error) { } // MultisigVerify verifies an assembled MultisigSig -func MultisigVerify(msg Hashable, addr Digest, sig MultisigSig) (verified bool, err error) { +func MultisigVerify(msg Hashable, addr Digest, sig MultisigSig) (err error) { batchVerifier := MakeBatchVerifier() - if verified, err = MultisigBatchVerify(msg, addr, sig, batchVerifier); err != nil { + if err = MultisigBatchPrep(msg, addr, sig, batchVerifier); err != nil { return } - if !verified { - return - } - if batchVerifier.GetNumberOfEnqueuedSignatures() == 0 { - return true, nil - } - if err = batchVerifier.Verify(); err != nil { - return false, err - } - return true, nil + err = batchVerifier.Verify() + return } -// MultisigBatchVerify verifies an assembled MultisigSig. -// it is the caller responsibility to call batchVerifier.verify() -func MultisigBatchVerify(msg Hashable, addr Digest, sig MultisigSig, batchVerifier *BatchVerifier) (verified bool, err error) { - verified = false +// 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) { // 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{}) { - err = errInvalidNumberOfSignature - return + return errInvalidNumberOfSignature } // check the address is correct @@ -251,20 +241,17 @@ func MultisigBatchVerify(msg Hashable, addr Digest, sig MultisigSig, batchVerifi return } if addr != addrnew { - err = errInvalidAddress - return + return errInvalidAddress } // check that we don't have too many multisig subsigs if len(sig.Subsigs) > maxMultisig { - err = errInvalidNumberOfSignature - return + return errInvalidNumberOfSignature } // check that we don't have too few multisig subsigs if len(sig.Subsigs) < int(sig.Threshold) { - err = errInvalidNumberOfSignature - return + return errInvalidNumberOfSignature } // checks the number of non-blank signatures is no less than threshold @@ -275,27 +262,23 @@ func MultisigBatchVerify(msg Hashable, addr Digest, sig MultisigSig, batchVerifi } } if counter < sig.Threshold { - err = errInvalidNumberOfSignature - return + return errInvalidNumberOfSignature } // checks individual signature verifies - var verifiedCount int + var sigCount int for _, subsigi := range sig.Subsigs { if (subsigi.Sig != Signature{}) { batchVerifier.EnqueueSignature(subsigi.Key, msg, subsigi.Sig) - verifiedCount++ + 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 verifiedCount < int(sig.Threshold) { - err = errInvalidNumberOfSignature - return + if sigCount < int(sig.Threshold) { + return errInvalidNumberOfSignature } - - verified = true return } diff --git a/crypto/multisig_test.go b/crypto/multisig_test.go index 28eec24598..5035300d21 100644 --- a/crypto/multisig_test.go +++ b/crypto/multisig_test.go @@ -136,15 +136,13 @@ func TestMultisig(t *testing.T) { require.NoError(t, err, "Multisig: unexpected failure in generating sig from pk 2") msig, err = MultisigAssemble(sigs) require.NoError(t, err, "Multisig: unexpected failure when assembling multisig") - verify, err := MultisigVerify(txid, addr, msig) + err = MultisigVerify(txid, addr, msig) require.NoError(t, err, "Multisig: unexpected verification failure with err") - require.True(t, verify, "Multisig: verification failed, verify flag was false") //test3: use the batch verification br := MakeBatchVerifier() - verify, err = MultisigBatchVerify(txid, addr, msig, br) + err = MultisigBatchPrep(txid, addr, msig, br) require.NoError(t, err, "Multisig: unexpected verification failure with err") - require.True(t, verify, "Multisig: verification failed, verify flag was false") res := br.Verify() require.NoError(t, res, "Multisig: batch verification failed") } @@ -200,8 +198,7 @@ func TestMultisigAddAndMerge(t *testing.T) { require.NoError(t, err, "Multisig: unexpected failure signing with pk 2") err = MultisigAdd(sigs, &msig1) require.NoError(t, err, "Multisig: unexpected err adding pk 2 signature to that of pk 0 and 1") - verify, err := MultisigVerify(txid, addr, msig1) - require.True(t, verify, "Multisig: verification failed, verify flag was false") + err = MultisigVerify(txid, addr, msig1) require.NoError(t, err, "Multisig: unexpected verification failure with err") // msig2 = {sig3, sig4} @@ -215,9 +212,8 @@ func TestMultisigAddAndMerge(t *testing.T) { // merge two msigs and then verify msigt, err := MultisigMerge(msig1, msig2) require.NoError(t, err, "Multisig: unexpected failure merging multisig messages {0, 1, 2} and {3, 4}") - verify, err = MultisigVerify(txid, addr, msigt) + err = MultisigVerify(txid, addr, msigt) require.NoError(t, err, "Multisig: unexpected verification failure with err") - require.True(t, verify, "Multisig: verification failed, verify flag was false") // create a valid duplicate on purpose // msig1 = {sig0, sig1, sig2} @@ -230,9 +226,8 @@ func TestMultisigAddAndMerge(t *testing.T) { require.NoError(t, err, "Multisig: unexpected failure adding pk 2 signature to that of pk 3 and 4") msigt, err = MultisigMerge(msig1, msig2) require.NoError(t, err, "Multisig: unexpected failure merging multisig messages {0, 1, 2} and {2, 3, 4}") - verify, err = MultisigVerify(txid, addr, msigt) + err = MultisigVerify(txid, addr, msigt) require.NoError(t, err, "Multisig: unexpected verification failure with err") - require.True(t, verify, "Multisig: verification failed, verify flag was false") return } @@ -254,12 +249,10 @@ func TestEmptyMultisig(t *testing.T) { addr, err := MultisigAddrGen(version, threshold, pks) require.NoError(t, err, "Multisig: unexpected failure generating message digest") emptyMutliSig := MultisigSig{Version: version, Threshold: threshold, Subsigs: make([]MultisigSubsig, 0)} - verify, err := MultisigVerify(txid, addr, emptyMutliSig) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigVerify(txid, addr, emptyMutliSig) require.Error(t, err, "Multisig: did not return error as expected") br := MakeBatchVerifier() - verify, err = MultisigBatchVerify(txid, addr, emptyMutliSig, br) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigBatchPrep(txid, addr, emptyMutliSig, br) require.Error(t, err, "Multisig: did not return error as expected") } @@ -282,12 +275,10 @@ func TestIncorrectAddrresInMultisig(t *testing.T) { MutliSig, err := MultisigSign(txid, addr, version, threshold, pks, *secrets) require.NoError(t, err, "Multisig: could not create mutlisig") addr[0] = addr[0] + 1 - verify, err := MultisigVerify(txid, addr, MutliSig) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigVerify(txid, addr, MutliSig) require.Error(t, err, "Multisig: did not return error as expected") br := MakeBatchVerifier() - verify, err = MultisigBatchVerify(txid, addr, MutliSig, br) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigBatchPrep(txid, addr, MutliSig, br) require.Error(t, err, "Multisig: did not return error as expected") } @@ -321,12 +312,10 @@ func TestMoreThanMaxSigsInMultisig(t *testing.T) { msig, err := MultisigAssemble(sigs) require.NoError(t, err, "Multisig: error assmeble multisig") - verify, err := MultisigVerify(txid, addr, msig) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigVerify(txid, addr, msig) require.Error(t, err, "Multisig: did not return error as expected") br := MakeBatchVerifier() - verify, err = MultisigBatchVerify(txid, addr, msig, br) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigBatchPrep(txid, addr, msig, br) require.Error(t, err, "Multisig: did not return error as expected") } @@ -360,12 +349,10 @@ func TestOneSignatureIsEmpty(t *testing.T) { msig, err := MultisigAssemble(sigs) require.NoError(t, err, "Multisig: error assmeble multisig") msig.Subsigs[0].Sig = Signature{} - verify, err := MultisigVerify(txid, addr, msig) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigVerify(txid, addr, msig) require.Error(t, err, "Multisig: did not return error as expected") br := MakeBatchVerifier() - verify, err = MultisigBatchVerify(txid, addr, msig, br) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigBatchPrep(txid, addr, msig, br) require.Error(t, err, "Multisig: did not return error as expected") } @@ -401,13 +388,11 @@ func TestOneSignatureIsInvalid(t *testing.T) { sigs[1].Subsigs[1].Sig[5] = sigs[1].Subsigs[1].Sig[5] + 1 msig, err := MultisigAssemble(sigs) require.NoError(t, err, "Multisig: error assmeble multisig") - verify, err := MultisigVerify(txid, addr, msig) - require.False(t, verify, "Multisig: verification succeeded, it should failed") + err = MultisigVerify(txid, addr, msig) require.Error(t, err, "Multisig: did not return error as expected") br := MakeBatchVerifier() - verify, err = MultisigBatchVerify(txid, addr, msig, br) + err = MultisigBatchPrep(txid, addr, msig, br) require.NoError(t, err, "Multisig: did not return error as expected") - require.True(t, verify, "Multisig: verification succeeded, it should failed") res := br.Verify() require.Error(t, res, "Multisig: batch verification passed on broken signature") @@ -455,15 +440,13 @@ func TestMultisigLessThanTrashold(t *testing.T) { msig, err = MultisigAssemble(sigs) require.NoError(t, err, "should be able to detect insufficient signatures for assembling") msig.Subsigs[1].Sig = BlankSignature - verify, err := MultisigVerify(txid, addr, msig) - require.False(t, verify, "Multisig: verification passed, should have failed") + err = MultisigVerify(txid, addr, msig) require.Error(t, err, "Multisig: expected verification failure with err") msig, err = MultisigAssemble(sigs) require.NoError(t, err, "should be able to detect insufficient signatures for assembling") msig.Subsigs = msig.Subsigs[:len(msig.Subsigs)-1] - verify, err = MultisigVerify(txid, addr, msig) - require.False(t, verify, "Multisig: verification passed, should have failed") + err = MultisigVerify(txid, addr, msig) require.Error(t, err, "Multisig: expected verification failure with err") } diff --git a/data/transactions/verify/txn.go b/data/transactions/verify/txn.go index 885d0882ec..4d0bd2717c 100644 --- a/data/transactions/verify/txn.go +++ b/data/transactions/verify/txn.go @@ -99,59 +99,43 @@ func (g *GroupContext) Equal(other *GroupContext) bool { g.minAvmVersion == other.minAvmVersion } -// Txn verifies a SignedTxn as being signed and having no obviously inconsistent data. +// txnBatchPrep verifies a SignedTxn having no obviously inconsistent data. // Block-assembly time checks of LogicSig and accounting rules may still block the txn. -func Txn(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext) error { - batchVerifier := crypto.MakeBatchVerifier() - - if err := TxnBatchVerify(s, txnIdx, groupCtx, batchVerifier); err != nil { - return err - } - - // this case is used for comapact certificate where no signature is supplied - if batchVerifier.GetNumberOfEnqueuedSignatures() == 0 { - return nil - } - return batchVerifier.Verify() -} - -// TxnBatchVerify verifies a SignedTxn having no obviously inconsistent data. -// Block-assembly time checks of LogicSig and accounting rules may still block the txn. -// it is the caller responsibility to call batchVerifier.verify() -func TxnBatchVerify(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, verifier *crypto.BatchVerifier) error { +// it is the caller responsibility to call batchVerifier.Verify() +func txnBatchPrep(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, verifier *crypto.BatchVerifier) error { if !groupCtx.consensusParams.SupportRekeying && (s.AuthAddr != basics.Address{}) { - return errors.New("nonempty AuthAddr but rekeying not supported") + return errors.New("nonempty AuthAddr but rekeying is not supported") } if err := s.Txn.WellFormed(groupCtx.specAddrs, groupCtx.consensusParams); err != nil { return err } - return stxnVerifyCore(s, txnIdx, groupCtx, verifier) + return stxnCoreChecks(s, txnIdx, groupCtx, verifier) } // TxnGroup verifies a []SignedTxn as being signed and having no obviously inconsistent data. func TxnGroup(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, cache VerifiedTransactionCache, ledger logic.LedgerForSignature) (groupCtx *GroupContext, err error) { batchVerifier := crypto.MakeBatchVerifier() - if groupCtx, err = TxnGroupBatchVerify(stxs, contextHdr, cache, ledger, batchVerifier); err != nil { + if groupCtx, err = txnGroupBatchPrep(stxs, contextHdr, ledger, batchVerifier); err != nil { return nil, err } - if batchVerifier.GetNumberOfEnqueuedSignatures() == 0 { - return groupCtx, nil - } - if err := batchVerifier.Verify(); err != nil { return nil, err } + if cache != nil { + cache.Add(stxs, groupCtx) + } + return } -// TxnGroupBatchVerify verifies a []SignedTxn having no obviously inconsistent data. -// it is the caller responsibility to call batchVerifier.verify() -func TxnGroupBatchVerify(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, cache VerifiedTransactionCache, ledger logic.LedgerForSignature, verifier *crypto.BatchVerifier) (groupCtx *GroupContext, err error) { +// txnGroupBatchPrep verifies a []SignedTxn having no obviously inconsistent data. +// it is the caller responsibility to call batchVerifier.Verify() +func txnGroupBatchPrep(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, ledger logic.LedgerForSignature, verifier *crypto.BatchVerifier) (groupCtx *GroupContext, err error) { groupCtx, err = PrepareGroupContext(stxs, contextHdr, ledger) if err != nil { return nil, err @@ -160,7 +144,7 @@ func TxnGroupBatchVerify(stxs []transactions.SignedTxn, contextHdr bookkeeping.B minFeeCount := uint64(0) feesPaid := uint64(0) for i, stxn := range stxs { - err = TxnBatchVerify(&stxn, i, groupCtx, verifier) + err = txnBatchPrep(&stxn, i, groupCtx, verifier) if err != nil { err = fmt.Errorf("transaction %+v invalid : %w", stxn, err) return @@ -184,13 +168,10 @@ func TxnGroupBatchVerify(stxs []transactions.SignedTxn, contextHdr bookkeeping.B return } - if cache != nil { - cache.Add(stxs, groupCtx) - } return } -func stxnVerifyCore(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error { +func stxnCoreChecks(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error { numSigs := 0 hasSig := false hasMsig := false @@ -227,16 +208,13 @@ func stxnVerifyCore(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContex return nil } if hasMsig { - if ok, _ := crypto.MultisigBatchVerify(s.Txn, - crypto.Digest(s.Authorizer()), - s.Msig, - batchVerifier); ok { - return nil + if err := crypto.MultisigBatchPrep(s.Txn, crypto.Digest(s.Authorizer()), s.Msig, batchVerifier); err != nil { + return fmt.Errorf("multisig validation failed: %w", err) } - return errors.New("multisig validation failed") + return nil } if hasLogicSig { - return logicSigBatchVerify(s, txnIdx, groupCtx) + return logicSigVerify(s, txnIdx, groupCtx) } return errors.New("has one mystery sig. WAT?") } @@ -246,22 +224,16 @@ func stxnVerifyCore(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContex func LogicSigSanityCheck(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext) error { batchVerifier := crypto.MakeBatchVerifier() - if err := LogicSigSanityCheckBatchVerify(txn, groupIndex, groupCtx, batchVerifier); err != nil { + if err := logicSigSanityCheckBatchPrep(txn, groupIndex, groupCtx, batchVerifier); err != nil { return err } - - // in case of contract account the signature len might 0. that's ok - if batchVerifier.GetNumberOfEnqueuedSignatures() == 0 { - return nil - } - return batchVerifier.Verify() } -// LogicSigSanityCheckBatchVerify checks that the signature is valid and that the program is basically well formed. +// logicSigSanityCheckBatchPrep checks that the signature is valid and that the program is basically well formed. // It does not evaluate the logic. -// it is the caller responsibility to call batchVerifier.verify() -func LogicSigSanityCheckBatchVerify(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error { +// it is the caller responsibility to call batchVerifier.Verify() +func logicSigSanityCheckBatchPrep(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext, batchVerifier *crypto.BatchVerifier) error { lsig := txn.Lsig if groupCtx.consensusParams.LogicSigVersion == 0 { @@ -323,16 +295,15 @@ func LogicSigSanityCheckBatchVerify(txn *transactions.SignedTxn, groupIndex int, batchVerifier.EnqueueSignature(crypto.PublicKey(txn.Authorizer()), &program, lsig.Sig) } else { program := logic.Program(lsig.Logic) - if ok, _ := crypto.MultisigBatchVerify(&program, crypto.Digest(txn.Authorizer()), lsig.Msig, batchVerifier); !ok { - return errors.New("logic multisig validation failed") + if err := crypto.MultisigBatchPrep(&program, crypto.Digest(txn.Authorizer()), lsig.Msig, batchVerifier); err != nil { + return fmt.Errorf("logic multisig validation failed: %w", err) } } return nil } -// logicSigBatchVerify checks that the signature is valid, executing the program. -// it is the caller responsibility to call batchVerifier.verify() -func logicSigBatchVerify(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext) error { +// logicSigVerify checks that the signature is valid, executing the program. +func logicSigVerify(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext) error { err := LogicSigSanityCheck(txn, groupIndex, groupCtx) if err != nil { return err @@ -404,17 +375,15 @@ func PaysetGroups(ctx context.Context, payset [][]transactions.SignedTxn, blkHea batchVerifier := crypto.MakeBatchVerifierWithHint(len(payset)) for i, signTxnsGrp := range txnGroups { - groupCtxs[i], grpErr = TxnGroupBatchVerify(signTxnsGrp, blkHeader, nil, ledger, batchVerifier) + groupCtxs[i], grpErr = txnGroupBatchPrep(signTxnsGrp, blkHeader, ledger, batchVerifier) // abort only if it's a non-cache error. if grpErr != nil { return grpErr } } - if batchVerifier.GetNumberOfEnqueuedSignatures() != 0 { - verifyErr := batchVerifier.Verify() - if verifyErr != nil { - return verifyErr - } + verifyErr := batchVerifier.Verify() + if verifyErr != nil { + return verifyErr } cache.AddPayset(txnGroups, groupCtxs) return nil diff --git a/data/transactions/verify/txn_test.go b/data/transactions/verify/txn_test.go index 3998352114..ea3f05b87e 100644 --- a/data/transactions/verify/txn_test.go +++ b/data/transactions/verify/txn_test.go @@ -51,6 +51,15 @@ var spec = transactions.SpecialAddresses{ RewardsPool: poolAddr, } +func verifyTxn(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext) error { + batchVerifier := crypto.MakeBatchVerifier() + + if err := txnBatchPrep(s, txnIdx, groupCtx, batchVerifier); err != nil { + return err + } + return batchVerifier.Verify() +} + func keypair() *crypto.SignatureSecrets { var seed crypto.Seed crypto.RandBytes(seed[:]) @@ -117,14 +126,14 @@ func TestSignedPayment(t *testing.T) { groupCtx, err := PrepareGroupContext(stxns, blockHeader, nil) require.NoError(t, err) require.NoError(t, payment.WellFormed(spec, proto), "generateTestObjects generated an invalid payment") - require.NoError(t, Txn(&stxn, 0, groupCtx), "generateTestObjects generated a bad signedtxn") + require.NoError(t, verifyTxn(&stxn, 0, groupCtx), "generateTestObjects generated a bad signedtxn") stxn2 := payment.Sign(secret) require.Equal(t, stxn2.Sig, stxn.Sig, "got two different signatures for the same transaction (our signing function is deterministic)") stxn2.MessUpSigForTesting() require.Equal(t, stxn.ID(), stxn2.ID(), "changing sig caused txid to change") - require.Error(t, Txn(&stxn2, 0, groupCtx), "verify succeeded with bad sig") + require.Error(t, verifyTxn(&stxn2, 0, groupCtx), "verify succeeded with bad sig") require.True(t, crypto.SignatureVerifier(addr).Verify(payment, stxn.Sig), "signature on the transaction is not the signature of the hash of the transaction under the spender's key") } @@ -137,7 +146,7 @@ func TestTxnValidationEncodeDecode(t *testing.T) { for _, txn := range signed { groupCtx, err := PrepareGroupContext([]transactions.SignedTxn{txn}, blockHeader, nil) require.NoError(t, err) - if Txn(&txn, 0, groupCtx) != nil { + if verifyTxn(&txn, 0, groupCtx) != nil { t.Errorf("signed transaction %#v did not verify", txn) } @@ -145,7 +154,7 @@ func TestTxnValidationEncodeDecode(t *testing.T) { var signedTx transactions.SignedTxn protocol.Decode(x, &signedTx) - if Txn(&signedTx, 0, groupCtx) != nil { + if verifyTxn(&signedTx, 0, groupCtx) != nil { t.Errorf("signed transaction %#v did not verify", txn) } } @@ -159,14 +168,14 @@ func TestTxnValidationEmptySig(t *testing.T) { for _, txn := range signed { groupCtx, err := PrepareGroupContext([]transactions.SignedTxn{txn}, blockHeader, nil) require.NoError(t, err) - if Txn(&txn, 0, groupCtx) != nil { + if verifyTxn(&txn, 0, groupCtx) != nil { t.Errorf("signed transaction %#v did not verify", txn) } txn.Sig = crypto.Signature{} txn.Msig = crypto.MultisigSig{} txn.Lsig = transactions.LogicSig{} - if Txn(&txn, 0, groupCtx) == nil { + if verifyTxn(&txn, 0, groupCtx) == nil { t.Errorf("transaction %#v verified without sig", txn) } } @@ -205,13 +214,13 @@ func TestTxnValidationStateProof(t *testing.T) { groupCtx, err := PrepareGroupContext([]transactions.SignedTxn{stxn}, blockHeader, nil) require.NoError(t, err) - err = Txn(&stxn, 0, groupCtx) + err = verifyTxn(&stxn, 0, groupCtx) require.NoError(t, err, "state proof txn %#v did not verify", stxn) stxn2 := stxn stxn2.Txn.Type = protocol.PaymentTx stxn2.Txn.Header.Fee = basics.MicroAlgos{Raw: proto.MinTxnFee} - err = Txn(&stxn2, 0, groupCtx) + err = verifyTxn(&stxn2, 0, groupCtx) require.Error(t, err, "payment txn %#v verified from StateProofSender", stxn2) secret := keypair() @@ -219,28 +228,28 @@ func TestTxnValidationStateProof(t *testing.T) { stxn2.Txn.Header.Sender = basics.Address(secret.SignatureVerifier) stxn2.Txn.Header.Fee = basics.MicroAlgos{Raw: proto.MinTxnFee} stxn2 = stxn2.Txn.Sign(secret) - err = Txn(&stxn2, 0, groupCtx) + err = verifyTxn(&stxn2, 0, groupCtx) require.Error(t, err, "state proof txn %#v verified from non-StateProofSender", stxn2) // state proof txns are not allowed to have non-zero values for many fields stxn2 = stxn stxn2.Txn.Header.Fee = basics.MicroAlgos{Raw: proto.MinTxnFee} - err = Txn(&stxn2, 0, groupCtx) + err = verifyTxn(&stxn2, 0, groupCtx) require.Error(t, err, "state proof txn %#v verified", stxn2) stxn2 = stxn stxn2.Txn.Header.Note = []byte{'A'} - err = Txn(&stxn2, 0, groupCtx) + err = verifyTxn(&stxn2, 0, groupCtx) require.Error(t, err, "state proof txn %#v verified", stxn2) stxn2 = stxn stxn2.Txn.Lease[0] = 1 - err = Txn(&stxn2, 0, groupCtx) + err = verifyTxn(&stxn2, 0, groupCtx) require.Error(t, err, "state proof txn %#v verified", stxn2) stxn2 = stxn stxn2.Txn.RekeyTo = basics.Address(secret.SignatureVerifier) - err = Txn(&stxn2, 0, groupCtx) + err = verifyTxn(&stxn2, 0, groupCtx) require.Error(t, err, "state proof txn %#v verified", stxn2) } @@ -258,7 +267,7 @@ func TestDecodeNil(t *testing.T) { // This used to panic when run on a zero value of SignedTxn. groupCtx, err := PrepareGroupContext([]transactions.SignedTxn{st}, blockHeader, nil) require.NoError(t, err) - Txn(&st, 0, groupCtx) + verifyTxn(&st, 0, groupCtx) } } @@ -425,9 +434,84 @@ func BenchmarkTxn(b *testing.B) { groupCtx, err := PrepareGroupContext(txnGroup, blk.BlockHeader, nil) require.NoError(b, err) for i, txn := range txnGroup { - err := Txn(&txn, i, groupCtx) + err := verifyTxn(&txn, i, groupCtx) require.NoError(b, err) } } b.StopTimer() } + +// TestTxnGroupCacheUpdate uses TxnGroup to verify txns and add them to the +// cache. Then makes sure that only the valid txns are verified and added to +// the cache. +func TestTxnGroupCacheUpdate(t *testing.T) { + partitiontest.PartitionTest(t) + + _, signedTxn, secrets, addrs := generateTestObjects(100, 20, 50) + blkHdr := bookkeeping.BlockHeader{ + Round: 50, + GenesisHash: crypto.Hash([]byte{1, 2, 3, 4, 5}), + UpgradeState: bookkeeping.UpgradeState{ + CurrentProtocol: protocol.ConsensusCurrentVersion, + }, + RewardsState: bookkeeping.RewardsState{ + FeeSink: feeSink, + RewardsPool: poolAddr, + }, + } + + txnGroups := generateTransactionGroups(signedTxn, secrets, addrs) + cache := MakeVerifiedTransactionCache(1000) + + // break the signature and see if it fails. + txnGroups[0][0].Sig[0] = txnGroups[0][0].Sig[0] + 1 + + _, err := TxnGroup(txnGroups[0], blkHdr, cache, nil) + require.Error(t, err) + + // The txns should not be in the cache + unverifiedGroups := cache.GetUnverifiedTransactionGroups(txnGroups[:1], spec, protocol.ConsensusCurrentVersion) + require.Len(t, unverifiedGroups, 1) + + unverifiedGroups = cache.GetUnverifiedTransactionGroups(txnGroups[:2], spec, protocol.ConsensusCurrentVersion) + require.Len(t, unverifiedGroups, 2) + + _, err = TxnGroup(txnGroups[1], blkHdr, cache, nil) + require.NoError(t, err) + + // Only the second txn should be in the cache + unverifiedGroups = cache.GetUnverifiedTransactionGroups(txnGroups[:2], spec, protocol.ConsensusCurrentVersion) + require.Len(t, unverifiedGroups, 1) + + // Fix the signature + txnGroups[0][0].Sig[0] = txnGroups[0][0].Sig[0] - 1 + + _, err = TxnGroup(txnGroups[0], blkHdr, cache, nil) + require.NoError(t, err) + + // Both transactions should be in the cache + unverifiedGroups = cache.GetUnverifiedTransactionGroups(txnGroups[:2], spec, protocol.ConsensusCurrentVersion) + require.Len(t, unverifiedGroups, 0) + + // Break a random signature + txgIdx := rand.Intn(len(txnGroups)) + txIdx := rand.Intn(len(txnGroups[txgIdx])) + txnGroups[txgIdx][txIdx].Sig[0] = txnGroups[0][0].Sig[0] + 1 + + numFailed := 0 + + // Add them to the cache by verifying them + for _, txng := range txnGroups { + _, err = TxnGroup(txng, blkHdr, cache, nil) + if err != nil { + numFailed++ + } + } + require.Equal(t, 1, numFailed) + + // Onle one transaction should not be in cache + unverifiedGroups = cache.GetUnverifiedTransactionGroups(txnGroups, spec, protocol.ConsensusCurrentVersion) + require.Len(t, unverifiedGroups, 1) + + require.Equal(t, unverifiedGroups[0], txnGroups[txgIdx]) +} diff --git a/test/e2e-go/kmd/e2e_kmd_wallet_multisig_test.go b/test/e2e-go/kmd/e2e_kmd_wallet_multisig_test.go index 3b43606d6d..be8f27e766 100644 --- a/test/e2e-go/kmd/e2e_kmd_wallet_multisig_test.go +++ b/test/e2e-go/kmd/e2e_kmd_wallet_multisig_test.go @@ -440,7 +440,6 @@ func TestMultisigSignProgram(t *testing.T) { err = protocol.Decode(resp3.Multisig, &msig) a.NoError(err) - ok, err := crypto.MultisigVerify(logic.Program(program), crypto.Digest(msigAddr), msig) + err = crypto.MultisigVerify(logic.Program(program), crypto.Digest(msigAddr), msig) a.NoError(err) - a.True(ok) } diff --git a/test/e2e-go/upgrades/rekey_support_test.go b/test/e2e-go/upgrades/rekey_support_test.go index 4988b79c4c..56f5b6408b 100644 --- a/test/e2e-go/upgrades/rekey_support_test.go +++ b/test/e2e-go/upgrades/rekey_support_test.go @@ -128,10 +128,10 @@ func TestRekeyUpgrade(t *testing.T) { _, err = client.BroadcastTransaction(rekeyed) // non empty err means the upgrade have not happened yet (as expected), ensure the error if err != nil { - // should be either "nonempty AuthAddr but rekeying not supported" or "txn dead" - if !strings.Contains(err.Error(), "nonempty AuthAddr but rekeying not supported") && + // should be either "nonempty AuthAddr but rekeying is not supported" or "txn dead" + if !strings.Contains(err.Error(), "nonempty AuthAddr but rekeying is not supported") && !strings.Contains(err.Error(), "txn dead") { - a.NoErrorf(err, "error message should be one of :\n%s\n%s", "nonempty AuthAddr but rekeying not supported", "txn dead") + a.NoErrorf(err, "error message should be one of :\n%s\n%s", "nonempty AuthAddr but rekeying is not supported", "txn dead") } } else { // if we had no error it must mean that we've upgraded already. Verify that.