Merge pull request #1377 from bigchaindb/bug/1343/double-spent-critical-on-voting

Bug/1343/double spent critical on voting

bigchaindb/core.py

@@ -324,43 +324,57 @@ class Bigchain(object):
     def get_spent(self, txid, output):
         """Check if a `txid` was already used as an input.
 
-        A transaction can be used as an input for another transaction. Bigchain needs to make sure that a
-        given `txid` is only used once.
+        A transaction can be used as an input for another transaction. Bigchain
+        needs to make sure that a given `(txid, output)` is only used once.
+
+        This method will check if the `(txid, output)` has already been
+        spent in a transaction that is in either the `VALID`, `UNDECIDED` or
+        `BACKLOG` state.
 
         Args:
             txid (str): The id of the transaction
             output (num): the index of the output in the respective transaction
 
         Returns:
-            The transaction (Transaction) that used the `txid` as an input else
-            `None`
+            The transaction (Transaction) that used the `(txid, output)` as an
+            input else `None`
+
+        Raises:
+            CriticalDoubleSpend: If the given `(txid, output)` was spent in
+            more than one valid transaction.
         """
         # checks if an input was already spent
         # checks if the bigchain has any transaction with input {'txid': ...,
         # 'output': ...}
-        transactions = list(backend.query.get_spent(self.connection, txid, output))
+        transactions = list(backend.query.get_spent(self.connection, txid,
+                                                    output))
 
         # a transaction_id should have been spent at most one time
-        if transactions:
-            # determine if these valid transactions appear in more than one valid block
-            num_valid_transactions = 0
-            for transaction in transactions:
-                # ignore invalid blocks
-                # FIXME: Isn't there a faster solution than doing I/O again?
-                if self.get_transaction(transaction['id']):
-                    num_valid_transactions += 1
-                if num_valid_transactions > 1:
-                    raise core_exceptions.CriticalDoubleSpend(
-                        '`{}` was spent more than once. There is a problem'
-                        ' with the chain'.format(txid))
+        # determine if these valid transactions appear in more than one valid
+        # block
+        num_valid_transactions = 0
+        non_invalid_transactions = []
+        for transaction in transactions:
+            # ignore transactions in invalid blocks
+            # FIXME: Isn't there a faster solution than doing I/O again?
+            _, status = self.get_transaction(transaction['id'],
+                                             include_status=True)
+            if status == self.TX_VALID:
+                num_valid_transactions += 1
+            # `txid` can only have been spent in at most on valid block.
+            if num_valid_transactions > 1:
+                raise core_exceptions.CriticalDoubleSpend(
+                    '`{}` was spent more than once. There is a problem'
+                    ' with the chain'.format(txid))
+            # if its not and invalid transaction
+            if status is not None:
+                non_invalid_transactions.append(transaction)
 
-            if num_valid_transactions:
-                return Transaction.from_dict(transactions[0])
-            else:
-                # all queried transactions were invalid
-                return None
-        else:
-            return None
+        if non_invalid_transactions:
+            return Transaction.from_dict(non_invalid_transactions[0])
+
+        # Either no transaction was returned spending the `(txid, output)` as
+        # input or the returned transactions are not valid.
 
     def get_outputs(self, owner):
         """Retrieve a list of links to transaction outputs for a given public

bigchaindb/models.py

@@ -187,6 +187,11 @@ class Block(object):
         if not self.is_signature_valid():
             raise InvalidSignature('Invalid block signature')
 
+        # Check that the block contains no duplicated transactions
+        txids = [tx.id for tx in self.transactions]
+        if len(txids) != len(set(txids)):
+            raise DuplicateTransaction('Block has duplicate transaction')
+
     def _validate_block_transactions(self, bigchain):
         """Validate Block transactions.
 
@@ -196,10 +201,6 @@ class Block(object):
         Raises:
             ValidationError: If an invalid transaction is found
         """
-        txids = [tx.id for tx in self.transactions]
-        if len(txids) != len(set(txids)):
-            raise DuplicateTransaction('Block has duplicate transaction')
-
         for tx in self.transactions:
             # If a transaction is not valid, `validate_transactions` will
             # throw an an exception and block validation will be canceled.

tests/pipelines/test_vote.py

@@ -111,6 +111,18 @@ def test_validate_block_with_invalid_id(b):
     assert invalid_dummy_tx == [vote_obj.invalid_dummy_tx]
 
 
+@pytest.mark.genesis
+def test_validate_block_with_duplicated_transactions(b):
+    from bigchaindb.pipelines import vote
+
+    tx = dummy_tx(b)
+    block = b.create_block([tx, tx]).to_dict()
+
+    vote_obj = vote.Vote()
+    block_id, invalid_dummy_tx = vote_obj.validate_block(block)
+    assert invalid_dummy_tx == [vote_obj.invalid_dummy_tx]
+
+
 @pytest.mark.genesis
 def test_validate_block_with_invalid_signature(b):
     from bigchaindb.pipelines import vote

tests/test_models.py

@@ -152,4 +152,4 @@ class TestBlockModel(object):
         tx = Transaction.create([b.me], [([b.me], 1)])
         block = b.create_block([tx, tx])
         with raises(DuplicateTransaction):
-            block._validate_block_transactions(b)
+            block._validate_block(b)