Fixed how validate_transaction handles double spends.

Create tests.
Fixed some flake8 warnings

bigchaindb/core.py

@@ -269,30 +269,18 @@ class Bigchain(object):
             txid (str): transaction id.
 
         Returns:
-            A tuple with the block id and the transactions that used the `txid` as an input if
+            The transaction that used the `txid` as an input if it exists else it returns `None`
-            it exists else it returns `None`
 
         """
         # checks if an input was already spent
         # checks if the bigchain has any transaction with input `transaction_id`
-        response = r.table('bigchain').group('id')\
+        response = r.table('bigchain').concat_map(lambda doc: doc['block']['transactions'])\
-            .concat_map(lambda doc: doc['block']['transactions'])\
             .filter(lambda transaction: transaction['transaction']['input'] == txid).run(self.conn)
 
-        # the query returns a dictionary in which keys are block numbers and values are list of transactions
-        # with that using that input inside the block. For it to be correct:
-            # - There should be at most one block with transactions using that input
-            # - There should be at most one transaction with that input
-
-        # flatten to dictionary into a list of [(block['id'], tx), ...]
-        transactions = []
-        for k, v in response.items():
-            for tx in v:
-                transactions.append((k, tx))
-
         # a transaction_id should have been spent at most one time
+        transactions = list(response)
         if transactions:
-            if len(transactions) > 1:
+            if len(transactions) != 1:
                 raise Exception('`{}` was spent more then once. There is a problem with the chain'.format(
                     txid))
             else:
@@ -364,11 +352,12 @@ class Bigchain(object):
                 raise exceptions.TransactionOwnerError('current_owner `{}` does not own the input `{}`'.format(
                     transaction['transaction']['current_owner'], transaction['transaction']['input']))
 
-            # check if the input was already spent
+            # check if the input was already spent by a transaction other then this one.
             spent = self.get_spent(tx_input['id'])
             if spent:
-                raise exceptions.DoubleSpend('input `{}` was already spent'.format(
+                if spent['id'] != transaction['id']:
-                    transaction['transaction']['input']))
+                    raise exceptions.DoubleSpend('input `{}` was already spent'.format(
+                        transaction['transaction']['input']))
 
         # Check hash of the transaction
         calculated_hash = hash_data(self.serialize(transaction['transaction']))
@@ -508,13 +497,11 @@ class Bigchain(object):
         # 2. create the block with one transaction
         # 3. write the block to the bigchain
 
-
         blocks_count = r.table('bigchain').count().run(self.conn)
 
         if blocks_count:
             raise GenesisBlockAlreadyExistsError('Cannot create the Genesis block')
 
-
         payload = {'message': 'Hello World from the Bigchain'}
         transaction = self.create_transaction(self.me, self.me, None, 'GENESIS', payload=payload)
         transaction_signed = self.sign_transaction(transaction, self.me_private)

tests/db/test_bigchain_api.py

@@ -45,6 +45,7 @@ def inputs(user_public_key):
 def test_remove_unclosed_sockets():
     pass
 
+
 class TestBigchainApi(object):
 
     def test_create_transaction(self, b):
@@ -58,7 +59,6 @@ class TestBigchainApi(object):
         with pytest.raises(TypeError):
             b.create_transaction('a', 'b', 'c', 'd', payload=[])
 
-
     def test_transaction_hash(self, b):
         payload = {'cats': 'are awesome'}
         tx = b.create_transaction('a', 'b', 'c', 'd', payload)
@@ -166,8 +166,8 @@ class TestBigchainApi(object):
             b.create_genesis_block()
 
         genesis_blocks = list(r.table('bigchain')
-                        .filter(r.row['block_number'] == 0)
+                              .filter(r.row['block_number'] == 0)
-                        .run(b.conn))
+                              .run(b.conn))
 
         assert len(genesis_blocks) == 1
 
@@ -349,6 +349,24 @@ class TestTransactionValidation(object):
         assert tx_valid_signed == b.validate_transaction(tx_valid_signed)
         assert tx_valid_signed == b.is_valid_transaction(tx_valid_signed)
 
+    @pytest.mark.usefixtures('inputs')
+    def test_valid_non_create_transaction_after_block_creation(self, b, user_public_key, user_private_key):
+        input_valid = b.get_owned_ids(user_public_key).pop()
+        tx_valid = b.create_transaction(user_public_key, 'b', input_valid, 'd')
+
+        tx_valid_signed = b.sign_transaction(tx_valid, user_private_key)
+        assert tx_valid_signed == b.validate_transaction(tx_valid_signed)
+        assert tx_valid_signed == b.is_valid_transaction(tx_valid_signed)
+
+        # create block
+        block = b.create_block([tx_valid_signed])
+        assert b.is_valid_block(block)
+        b.write_block(block, durability='hard')
+
+        # check that the transaction is still valid after being written to the bigchain
+        assert tx_valid_signed == b.validate_transaction(tx_valid_signed)
+        assert tx_valid_signed == b.is_valid_transaction(tx_valid_signed)
+
 
 class TestBlockValidation(object):
 
@@ -357,7 +375,7 @@ class TestBlockValidation(object):
 
         # change block hash
         block.update({'id': 'abc'})
-        with pytest.raises(exceptions.InvalidHash) as excinfo:
+        with pytest.raises(exceptions.InvalidHash):
             b.validate_block(block)
 
     @pytest.mark.skipif(reason='Separated tx validation from block creation.')
@@ -368,7 +386,6 @@ class TestBlockValidation(object):
         tx_invalid = b.create_transaction('a', 'b', valid_input, 'c')
 
         block = b.create_block([tx_invalid])
-        assert invalid_transactions == [tx_invalid]
 
         # create a block with invalid transactions
         block = {
@@ -520,7 +537,6 @@ class TestBigchainVoter(object):
         time.sleep(1)
         voter.kill()
 
-
         # retrive block from bigchain
         bigchain_block = r.table('bigchain').get(block['id']).run(b.conn)
 
@@ -765,4 +781,3 @@ class TestBigchainBlock(object):
 
     def test_duplicated_transactions(self):
         pytest.skip('We may have duplicates in the initial_results and changefeed')
-

tests/db/test_voter.py

@@ -3,7 +3,6 @@ import time
 import rethinkdb as r
 import multiprocessing as mp
 
-from bigchaindb import Bigchain
 from bigchaindb.voter import Voter, BlockStream
 from bigchaindb.crypto import PublicKey
 
@@ -35,7 +34,6 @@ class TestBigchainVoter(object):
                        .order_by(r.asc((r.row['block']['timestamp'])))
                        .run(b.conn))
 
-
         # validate vote
         assert len(blocks[1]['votes']) == 1
         vote = blocks[1]['votes'][0]
@@ -78,7 +76,6 @@ class TestBigchainVoter(object):
                        .order_by(r.asc((r.row['block']['timestamp'])))
                        .run(b.conn))
 
-
         # validate vote
         assert len(blocks[1]['votes']) == 1
         vote = blocks[1]['votes'][0]
@@ -93,7 +90,7 @@ class TestBigchainVoter(object):
     def test_valid_block_voting_with_transfer_transactions(self, b):
         q_new_block = mp.Queue()
 
-        genesis = b.create_genesis_block()
+        b.create_genesis_block()
 
         # create a `CREATE` transaction
         test_user_priv, test_user_pub = b.generate_keys()
@@ -121,11 +118,9 @@ class TestBigchainVoter(object):
                        .order_by(r.asc((r.row['block']['timestamp'])))
                        .run(b.conn))
 
-
         # validate vote
         assert len(blocks[1]['votes']) == 1
 
-        
         # create a `TRANSFER` transaction
         test_user2_priv, test_user2_pub = b.generate_keys()
         tx2 = b.create_transaction(test_user_pub, test_user2_pub, tx['id'], 'TRANSFER')
@@ -152,7 +147,6 @@ class TestBigchainVoter(object):
                        .order_by(r.asc((r.row['block']['timestamp'])))
                        .run(b.conn))
 
-
         # validate vote
         assert len(blocks[2]['votes']) == 1
 
@@ -182,7 +176,6 @@ class TestBigchainVoter(object):
         assert not b.is_valid_block(block)
         b.write_block(block, durability='hard')
 
-
         # vote
         voter.start()
         time.sleep(1)
@@ -283,7 +276,6 @@ class TestBigchainVoter(object):
         time.sleep(1)
         voter.kill()
 
-
         # retrive blocks from bigchain
         blocks = list(r.table('bigchain')
                        .order_by(r.asc((r.row['block']['timestamp'])))
@@ -303,7 +295,6 @@ class TestBigchainVoter(object):
         pass
 
 
-
 class TestBlockStream(object):
 
     def test_if_federation_size_is_greater_than_one_ignore_past_blocks(self, b):
@@ -335,7 +326,6 @@ class TestBlockStream(object):
         assert bs.get() == block_1
         assert bs.get() == block_2
 
-
     def test_if_old_blocks_get_should_return_old_block_first(self, b):
         # create two blocks
         block_1 = b.create_block([])