Merge pull request #1257 from bigchaindb/tx-structure

Schemas for CREATE and TRANSFER transactions

bigchaindb/common/schema/__init__.py

@@ -28,7 +28,9 @@ def _load_schema(name):
     return path, schema
 
 
-TX_SCHEMA_PATH, TX_SCHEMA = _load_schema('transaction')
+TX_SCHEMA_PATH, TX_SCHEMA_COMMON = _load_schema('transaction')
+_, TX_SCHEMA_CREATE = _load_schema('transaction_create')
+_, TX_SCHEMA_TRANSFER = _load_schema('transaction_transfer')
 VOTE_SCHEMA_PATH, VOTE_SCHEMA = _load_schema('vote')
 
 
@@ -41,8 +43,17 @@ def _validate_schema(schema, body):
 
 
 def validate_transaction_schema(tx):
-    """ Validate a transaction dict """
+    """
-    _validate_schema(TX_SCHEMA, tx)
+    Validate a transaction dict.
+
+    TX_SCHEMA_COMMON contains properties that are common to all types of
+    transaction. TX_SCHEMA_[TRANSFER|CREATE] add additional constraints on top.
+    """
+    _validate_schema(TX_SCHEMA_COMMON, tx)
+    if tx['operation'] == 'TRANSFER':
+        _validate_schema(TX_SCHEMA_TRANSFER, tx)
+    else:
+        _validate_schema(TX_SCHEMA_CREATE, tx)
 
 
 def validate_vote_schema(vote):

bigchaindb/common/schema/transaction_create.yaml

@@ -0,0 +1,28 @@
+---
+"$schema": "http://json-schema.org/draft-04/schema#"
+type: object
+title: Transaction Schema - CREATE/GENESIS specific constraints
+required:
+- asset
+- inputs
+properties:
+  asset:
+    additionalProperties: false
+    properties:
+      data:
+        anyOf:
+        - type: object
+          additionalProperties: true
+        - type: 'null'
+  inputs:
+    type: array
+    title: "Transaction inputs"
+    maxItems: 1
+    minItems: 1
+    items:
+      type: "object"
+      required:
+      - fulfills
+      properties:
+        fulfills:
+            type: "null"

bigchaindb/common/schema/transaction_transfer.yaml

@@ -0,0 +1,29 @@
+---
+"$schema": "http://json-schema.org/draft-04/schema#"
+type: object
+title: Transaction Schema - TRANSFER specific properties
+required:
+- asset
+properties:
+  asset:
+    additionalProperties: false
+    properties:
+      id:
+        "$ref": "#/definitions/sha3_hexdigest"
+        description: |
+          ID of the transaction that created the asset.
+  inputs:
+    type: array
+    title: "Transaction inputs"
+    minItems: 1
+    items:
+      type: "object"
+      required:
+      - fulfills
+      properties:
+        fulfills:
+            type: "object"
+definitions:
+  sha3_hexdigest:
+    pattern: "[0-9a-f]{64}"
+    type: string

bigchaindb/common/transaction.py

@@ -999,7 +999,8 @@ class Transaction(object):
             transactions = [transactions]
 
         # create a set of the transactions' asset ids
-        asset_ids = {tx.id if tx.operation == Transaction.CREATE else tx.asset['id']
+        asset_ids = {tx.id if tx.operation == Transaction.CREATE
+                     else tx.asset['id']
                      for tx in transactions}
 
         # check that all the transasctions have the same asset id
@@ -1009,7 +1010,7 @@ class Transaction(object):
         return asset_ids.pop()
 
     @staticmethod
-    def validate_structure(tx_body):
+    def validate_id(tx_body):
         """Validate the transaction ID of a transaction
 
             Args:
@@ -1041,7 +1042,7 @@ class Transaction(object):
             Returns:
                 :class:`~bigchaindb.common.transaction.Transaction`
         """
-        cls.validate_structure(tx)
+        cls.validate_id(tx)
         inputs = [Input.from_dict(input_) for input_ in tx['inputs']]
         outputs = [Output.from_dict(output) for output in tx['outputs']]
         return cls(tx['operation'], tx['asset'], inputs, outputs,

bigchaindb/models.py

@@ -3,7 +3,7 @@ from bigchaindb.common.exceptions import (InvalidHash, InvalidSignature,
                                           DoubleSpend, InputDoesNotExist,
                                           TransactionNotInValidBlock,
                                           AssetIdMismatch, AmountError,
-                                          SybilError, ValidationError,
+                                          SybilError,
                                           DuplicateTransaction)
 from bigchaindb.common.transaction import Transaction
 from bigchaindb.common.utils import gen_timestamp, serialize
@@ -12,7 +12,7 @@ from bigchaindb.common.schema import validate_transaction_schema
 
 class Transaction(Transaction):
     def validate(self, bigchain):
-        """Validate a transaction.
+        """Validate transaction spend
 
         Args:
             bigchain (Bigchain): an instantiated bigchaindb.Bigchain object.
@@ -25,34 +25,9 @@ class Transaction(Transaction):
         Raises:
             ValidationError: If the transaction is invalid
         """
-        if len(self.inputs) == 0:
-            raise ValidationError('Transaction contains no inputs')
-
         input_conditions = []
-        inputs_defined = all([input_.fulfills for input_ in self.inputs])
-
-        # validate amounts
-        if any(output.amount < 1 for output in self.outputs):
-            raise AmountError('`amount` needs to be greater than zero')
-
-        if self.operation in (Transaction.CREATE, Transaction.GENESIS):
-            # validate asset
-            if self.asset['data'] is not None and not isinstance(self.asset['data'], dict):
-                raise ValidationError(('`asset.data` must be a dict instance or '
-                                      'None for `CREATE` transactions'))
-            # validate inputs
-            if inputs_defined:
-                raise ValidationError('A CREATE operation has no inputs')
-        elif self.operation == Transaction.TRANSFER:
-            # validate asset
-            if not isinstance(self.asset['id'], str):
-                raise ValidationError('`asset.id` must be a string for '
-                                      '`TRANSFER` transations')
-            # check inputs
-            if not inputs_defined:
-                raise ValidationError('Only `CREATE` transactions can have '
-                                      'null inputs')
 
+        if self.operation == Transaction.TRANSFER:
             # store the inputs so that we can check if the asset ids match
             input_txs = []
             for input_ in self.inputs:
@@ -77,8 +52,6 @@ class Transaction(Transaction):
                 output = input_tx.outputs[input_.fulfills.output]
                 input_conditions.append(output)
                 input_txs.append(input_tx)
-                if output.amount < 1:
-                    raise AmountError('`amount` needs to be greater than zero')
 
             # Validate that all inputs are distinct
             links = [i.fulfills.to_uri() for i in self.inputs]
@@ -92,11 +65,6 @@ class Transaction(Transaction):
                                        ' match the asset id of the'
                                        ' transaction'))
 
-            # validate the amounts
-            for output in self.outputs:
-                if output.amount < 1:
-                    raise AmountError('`amount` needs to be greater than zero')
-
             input_amount = sum([input_condition.amount for input_condition in input_conditions])
             output_amount = sum([output_condition.amount for output_condition in self.outputs])
 
@@ -106,11 +74,6 @@ class Transaction(Transaction):
                                    ' in the outputs `{}`')
                                   .format(input_amount, output_amount))
 
-        else:
-            allowed_operations = ', '.join(Transaction.ALLOWED_OPERATIONS)
-            raise ValidationError('`operation`: `{}` must be either {}.'
-                                  .format(self.operation, allowed_operations))
-
         if not self.inputs_valid(input_conditions):
             raise InvalidSignature('Transaction signature is invalid.')
 

tests/assets/test_digital_assets.py

@@ -28,7 +28,7 @@ def test_validate_bad_asset_creation(b, user_pk):
     tx_signed = tx.sign([b.me_private])
 
     with pytest.raises(ValidationError):
-        b.validate_transaction(tx_signed)
+        Transaction.from_dict(tx_signed.to_dict())
 
 
 @pytest.mark.bdb
@@ -93,15 +93,15 @@ def test_asset_id_mismatch(b, user_pk):
 
 def test_create_invalid_divisible_asset(b, user_pk, user_sk):
     from bigchaindb.models import Transaction
-    from bigchaindb.common.exceptions import AmountError
+    from bigchaindb.common.exceptions import ValidationError
 
     # Asset amount must be more than 0
     tx = Transaction.create([user_pk], [([user_pk], 1)])
     tx.outputs[0].amount = 0
     tx.sign([user_sk])
 
-    with pytest.raises(AmountError):
+    with pytest.raises(ValidationError):
-        b.validate_transaction(tx)
+        Transaction.from_dict(tx.to_dict())
 
 
 def test_create_valid_divisible_asset(b, user_pk, user_sk):

tests/assets/test_divisible_assets.py

@@ -638,6 +638,7 @@ def test_divide(b, user_pk, user_sk):
 
 
 # Check that negative inputs are caught when creating a TRANSFER transaction
+@pytest.mark.skip(reason='part of tx structural tests')
 @pytest.mark.bdb
 @pytest.mark.usefixtures('inputs')
 def test_non_positive_amounts_on_transfer(b, user_pk):
@@ -662,6 +663,7 @@ def test_non_positive_amounts_on_transfer(b, user_pk):
 
 
 # Check that negative inputs are caught when validating a TRANSFER transaction
+@pytest.mark.skip(reason='part of tx structural tests')
 @pytest.mark.bdb
 @pytest.mark.usefixtures('inputs')
 def test_non_positive_amounts_on_transfer_validate(b, user_pk, user_sk):
@@ -704,6 +706,7 @@ def test_non_positive_amounts_on_create(b, user_pk):
 
 
 # Check that negative inputs are caught when validating a CREATE transaction
+@pytest.mark.skip(reason='part of tx structural tests')
 @pytest.mark.bdb
 @pytest.mark.usefixtures('inputs')
 def test_non_positive_amounts_on_create_validate(b, user_pk):

tests/common/schema/test_transaction_schema.py

@@ -29,3 +29,32 @@ def test_validate_fails_metadata_empty_dict(create_tx):
     create_tx.metadata = {}
     with raises(SchemaValidationError):
         validate_transaction_schema(create_tx.to_dict())
+
+
+def test_transfer_asset_schema(signed_transfer_tx):
+    tx = signed_transfer_tx.to_dict()
+    validate_transaction_schema(tx)
+    tx['asset']['data'] = {}
+    with raises(SchemaValidationError):
+        validate_transaction_schema(tx)
+    del tx['asset']['data']
+    tx['asset']['id'] = 'b' * 63
+    with raises(SchemaValidationError):
+        validate_transaction_schema(tx)
+
+
+def test_create_single_input(create_tx):
+    tx = create_tx.to_dict()
+    tx['inputs'] += tx['inputs']
+    with raises(SchemaValidationError):
+        validate_transaction_schema(tx)
+    tx['inputs'] = []
+    with raises(SchemaValidationError):
+        validate_transaction_schema(tx)
+
+
+def test_create_tx_no_fulfills(create_tx):
+    tx = create_tx.to_dict()
+    tx['inputs'][0]['fulfills'] = {'tx': 'a' * 64, 'output': 0}
+    with raises(SchemaValidationError):
+        validate_transaction_schema(tx)

tests/common/test_transaction.py

@@ -352,6 +352,17 @@ def test_tx_serialization_with_incorrect_hash(utx):
     utx_dict.pop('id')
 
 
+def test_tx_serialization_hash_function(tx):
+    import sha3
+    import json
+    tx_dict = tx.to_dict()
+    tx_dict['inputs'][0]['fulfillment'] = None
+    del tx_dict['id']
+    payload = json.dumps(tx_dict, skipkeys=False, sort_keys=True,
+                         separators=(',', ':'))
+    assert sha3.sha3_256(payload.encode()).hexdigest() == tx.id
+
+
 def test_invalid_input_initialization(user_input, user_pub):
     from bigchaindb.common.transaction import Input
 
@@ -445,12 +456,15 @@ def test_transaction_link_eq():
 
 def test_add_input_to_tx(user_input, asset_definition):
     from bigchaindb.common.transaction import Transaction
+    from .utils import validate_transaction_model
 
     tx = Transaction(Transaction.CREATE, asset_definition, [], [])
     tx.add_input(user_input)
 
     assert len(tx.inputs) == 1
 
+    validate_transaction_model(tx)
+
 
 def test_add_input_to_tx_with_invalid_parameters(asset_definition):
     from bigchaindb.common.transaction import Transaction
@@ -460,11 +474,11 @@ def test_add_input_to_tx_with_invalid_parameters(asset_definition):
         tx.add_input('somewronginput')
 
 
-def test_add_output_to_tx(user_output, asset_definition):
+def test_add_output_to_tx(user_output, user_input, asset_definition):
     from bigchaindb.common.transaction import Transaction
     from .utils import validate_transaction_model
 
-    tx = Transaction(Transaction.CREATE, asset_definition)
+    tx = Transaction(Transaction.CREATE, asset_definition, [user_input])
     tx.add_output(user_output)
 
     assert len(tx.outputs) == 1
@@ -546,40 +560,6 @@ def test_validate_input_with_invalid_parameters(utx):
     assert not valid
 
 
-def test_validate_multiple_inputs(user_input, user_output, user_priv,
-                                  asset_definition):
-    from copy import deepcopy
-
-    from bigchaindb.common.crypto import PrivateKey
-    from bigchaindb.common.transaction import Transaction
-    from .utils import validate_transaction_model
-
-    tx = Transaction(Transaction.CREATE, asset_definition,
-                     [user_input, deepcopy(user_input)],
-                     [user_output, deepcopy(user_output)])
-
-    expected_first = deepcopy(tx)
-    expected_second = deepcopy(tx)
-    expected_first.inputs = [expected_first.inputs[0]]
-    expected_second.inputs = [expected_second.inputs[1]]
-
-    expected_first_bytes = str(expected_first).encode()
-    expected_first.inputs[0].fulfillment.sign(expected_first_bytes,
-                                              PrivateKey(user_priv))
-    expected_second_bytes = str(expected_second).encode()
-    expected_second.inputs[0].fulfillment.sign(expected_second_bytes,
-                                               PrivateKey(user_priv))
-    tx.sign([user_priv])
-
-    assert tx.inputs[0].to_dict()['fulfillment'] == \
-        expected_first.inputs[0].fulfillment.serialize_uri()
-    assert tx.inputs[1].to_dict()['fulfillment'] == \
-        expected_second.inputs[0].fulfillment.serialize_uri()
-    assert tx.inputs_valid() is True
-
-    validate_transaction_model(tx)
-
-
 def test_validate_tx_threshold_create_signature(user_user2_threshold_input,
                                                 user_user2_threshold_output,
                                                 user_pub,
@@ -621,8 +601,7 @@ def test_multiple_input_validation_of_transfer_tx(user_input, user_output,
     from cryptoconditions import Ed25519Fulfillment
     from .utils import validate_transaction_model
 
-    tx = Transaction(Transaction.CREATE, asset_definition,
+    tx = Transaction(Transaction.CREATE, asset_definition, [user_input],
-                     [user_input, deepcopy(user_input)],
                      [user_output, deepcopy(user_output)])
     tx.sign([user_priv])
 
@@ -985,3 +964,20 @@ def test_validate_version(utx):
     utx.version = '1.0.0'
     with raises(SchemaValidationError):
         validate_transaction_model(utx)
+
+
+def test_create_tx_no_asset_id(b, utx):
+    from bigchaindb.common.exceptions import SchemaValidationError
+    from .utils import validate_transaction_model
+    utx.asset['id'] = 'b' * 64
+    with raises(SchemaValidationError):
+        validate_transaction_model(utx)
+
+
+def test_transfer_tx_asset_schema(transfer_utx):
+    from bigchaindb.common.exceptions import SchemaValidationError
+    from .utils import validate_transaction_model
+    tx = transfer_utx
+    tx.asset['data'] = {}
+    with raises(SchemaValidationError):
+        validate_transaction_model(tx)

tests/db/test_bigchain_api.py

@@ -3,8 +3,6 @@ from time import sleep
 import pytest
 from unittest.mock import patch
 
-from bigchaindb.common.exceptions import ValidationError
-
 pytestmark = pytest.mark.bdb
 
 
@@ -544,24 +542,6 @@ class TestBigchainApi(object):
 
 
 class TestTransactionValidation(object):
-    def test_create_operation_with_inputs(self, b, user_pk, create_tx):
-        from bigchaindb.common.transaction import TransactionLink
-
-        # Manipulate input so that it has a `fulfills` defined even
-        # though it shouldn't have one
-        create_tx.inputs[0].fulfills = TransactionLink('abc', 0)
-        with pytest.raises(ValidationError) as excinfo:
-            b.validate_transaction(create_tx)
-        assert excinfo.value.args[0] == 'A CREATE operation has no inputs'
-
-    def test_transfer_operation_no_inputs(self, b, user_pk,
-                                          signed_transfer_tx):
-        signed_transfer_tx.inputs[0].fulfills = None
-        with pytest.raises(ValidationError) as excinfo:
-            b.validate_transaction(signed_transfer_tx)
-
-        assert excinfo.value.args[0] == 'Only `CREATE` transactions can have null inputs'
-
     def test_non_create_input_not_found(self, b, user_pk, signed_transfer_tx):
         from bigchaindb.common.exceptions import InputDoesNotExist
         from bigchaindb.common.transaction import TransactionLink
@@ -1261,10 +1241,3 @@ def test_is_new_transaction(b, genesis_block):
     # Tx is new because it's only found in an invalid block
     assert b.is_new_transaction(tx.id)
     assert b.is_new_transaction(tx.id, exclude_block_id=block.id)
-
-
-def test_validate_asset_id_string(signed_transfer_tx):
-    from bigchaindb.common.exceptions import ValidationError
-    signed_transfer_tx.asset['id'] = 1
-    with pytest.raises(ValidationError):
-        signed_transfer_tx.validate(None)

tests/test_models.py

@@ -1,21 +1,4 @@
 from pytest import raises
-from bigchaindb.common.exceptions import ValidationError
-
-
-class TestTransactionModel(object):
-    def test_validating_an_invalid_transaction(self, b):
-        from bigchaindb.models import Transaction
-
-        tx = Transaction.create([b.me], [([b.me], 1)])
-        tx.operation = 'something invalid'
-
-        with raises(ValidationError):
-            tx.validate(b)
-
-        tx.operation = 'CREATE'
-        tx.inputs = []
-        with raises(ValidationError):
-            tx.validate(b)
 
 
 class TestBlockModel(object):

tests/web/test_outputs.py

@@ -100,7 +100,7 @@ def test_get_divisble_transactions_returns_500(b, client):
 
     asset_id = create_tx.id
 
-    url = TX_ENDPOINT + "?asset_id=" + asset_id
+    url = TX_ENDPOINT + '?asset_id=' + asset_id
     assert client.get(url).status_code == 200
     assert len(client.get(url).json) == 3