Merge pull request #1492 from bigchaindb/pipeline-fast-transaction

Transaction schema validation pipeline tweaks

bigchaindb/models.py

@@ -267,11 +267,12 @@ class Block(object):
             return False
 
     @classmethod
-    def from_dict(cls, block_body):
+    def from_dict(cls, block_body, tx_construct=Transaction.from_dict):
         """Transform a Python dictionary to a Block object.
 
         Args:
             block_body (dict): A block dictionary to be transformed.
+            tx_construct (functions): Function to instantiate Transaction instance
 
         Returns:
             :class:`~Block`
@@ -288,8 +289,7 @@ class Block(object):
         if block_id != block_body['id']:
             raise InvalidHash()
 
-        transactions = [Transaction.from_dict(tx) for tx
+        transactions = [tx_construct(tx) for tx in block['transactions']]
-                        in block['transactions']]
 
         signature = block_body.get('signature')
 
@@ -328,7 +328,7 @@ class Block(object):
         }
 
     @classmethod
-    def from_db(cls, bigchain, block_dict):
+    def from_db(cls, bigchain, block_dict, from_dict_kwargs=None):
         """
         Helper method that reconstructs a block_dict that was returned from
         the database. It checks what asset_ids to retrieve, retrieves the
@@ -339,6 +339,7 @@ class Block(object):
                 used to perform database queries.
             block_dict(:obj:`dict`): The block dict as returned from the
                 database.
+            from_dict_kwargs (:obj:`dict`): additional kwargs to pass to from_dict
 
         Returns:
             :class:`~Block`
@@ -347,7 +348,8 @@ class Block(object):
         asset_ids = cls.get_asset_ids(block_dict)
         assets = bigchain.get_assets(asset_ids)
         block_dict = cls.couple_assets(block_dict, assets)
-        return cls.from_dict(block_dict)
+        kwargs = from_dict_kwargs or {}
+        return cls.from_dict(block_dict, **kwargs)
 
     def decouple_assets(self):
         """
@@ -419,3 +421,22 @@ class Block(object):
 
     def to_str(self):
         return serialize(self.to_dict())
+
+
+class FastTransaction:
+    """
+    A minimal wrapper around a transaction dictionary. This is useful for
+    when validation is not required but a routine expects something that looks
+    like a transaction, for example during block creation.
+
+    Note: immutability could also be provided
+    """
+    def __init__(self, tx_dict):
+        self.data = tx_dict
+
+    @property
+    def id(self):
+        return self.data['id']
+
+    def to_dict(self):
+        return self.data

bigchaindb/pipelines/vote.py

@@ -10,8 +10,8 @@ from collections import Counter
 
 from multipipes import Pipeline, Node
 
-from bigchaindb import Bigchain, backend
+from bigchaindb import backend, Bigchain
-from bigchaindb.models import Transaction, Block
+from bigchaindb.models import Transaction, Block, FastTransaction
 from bigchaindb.common import exceptions
 
 
@@ -41,20 +41,23 @@ class Vote:
         self.counters = Counter()
         self.validity = {}
 
-        self.invalid_dummy_tx = Transaction.create([self.bigchain.me],
+        dummy_tx = Transaction.create([self.bigchain.me],
-                                                   [([self.bigchain.me], 1)])
+                                      [([self.bigchain.me], 1)]).to_dict()
+        self.invalid_dummy_tx = dummy_tx
 
-    def validate_block(self, block):
+    def validate_block(self, block_dict):
-        if not self.bigchain.has_previous_vote(block['id']):
+        if not self.bigchain.has_previous_vote(block_dict['id']):
             try:
-                block = Block.from_db(self.bigchain, block)
+                block = Block.from_db(self.bigchain, block_dict, from_dict_kwargs={
+                    'tx_construct': FastTransaction
+                })
             except (exceptions.InvalidHash):
                 # XXX: if a block is invalid we should skip the `validate_tx`
                 # step, but since we are in a pipeline we cannot just jump to
                 # another function. Hackish solution: generate an invalid
                 # transaction and propagate it to the next steps of the
                 # pipeline.
-                return block['id'], [self.invalid_dummy_tx]
+                return block_dict['id'], [self.invalid_dummy_tx]
             try:
                 block._validate_block(self.bigchain)
             except exceptions.ValidationError:
@@ -64,14 +67,14 @@ class Vote:
                 # transaction and propagate it to the next steps of the
                 # pipeline.
                 return block.id, [self.invalid_dummy_tx]
-            return block.id, block.transactions
+            return block.id, block_dict['block']['transactions']
 
     def ungroup(self, block_id, transactions):
         """Given a block, ungroup the transactions in it.
 
         Args:
             block_id (str): the id of the block in progress.
-            transactions (list(Transaction)): transactions of the block in
+            transactions (list(dict)): transactions of the block in
                 progress.
 
         Returns:
@@ -84,12 +87,12 @@ class Vote:
         for tx in transactions:
             yield tx, block_id, num_tx
 
-    def validate_tx(self, tx, block_id, num_tx):
+    def validate_tx(self, tx_dict, block_id, num_tx):
         """Validate a transaction. Transaction must also not be in any VALID
            block.
 
         Args:
-            tx (dict): the transaction to validate
+            tx_dict (dict): the transaction to validate
             block_id (str): the id of block containing the transaction
             num_tx (int): the total number of transactions to process
 
@@ -97,16 +100,17 @@ class Vote:
             Three values are returned, the validity of the transaction,
             ``block_id``, ``num_tx``.
         """
-        new = self.bigchain.is_new_transaction(tx.id, exclude_block_id=block_id)
-        if not new:
-            return False, block_id, num_tx
 
         try:
+            tx = Transaction.from_dict(tx_dict)
+            new = self.bigchain.is_new_transaction(tx.id, exclude_block_id=block_id)
+            if not new:
+                raise exceptions.ValidationError('Tx already exists, %s', tx.id)
             tx.validate(self.bigchain)
             valid = True
         except exceptions.ValidationError as e:
-            logger.warning('Invalid tx: %s', e)
             valid = False
+            logger.warning('Invalid tx: %s', e)
 
         return valid, block_id, num_tx
 

tests/pipelines/test_vote.py

@@ -94,7 +94,7 @@ def test_vote_validate_block(b):
     validation = vote_obj.validate_block(block_dict)
     assert validation[0] == block.id
     for tx1, tx2 in zip(validation[1], block.transactions):
-        assert tx1 == tx2
+        assert tx1 == tx2.to_dict()
 
     block = b.create_block([tx])
     # NOTE: Setting a blocks signature to `None` invalidates it.
@@ -152,7 +152,7 @@ def test_vote_validate_transaction(b):
     from bigchaindb.pipelines import vote
     from bigchaindb.common.exceptions import ValidationError
 
-    tx = dummy_tx(b)
+    tx = dummy_tx(b).to_dict()
     vote_obj = vote.Vote()
     validation = vote_obj.validate_tx(tx, 123, 1)
     assert validation == (True, 123, 1)
@@ -175,15 +175,13 @@ def test_vote_accumulates_transactions(b):
 
     vote_obj = vote.Vote()
 
-    for _ in range(10):
+    tx = dummy_tx(b)
-        tx = dummy_tx(b)
 
-    tx = tx
+    validation = vote_obj.validate_tx(tx.to_dict(), 123, 1)
-    validation = vote_obj.validate_tx(tx, 123, 1)
     assert validation == (True, 123, 1)
 
     tx.inputs[0].fulfillment.signature = None
-    validation = vote_obj.validate_tx(tx, 456, 10)
+    validation = vote_obj.validate_tx(tx.to_dict(), 456, 10)
     assert validation == (False, 456, 10)
 
 
@@ -195,16 +193,17 @@ def test_valid_block_voting_sequential(b, genesis_block, monkeypatch):
 
     monkeypatch.setattr('time.time', lambda: 1111111111)
     vote_obj = vote.Vote()
-    block = dummy_block(b)
+    block = dummy_block(b).to_dict()
+    txs = block['block']['transactions']
 
-    for tx, block_id, num_tx in vote_obj.ungroup(block.id, block.transactions):
+    for tx, block_id, num_tx in vote_obj.ungroup(block['id'], txs):
         last_vote = vote_obj.vote(*vote_obj.validate_tx(tx, block_id, num_tx))
 
     vote_obj.write_vote(last_vote)
     vote_rs = query.get_votes_by_block_id_and_voter(b.connection, block_id, b.me)
     vote_doc = vote_rs.next()
 
-    assert vote_doc['vote'] == {'voting_for_block': block.id,
+    assert vote_doc['vote'] == {'voting_for_block': block['id'],
                                 'previous_block': genesis_block.id,
                                 'is_block_valid': True,
                                 'invalid_reason': None,
@@ -578,9 +577,20 @@ def test_vote_no_double_inclusion(b):
 
     tx = dummy_tx(b)
     block = b.create_block([tx])
-    r = vote.Vote().validate_tx(tx, block.id, 1)
+    r = vote.Vote().validate_tx(tx.to_dict(), block.id, 1)
     assert r == (True, block.id, 1)
 
     b.write_block(block)
-    r = vote.Vote().validate_tx(tx, 'other_block_id', 1)
+    r = vote.Vote().validate_tx(tx.to_dict(), 'other_block_id', 1)
     assert r == (False, 'other_block_id', 1)
+
+
+@pytest.mark.genesis
+def test_duplicate_transaction(signed_create_tx):
+    from bigchaindb.pipelines import vote
+
+    with patch('bigchaindb.core.Bigchain.is_new_transaction') as is_new:
+        is_new.return_value = False
+        res = vote.Vote().validate_tx(signed_create_tx.to_dict(), 'a', 1)
+    assert res == (False, 'a', 1)
+    assert is_new.call_count == 1