remove structural and schematic validations from Transaction.validate which is validating the spend

bigchaindb/common/schema/transaction_transfer.yaml

@@ -12,6 +12,17 @@ properties:
         "$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}"

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.
@@ -26,30 +26,8 @@ class Transaction(Transaction):
             ValidationError: If the transaction is invalid
         """
         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:
@@ -74,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]
@@ -89,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])
 
@@ -103,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/test_transaction.py

@@ -972,3 +972,12 @@ def test_create_tx_no_asset_id(b, utx):
     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
 
 
@@ -596,24 +594,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
@@ -1313,10 +1293,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)