Two if's before sending an inventory message

Going through main.cpp I couldn't completely understand the purpose of a double barrier coded as if-statements:
I'm referring to ...

if (pto->setInventoryKnown.count(inv))

and

if (pto->setInventoryKnown.insert(inv).second)

What is the idea behind checking if an inventory is known to the target peer twice?
I'm aware that if (pto->setInventoryKnown.count(inv)) only checks if entry is known to target peer and if (pto->setInventoryKnown.insert(inv).second) inserts the inventory item after checking its uniqueness.
I mean couldn't one just use only one if-statement combining?

Aliakbar Ahmadi

Posted 2015-06-10T11:27:10.263

Reputation: 1 335

Answers

I don't see any reason for the second if statement, either. I'm not speaking from any form of authority here, but it looks like a (tiny) efficiency bug to me.

The second if statement has been around since the first commit. The first if statement was inserted about a year later when "trickling" of tx inv messages was added, presumably to check the set membership earlier to avoid running through the trickling logic when it's not necessary. I'd guess that the removal of the second if was simply overlooked.

Christopher Gurnee

Posted 2015-06-10T11:27:10.263

Reputation: 2 263

I'm not sure if the 2nd if is useless.

The second if does a more detailed check (see mruset.hL50 https://github.com/bitcoin/bitcoin/blob/3fce72eaa3ea75aa911e32c4d96313848338cede/src/mruset.h#L51) which i think reduces the size of the set, etc.

Jonas Schnelli

Posted 2015-06-10T11:27:10.263

Reputation: 5 465

I wasn't arguing (although maybe I was unclear) that the insert was useless, only that checking the result of the insert was useless because (as Aliakbar Ahmadi noticed) the result will always be true. Does my answer need updating? – Christopher Gurnee – 2015-06-10T19:56:37.270

Ah. Right. The if itself is useless but not the expression within the if. Fair enough. :) – Jonas Schnelli – 2015-06-10T20:02:55.877