Merge pull request #1101 from bigchaindb/revert-duplicate-asset-id

Revert "duplicate asset ID" and apply "get_txids_filtered" interface.

bigchaindb/backend/mongodb/query.py

@@ -1,12 +1,14 @@
 """Query implementation for MongoDB"""
 
 from time import time
+from itertools import chain
 
 from pymongo import ReturnDocument
 from pymongo import errors
 
 from bigchaindb import backend
 from bigchaindb.common.exceptions import CyclicBlockchainError
+from bigchaindb.common.transaction import Transaction
 from bigchaindb.backend.utils import module_dispatch_registrar
 from bigchaindb.backend.mongodb.connection import MongoDBConnection
 
@@ -82,6 +84,43 @@ def get_blocks_status_from_transaction(conn, transaction_id):
                   projection=['id', 'block.voters'])
 
 
+@register_query(MongoDBConnection)
+def get_txids_filtered(conn, asset_id, operation=None):
+    parts = []
+
+    if operation in (Transaction.CREATE, None):
+        # get the txid of the create transaction for asset_id
+        cursor = conn.db['bigchain'].aggregate([
+            {'$match': {
+                'block.transactions.id': asset_id,
+                'block.transactions.operation': 'CREATE'
+            }},
+            {'$unwind': '$block.transactions'},
+            {'$match': {
+                'block.transactions.id': asset_id,
+                'block.transactions.operation': 'CREATE'
+            }},
+            {'$project': {'block.transactions.id': True}}
+        ])
+        parts.append(elem['block']['transactions']['id'] for elem in cursor)
+
+    if operation in (Transaction.TRANSFER, None):
+        # get txids of transfer transaction with asset_id
+        cursor = conn.db['bigchain'].aggregate([
+            {'$match': {
+                'block.transactions.asset.id': asset_id
+            }},
+            {'$unwind': '$block.transactions'},
+            {'$match': {
+                'block.transactions.asset.id': asset_id
+            }},
+            {'$project': {'block.transactions.id': True}}
+        ])
+        parts.append(elem['block']['transactions']['id'] for elem in cursor)
+
+    return chain(*parts)
+
+
 @register_query(MongoDBConnection)
 def get_asset_by_id(conn, asset_id):
     cursor = conn.db['bigchain'].aggregate([
@@ -234,19 +273,3 @@ def get_unvoted_blocks(conn, node_pubkey):
             'votes': False, '_id': False
         }}
     ])
-
-
-@register_query(MongoDBConnection)
-def get_txids_filtered(conn, asset_id, operation=None):
-    match = {'block.transactions.asset.id': asset_id}
-
-    if operation:
-        match['block.transactions.operation'] = operation
-
-    cursor = conn.db['bigchain'].aggregate([
-        {'$match': match},
-        {'$unwind': '$block.transactions'},
-        {'$match': match},
-        {'$project': {'block.transactions.id': True}}
-    ])
-    return (r['block']['transactions']['id'] for r in cursor)

bigchaindb/backend/rethinkdb/query.py

@@ -1,9 +1,11 @@
+from itertools import chain
 from time import time
 
 import rethinkdb as r
 
 from bigchaindb import backend, utils
 from bigchaindb.common import exceptions
+from bigchaindb.common.transaction import Transaction
 from bigchaindb.backend.utils import module_dispatch_registrar
 from bigchaindb.backend.rethinkdb.connection import RethinkDBConnection
 
@@ -71,6 +73,30 @@ def get_blocks_status_from_transaction(connection, transaction_id):
             .pluck('votes', 'id', {'block': ['voters']}))
 
 
+@register_query(RethinkDBConnection)
+def get_txids_filtered(connection, asset_id, operation=None):
+    # here we only want to return the transaction ids since later on when
+    # we are going to retrieve the transaction with status validation
+
+    parts = []
+
+    if operation in (Transaction.CREATE, None):
+        # First find the asset's CREATE transaction
+        parts.append(connection.run(
+            _get_asset_create_tx_query(asset_id).get_field('id')))
+
+    if operation in (Transaction.TRANSFER, None):
+        # Then find any TRANSFER transactions related to the asset
+        parts.append(connection.run(
+            r.table('bigchain')
+            .get_all(asset_id, index='asset_id')
+            .concat_map(lambda block: block['block']['transactions'])
+            .filter(lambda transaction: transaction['asset']['id'] == asset_id)
+            .get_field('id')))
+
+    return chain(*parts)
+
+
 @register_query(RethinkDBConnection)
 def get_asset_by_id(connection, asset_id):
     return connection.run(_get_asset_create_tx_query(asset_id).pluck('asset'))
@@ -233,20 +259,3 @@ def get_unvoted_blocks(connection, node_pubkey):
     #        database level. Solving issue #444 can help untangling the situation
     unvoted_blocks = filter(lambda block: not utils.is_genesis_block(block), unvoted)
     return unvoted_blocks
-
-
-@register_query(RethinkDBConnection)
-def get_txids_filtered(connection, asset_id, operation=None):
-    # here we only want to return the transaction ids since later on when
-    # we are going to retrieve the transaction with status validation
-
-    tx_filter = r.row['asset']['id'] == asset_id
-    if operation:
-        tx_filter &= r.row['operation'] == operation
-
-    return connection.run(
-        r.table('bigchain')
-         .get_all(asset_id, index='asset_id')
-         .concat_map(lambda block: block['block']['transactions'])
-         .filter(tx_filter)
-         .get_field('id'))

bigchaindb/common/schema/transaction.yaml

@@ -103,8 +103,8 @@ definitions:
     description: |
         Description of the asset being transacted. In the case of a ``TRANSFER``
         transaction, this field contains only the ID of asset. In the case
-        of a ``CREATE`` transaction, this field contains the user-defined
+        of a ``CREATE`` transaction, this field contains only the user-defined
-        payload and the asset ID (duplicated from the Transaction ID).
+        payload.
     additionalProperties: false
     properties:
       id:

bigchaindb/common/transaction.py

@@ -444,7 +444,6 @@ class Transaction(object):
                 asset is not None and not (isinstance(asset, dict) and 'data' in asset)):
             raise TypeError(('`asset` must be None or a dict holding a `data` '
                              " property instance for '{}' Transactions".format(operation)))
-            asset.pop('id', None)  # Remove duplicated asset ID if there is one
         elif (operation == Transaction.TRANSFER and
                 not (isinstance(asset, dict) and 'id' in asset)):
             raise TypeError(('`asset` must be a dict holding an `id` property '
@@ -927,11 +926,9 @@ class Transaction(object):
 
         tx_no_signatures = Transaction._remove_signatures(tx)
         tx_serialized = Transaction._to_str(tx_no_signatures)
-        tx['id'] = Transaction._to_hash(tx_serialized)
+        tx_id = Transaction._to_hash(tx_serialized)
-        if self.operation == Transaction.CREATE:
+
-            # Duplicate asset into asset for consistency with TRANSFER
+        tx['id'] = tx_id
-            # transactions
-            tx['asset']['id'] = tx['id']
         return tx
 
     @staticmethod
@@ -955,9 +952,6 @@ class Transaction(object):
             #       case could yield incorrect signatures. This is why we only
             #       set it to `None` if it's set in the dict.
             input_['fulfillment'] = None
-        # Pop duplicated asset_id from CREATE tx
-        if tx_dict['operation'] == Transaction.CREATE:
-            tx_dict['asset'].pop('id', None)
         return tx_dict
 
     @staticmethod
@@ -1037,10 +1031,6 @@ class Transaction(object):
                        "the hash of its body, i.e. it's not valid.")
             raise InvalidHash(err_msg.format(proposed_tx_id))
 
-        if tx_body.get('operation') == Transaction.CREATE:
-            if proposed_tx_id != tx_body['asset'].get('id'):
-                raise InvalidHash('CREATE tx has wrong asset_id')
-
     @classmethod
     def from_dict(cls, tx):
         """Transforms a Python dictionary to a Transaction object.

docs/server/source/data-models/asset-model.md

@@ -1,11 +1,10 @@
 # The Digital Asset Model
 
-The asset ID is the same as the ID of the CREATE transaction that defined the asset.
+To avoid redundant data in transactions, the digital asset model is different for `CREATE` and `TRANSFER` transactions.
 
-In the case of a CREATE transaction, the transaction ID is duplicated into the asset object for clarity and consistency in the database. The CREATE transaction also contains a user definable payload to describe the asset:
+A digital asset's properties are defined in a `CREATE` transaction with the following model:
 ```json
 {
-    "id": "<same as transaction ID (sha3-256 hash)>",
     "data": "<json document>"
 }
 ```

tests/common/test_transaction.py

@@ -300,7 +300,6 @@ def test_transaction_serialization(user_input, user_output, data):
         'operation': Transaction.CREATE,
         'metadata': None,
         'asset': {
-            'id': tx_id,
             'data': data,
         }
     }
@@ -308,7 +307,7 @@ def test_transaction_serialization(user_input, user_output, data):
     tx = Transaction(Transaction.CREATE, {'data': data}, [user_input],
                      [user_output])
     tx_dict = tx.to_dict()
-    tx_dict['id'] = tx_dict['asset']['id'] = tx_id
+    tx_dict['id'] = tx_id
 
     assert tx_dict == expected
 
@@ -335,7 +334,6 @@ def test_transaction_deserialization(user_input, user_output, data):
     }
     tx_no_signatures = Transaction._remove_signatures(tx)
     tx['id'] = Transaction._to_hash(Transaction._to_str(tx_no_signatures))
-    tx['asset']['id'] = tx['id']
     tx = Transaction.from_dict(tx)
 
     assert tx == expected
@@ -691,7 +689,6 @@ def test_create_create_transaction_single_io(user_output, user_pub, data):
     tx_dict = tx.to_dict()
     tx_dict['inputs'][0]['fulfillment'] = None
     tx_dict.pop('id')
-    tx_dict['asset'].pop('id')
 
     assert tx_dict == expected
 
@@ -775,7 +772,6 @@ def test_create_create_transaction_threshold(user_pub, user2_pub, user3_pub,
                             metadata=data, asset=data)
     tx_dict = tx.to_dict()
     tx_dict.pop('id')
-    tx_dict['asset'].pop('id')
     tx_dict['inputs'][0]['fulfillment'] = None
 
     assert tx_dict == expected
@@ -989,25 +985,3 @@ def test_validate_version(utx):
     utx.version = '1.0.0'
     with raises(SchemaValidationError):
         validate_transaction_model(utx)
-
-
-def test_create_tx_has_asset_id(tx):
-    tx = tx.to_dict()
-    assert tx['id'] == tx['asset']['id']
-
-
-def test_create_tx_validates_asset_id(tx):
-    from bigchaindb.common.transaction import Transaction
-    from bigchaindb.common.exceptions import InvalidHash
-
-    tx = tx.to_dict()
-
-    # Test fails with wrong asset_id
-    tx['asset']['id'] = tx['asset']['id'][::-1]
-    with raises(InvalidHash):
-        Transaction.from_dict(tx)
-
-    # Test fails with no asset_id
-    tx['asset'].pop('id')
-    with raises(InvalidHash):
-        Transaction.from_dict(tx)