performance: don't create child cow in TestTransactionGroup

[cce]

Summary

TestTransactionGroup is used by the transaction message handler to see if a transaction is already in the pool. It makes a child cow, which allocates a fair amount of objects, only to look up the transaction ID later from an empty map (and afterwards looking up the ID in the parent's map). This skips those allocations in TestTransactionGroup.

Test Plan

Existing tests should pass, new tests could be written to assert that TestTransactionGroup does not modify the parent cow.

[jannotti]

How about remove the cow parameter from eval.TestTransaction method, and use eval.state for the checkDup call that's inside?

[cce]

@jannotti updated as per feedback

[codecov]

Codecov Report

Merging #4706 (cd687ae) into master (ad08f74) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4706      +/-   ##
==========================================
- Coverage   54.49%   54.46%   -0.03%     
==========================================
  Files         407      407              
  Lines       52389    52388       -1     
==========================================
- Hits        28548    28535      -13     
- Misses      21455    21466      +11     
- Partials     2386     2387       +1     

Impacted Files	Coverage Δ
ledger/internal/eval.go	67.79% <100.00%> (-0.05%)	⬇️
ledger/roundlru.go	90.56% <0.00%> (-5.67%)	⬇️
crypto/merkletrie/trie.go	66.42% <0.00%> (-2.19%)	⬇️
agreement/cryptoVerifier.go	67.60% <0.00%> (-2.12%)	⬇️
agreement/proposalManager.go	96.07% <0.00%> (-1.97%)	⬇️
crypto/merkletrie/node.go	91.62% <0.00%> (-1.87%)	⬇️
catchup/peerSelector.go	98.95% <0.00%> (-1.05%)	⬇️
data/transactions/verify/txn.go	76.19% <0.00%> (-0.96%)	⬇️
ledger/acctupdates.go	69.59% <0.00%> (-0.60%)	⬇️
network/wsPeer.go	68.44% <0.00%> (ø)
... and 6 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jannotti]

ship it