From b0ef3a0228e8802cde04bf0bed1b328c74aeafda Mon Sep 17 00:00:00 2001 From: algonautshant Date: Tue, 27 Sep 2022 15:28:20 -0400 Subject: [PATCH] add test and refactoring --- data/transactions/verify/txn.go | 19 ++++--- data/transactions/verify/txn_test.go | 76 +++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 11 deletions(-) diff --git a/data/transactions/verify/txn.go b/data/transactions/verify/txn.go index 4fbf312973..781669fb93 100644 --- a/data/transactions/verify/txn.go +++ b/data/transactions/verify/txn.go @@ -118,7 +118,7 @@ func txnBatchVerifyPrep(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupCo func TxnGroup(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, cache VerifiedTransactionCache, ledger logic.LedgerForSignature) (groupCtx *GroupContext, err error) { batchVerifier := crypto.MakeBatchVerifier() - if groupCtx, err = txnGroupBatchVerifyPrep(stxs, contextHdr, cache, ledger, batchVerifier); err != nil { + if groupCtx, err = txnGroupBatchVerifyPrep(stxs, contextHdr, ledger, batchVerifier); err != nil { return nil, err } @@ -130,12 +130,16 @@ func TxnGroup(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, return nil, err } + if cache != nil { + cache.Add(stxs, groupCtx) + } + return } // txnGroupBatchVerifyPrep verifies a []SignedTxn having no obviously inconsistent data. // it is the caller responsibility to call batchVerifier.Verify() -func txnGroupBatchVerifyPrep(stxs []transactions.SignedTxn, contextHdr bookkeeping.BlockHeader, cache VerifiedTransactionCache, ledger logic.LedgerForSignature, verifier *crypto.BatchVerifier) (groupCtx *GroupContext, err error) { +func txnGroupBatchVerifyPrep(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 @@ -168,9 +172,6 @@ func txnGroupBatchVerifyPrep(stxs []transactions.SignedTxn, contextHdr bookkeepi return } - if cache != nil { - cache.Add(stxs, groupCtx) - } return } @@ -220,7 +221,7 @@ func stxnCoreChecks(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContex return errors.New("multisig validation failed") } if hasLogicSig { - return logicSigBatchVerify(s, txnIdx, groupCtx) + return logicSigVerify(s, txnIdx, groupCtx) } return errors.New("has one mystery sig. WAT?") } @@ -314,9 +315,7 @@ func logicSigSanityCheckBatchVerifyPrep(txn *transactions.SignedTxn, groupIndex 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 { +func logicSigVerify(txn *transactions.SignedTxn, groupIndex int, groupCtx *GroupContext) error { err := LogicSigSanityCheck(txn, groupIndex, groupCtx) if err != nil { return err @@ -388,7 +387,7 @@ func PaysetGroups(ctx context.Context, payset [][]transactions.SignedTxn, blkHea batchVerifier := crypto.MakeBatchVerifierWithHint(len(payset)) for i, signTxnsGrp := range txnGroups { - groupCtxs[i], grpErr = txnGroupBatchVerifyPrep(signTxnsGrp, blkHeader, nil, ledger, batchVerifier) + groupCtxs[i], grpErr = txnGroupBatchVerifyPrep(signTxnsGrp, blkHeader, ledger, batchVerifier) // abort only if it's a non-cache error. if grpErr != nil { return grpErr diff --git a/data/transactions/verify/txn_test.go b/data/transactions/verify/txn_test.go index 44108ad5bf..7bd3c68279 100644 --- a/data/transactions/verify/txn_test.go +++ b/data/transactions/verify/txn_test.go @@ -58,7 +58,6 @@ func verifyTxn(s *transactions.SignedTxn, txnIdx int, groupCtx *GroupContext) er return err } - // this case is used for comapact certificate where no signature is supplied if batchVerifier.GetNumberOfEnqueuedSignatures() == 0 { return nil } @@ -445,3 +444,78 @@ func BenchmarkTxn(b *testing.B) { } 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.Equal(t, 1, len(unverifiedGroups)) + + unverifiedGroups = cache.GetUnverifiedTransactionGroups(txnGroups[:2], spec, protocol.ConsensusCurrentVersion) + require.Equal(t, 2, len(unverifiedGroups)) + + _, 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.Equal(t, 1, len(unverifiedGroups)) + + // 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.Equal(t, 0, len(unverifiedGroups)) + + // 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.Equal(t, 1, len(unverifiedGroups)) + + require.Equal(t, unverifiedGroups[0], txnGroups[txgIdx]) +}