Skip processing of TXs that we already have. #9414

1. 
   rebroad commented at 4:54 pm on December 23, 2016: contributor

   Previously TXs were processed more than they needed to be even though we already have them.
2. Skip processing of TXs that we already have.

   Previously TXs were processed more than they needed to be even though we
   already have them.

   715b64abe2
3. in src/net_processing.cpp: in 715b64abe2

   1684@@ -1682,9 +1685,9 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
   1685 
   1686             BOOST_FOREACH(uint256 hash, vEraseQueue)
   1687                 EraseOrphanTx(hash);
   1688-        }
   1689+        } // if accepted to memory pool
   1690         else if (fMissingInputs)
   

   ---

   ---

   

   rebroad commented at 4:55 pm on December 23, 2016:

   for fMissingInputs to be true AlreadyHave() had to be false
4. in src/net_processing.cpp: in 715b64abe2

   1711@@ -1709,7 +1712,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
   1712             } else {
   1713                 LogPrint("mempool", "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString());
   1714             }
   1715-        } else {
   1716+        } else { // not accepted and not missing inputs, i.e. invalid
   

   ---

   ---

   

   rebroad commented at 4:56 pm on December 23, 2016:

   whereas here AlreadyHave() could have been true still, so for whitelisted nodes, the same tx could be relayed multiple times

   ---

   

   gmaxwell commented at 7:48 pm on December 23, 2016:

   That is the intended and advertised purpose of the whitelisting, read the comment in the else block!
5. 
   gmaxwell commented at 5:45 pm on December 23, 2016: contributor

   && is shortcutting so this doesn’t change it to skip AcceptToMemoryPool. It does however change it so that it does not run the else clause which is needed for the forced relaying for whitelisted peers (which needs to run already for transactions we already have).

   So I think the only real behavior change this makes is to break the functionality.

6. 
   rebroad commented at 5:11 am on December 24, 2016: contributor

   @gmaxwell Ah, yes, my apologies I has missed the whitelist intention there. It was primarily noticing that (!tx.HasWitness() && !state.CorruptionPossible()) is bring run against txs we already have in the mempool that made me wonder if there was a logic oversight here, but perhaps the overhead from running this checks again is not significant.

   I am not aware yet of why we would want to relay txs (possibly multiple times) to whitelisted nodes that we already have in our mempool though - is this requirement documented somewhere please?

7. rebroad closed this on Dec 24, 2016

   ---

8. MarcoFalke locked this on Sep 8, 2021

Contributors
rebroad gmaxwell