Show peer that sent transaction when it causes a DoS(100). #1375
Conversation
|
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 ;) |
Conflicts: src/main.cpp
|
Consensus: not worth it, closing |
* fix recently observed crash on IsValid +some small cleanup * txin.prevout.n should not exceed txOutpointCreated.vout.size() * fix log output
In the previous attempt I forgot to change an operator definition in miner.cpp and one in txmempool.h
…he UI model 9182b90 Initialize logging before we do parameter interaction (Jonas Schnelli) dec0787 [QT] Call inits parameter interaction before we create the options model (Jonas Schnelli) 18480ce Refactor parameter interaction, call it before AppInit2() (Jonas Schnelli) Pull request description: Backport of bitcoin#6780 Currently optionsModel(QT) parameters interaction overwrites/dominates initial and depending parameter settings. This PR would factor out init's parameter interaction and call it before AppInit2(). ACKs for top commit: furszy: ACK 9182b90 random-zebra: ACK rebase 9182b90 and merging... Tree-SHA512: f01c9b6603452d59c3fa7df8b8f85998b9c716fd3209f80fc6d4591035c995ad577b217d1d7ffd7f642d16fe130ee922ba0394870ec391b8e972587022962611
…ns can be shared 77998ed Combine common error strings so translations can be shared and reused (Luke Dashjr) Pull request description: Backport of bitcoin#7257 Adds helper methods for common translatable string outputs during init, end result is that the number of strings sent to Transifex is reduced/standardized. Based on top of bitcoin#1375 Conflicts with bitcoin#1373 (this to be rebased after both bitcoin#1373 and bitcoin#1375 are merged) ACKs for top commit: random-zebra: utACK 77998ed furszy: utACK 77998ed and merging.. Tree-SHA512: 9805b931829f76cbc34b43b7e6141b236d162bd0267342714222612b8e51cdc9d8b35e68971775bd7a489e95a3c4f9b533bbf1ecaa0e56b19a443ed308141c87
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.