From adb579ac0a29ac857eed5886cabfed17b1121994 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 26 Jan 2017 13:52:09 +0100 Subject: [PATCH] Revert "duplicate asset ID" and apply "get_txids_filtered" interface. --- bigchaindb/backend/mongodb/query.py | 55 +++++++++++++------ bigchaindb/backend/rethinkdb/query.py | 43 +++++++++------ bigchaindb/common/schema/transaction.yaml | 4 +- bigchaindb/common/transaction.py | 16 +----- docs/server/source/data-models/asset-model.md | 5 +- tests/common/test_transaction.py | 28 +--------- 6 files changed, 73 insertions(+), 78 deletions(-) diff --git a/bigchaindb/backend/mongodb/query.py b/bigchaindb/backend/mongodb/query.py index c4e3cdc8..d7ee6afc 100644 --- a/bigchaindb/backend/mongodb/query.py +++ b/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) diff --git a/bigchaindb/backend/rethinkdb/query.py b/bigchaindb/backend/rethinkdb/query.py index fd0bdcb3..aa7c3be6 100644 --- a/bigchaindb/backend/rethinkdb/query.py +++ b/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')) diff --git a/bigchaindb/common/schema/transaction.yaml b/bigchaindb/common/schema/transaction.yaml index a0edd1e3..86e5947b 100644 --- a/bigchaindb/common/schema/transaction.yaml +++ b/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 - payload and the asset ID (duplicated from the Transaction ID). + of a ``CREATE`` transaction, this field contains only the user-defined + payload. additionalProperties: false properties: id: diff --git a/bigchaindb/common/transaction.py b/bigchaindb/common/transaction.py index 0e6d84f2..65b12eed 100644 --- a/bigchaindb/common/transaction.py +++ b/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) - if self.operation == Transaction.CREATE: - # Duplicate asset into asset for consistency with TRANSFER - # transactions - tx['asset']['id'] = tx['id'] + tx_id = Transaction._to_hash(tx_serialized) + + tx['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. diff --git a/docs/server/source/data-models/asset-model.md b/docs/server/source/data-models/asset-model.md index 16188400..312c6765 100644 --- a/docs/server/source/data-models/asset-model.md +++ b/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": "", "data": "" } ``` diff --git a/tests/common/test_transaction.py b/tests/common/test_transaction.py index 7e70939d..a2782583 100644 --- a/tests/common/test_transaction.py +++ b/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)