Problem: Stateful validation doesn't raise double spend exception#2422
Problem: Stateful validation doesn't raise double spend exception#2422ttmc merged 2 commits intobigchaindb:masterfrom
Conversation
bigchaindb/events.py
Outdated
|
|
||
| try: | ||
| self.started_queue.get_nowait() | ||
| self.started_queue.get(timeout=0.01) |
There was a problem hiding this comment.
After some debugging I found that even when the stated_queue contained the message 'STARTED' the get_nowait would return to soon and hence the correct exception RuntimeError wasn't being raised. I have changed it to use a timeout of 10 milliseconds.
There was a problem hiding this comment.
NOTE: This fix is to avoid random failure for test tests/test_events.py::test_event_handler_raises_when_called_after_start when running tests.
Codecov Report
@@ Coverage Diff @@
## master #2422 +/- ##
=======================================
Coverage 88.43% 88.43%
=======================================
Files 39 39
Lines 2275 2275
=======================================
Hits 2012 2012
Misses 263 263 |
| spent = bigchain.get_spent(input_txid, input_.fulfills.output, | ||
| current_transactions) | ||
| if spent and spent.id != self.id: | ||
| if spent: |
There was a problem hiding this comment.
This is a more strict condition. IMHO there is no need to match ids i.e. if there a txn which spends the current input then regardless of the transaction id it should be considered a double spend.
There was a problem hiding this comment.
Yes, because we also check to see if the current transaction (self) has more than one input trying to spend the same output (below). Actually, the line:
raise DoubleSpend('tx "{}" spends inputs twice'.format(self.id))
should say
raise DoubleSpend('tx "{}" spends the same output more than once'.format(self.id))
Maybe make that change in this PR as well, because it helps to make the logic more clear.
Solution: Transaction.validate should raise exception DoubleSpend if the given transaction is already a part of the database
86509fb to
22b462a
Compare
ttmc
left a comment
There was a problem hiding this comment.
I suggested a small change in a related line of code.
Solution: The exception message should state that the double spend is because of spending the same input more than once in the transaction
|
I don't know what Codecov is upset about. I'm going to ignore it. |
Solution: Transaction.validate should raise exception DoubleSpend if the given that the transaction is already a part of the database.