Show peer that sent transaction when it causes a DoS(100). #1375

I originally raised pull #1311 with this, but that was too big and contained too many changes. Pull #1338 followed, but still had some changes which were contentious. This pull contains just the addition of displaying the peer of DoS(100) transactions, and pull #1338 has had that part removed.

Is this thread safe? What if the thread executing the if (txnode) is interrupted after the if test, could another thread wake up and set txnode=NULL ?

NACK in general-- too many code changes, the risks outweigh the (minimal) benefits.

NACK, example of bad use of a global IMO

@gavinandresen It's thread safe unless some future change tries to change the value of txnode from a different thread. So far, only one thread uses txnode, so the value won't get changed between the test and the use. @luke-jr - thanks for the feedback. How would you recommend doing it? I realise I could just add one line in ProcessMessage() to log the peer against the tx, but I guess my motivation was to keep debug.log small (at the expense of code brevity)....

You're right that currently txnode is only set in ProcessMessage, which is only used in a single thread. But I agree it's a bad use of a global. If you really need to know who sent it, pass it along in arguments, or only report the offender from functions that do know.

Not worth it, imho.

@sipa I did try passing it in arguments, but I was getting errors about incorrect number of arguments. Perhaps my understanding of C++ isn't quite sufficient.. :-s Is there any way to restrict the use of txnode so that it can't be used by other threads?

Too bad C++ doesn't have Perl's locals ;)

Consensus: not worth it, closing