Make ValidationError a superclass of all validation errors and use it

bigchaindb/common/exceptions.py

@@ -7,40 +7,6 @@ class ConfigurationError(BigchainDBError):
     """Raised when there is a problem with server configuration"""
 
 
-class OperationError(BigchainDBError):
-    """Raised when an operation cannot go through"""
-
-
-class TransactionDoesNotExist(BigchainDBError):
-    """Raised if the transaction is not in the database"""
-
-
-class TransactionOwnerError(BigchainDBError):
-    """Raised if a user tries to transfer a transaction they don't own"""
-
-
-class DoubleSpend(BigchainDBError):
-    """Raised if a double spend is found"""
-
-
-class ValidationError(BigchainDBError):
-    """Raised if there was an error in validation"""
-
-
-class InvalidHash(ValidationError):
-    """Raised if there was an error checking the hash for a particular
-    operation"""
-
-
-class SchemaValidationError(ValidationError):
-    """Raised if there was any error validating an object's schema"""
-
-
-class InvalidSignature(BigchainDBError):
-    """Raised if there was an error checking the signature for a particular
-    operation"""
-
-
 class DatabaseAlreadyExists(BigchainDBError):
     """Raised when trying to create the database but the db is already there"""
 
@@ -49,6 +15,18 @@ class DatabaseDoesNotExist(BigchainDBError):
     """Raised when trying to delete the database but the db is not there"""
 
 
+class StartupError(BigchainDBError):
+    """Raised when there is an error starting up the system"""
+
+
+class GenesisBlockAlreadyExistsError(BigchainDBError):
+    """Raised when trying to create the already existing genesis block"""
+
+
+class CyclicBlockchainError(BigchainDBError):
+    """Raised when there is a cycle in the blockchain"""
+
+
 class KeypairNotFoundException(BigchainDBError):
     """Raised if operation cannot proceed because the keypair was not given"""
 
@@ -58,34 +36,64 @@ class KeypairMismatchException(BigchainDBError):
     current owner(s)"""
 
 
-class StartupError(BigchainDBError):
+class OperationError(BigchainDBError):
-    """Raised when there is an error starting up the system"""
+    """Raised when an operation cannot go through"""
 
 
-class ImproperVoteError(BigchainDBError):
+################################################################################
+# Validation errors
+
+
+class ValidationError(BigchainDBError):
+    """Raised if there was an error in validation"""
+
+
+class DoubleSpend(ValidationError):
+    """Raised if a double spend is found"""
+
+
+class InvalidHash(ValidationError):
+    """Raised if there was an error checking the hash for a particular
+    operation"""
+
+
+class SchemaValidationError(ValidationError):
+    """Raised if there was any error validating an object's schema"""
+
+
+class InvalidSignature(ValidationError):
+    """Raised if there was an error checking the signature for a particular
+    operation"""
+
+
+class ImproperVoteError(ValidationError):
     """Raised if a vote is not constructed correctly, or signed incorrectly"""
 
 
-class MultipleVotesError(BigchainDBError):
+class MultipleVotesError(ValidationError):
     """Raised if a voter has voted more than once"""
 
 
-class GenesisBlockAlreadyExistsError(BigchainDBError):
+class TransactionNotInValidBlock(ValidationError):
-    """Raised when trying to create the already existing genesis block"""
-
-
-class CyclicBlockchainError(BigchainDBError):
-    """Raised when there is a cycle in the blockchain"""
-
-
-class TransactionNotInValidBlock(BigchainDBError):
     """Raised when a transfer transaction is attempting to fulfill the
     outputs of a transaction that is in an invalid or undecided block"""
 
 
-class AssetIdMismatch(BigchainDBError):
+class AssetIdMismatch(ValidationError):
     """Raised when multiple transaction inputs related to different assets"""
 
 
-class AmountError(BigchainDBError):
+class AmountError(ValidationError):
     """Raised when there is a problem with a transaction's output amounts"""
+
+
+class TransactionDoesNotExist(ValidationError):
+    """Raised if the transaction is not in the database"""
+
+
+class TransactionOwnerError(ValidationError):
+    """Raised if a user tries to transfer a transaction they don't own"""
+
+
+class SybilError(ValidationError):
+    """If a block or vote comes from an unidentifiable node"""

bigchaindb/core.py

@@ -162,31 +162,6 @@ class Bigchain(object):
 
         return self.consensus.validate_transaction(self, transaction)
 
-    def is_valid_transaction(self, transaction):
-        """Check whether a transaction is valid or invalid.
-
-        Similar to :meth:`~bigchaindb.Bigchain.validate_transaction`
-        but never raises an exception. It returns :obj:`False` if
-        the transaction is invalid.
-
-        Args:
-            transaction (:Class:`~bigchaindb.models.Transaction`): transaction
-                to check.
-
-        Returns:
-            The :class:`~bigchaindb.models.Transaction` instance if valid,
-            otherwise :obj:`False`.
-        """
-
-        try:
-            return self.validate_transaction(transaction)
-        except (ValueError, exceptions.OperationError,
-                exceptions.TransactionDoesNotExist,
-                exceptions.TransactionOwnerError, exceptions.DoubleSpend,
-                exceptions.InvalidHash, exceptions.InvalidSignature,
-                exceptions.TransactionNotInValidBlock, exceptions.AmountError):
-            return False
-
     def is_new_transaction(self, txid, exclude_block_id=None):
         """
         Return True if the transaction does not exist in any
@@ -386,10 +361,9 @@ class Bigchain(object):
                 if self.get_transaction(transaction['id']):
                     num_valid_transactions += 1
                 if num_valid_transactions > 1:
-                    raise exceptions.DoubleSpend(('`{}` was spent more than'
+                    raise exceptions.BigchainDBCritical(
-                                                  ' once. There is a problem'
+                        '`{}` was spent more than once. There is a problem'
-                                                  ' with the chain')
+                        ' with the chain'.format(txid))
-                                                 .format(txid))
 
             if num_valid_transactions:
                 return Transaction.from_dict(transactions[0])

bigchaindb/models.py

@@ -1,9 +1,9 @@
 from bigchaindb.common.crypto import hash_data, PublicKey, PrivateKey
 from bigchaindb.common.exceptions import (InvalidHash, InvalidSignature,
-                                          OperationError, DoubleSpend,
+                                          DoubleSpend, TransactionDoesNotExist,
-                                          TransactionDoesNotExist,
                                           TransactionNotInValidBlock,
-                                          AssetIdMismatch, AmountError)
+                                          AssetIdMismatch, AmountError,
+                                          SybilError, ValidationError)
 from bigchaindb.common.transaction import Transaction
 from bigchaindb.common.utils import gen_timestamp, serialize
 from bigchaindb.common.schema import validate_transaction_schema
@@ -22,19 +22,10 @@ class Transaction(Transaction):
             invalid.
 
         Raises:
-            OperationError: if the transaction operation is not supported
+            ValidationError: If the transaction is invalid
-            TransactionDoesNotExist: if the input of the transaction is not
-                                     found
-            TransactionNotInValidBlock: if the input of the transaction is not
-                                        in a valid block
-            TransactionOwnerError: if the new transaction is using an input it
-                                   doesn't own
-            DoubleSpend: if the transaction is a double spend
-            InvalidHash: if the hash of the transaction is wrong
-            InvalidSignature: if the signature of the transaction is wrong
         """
         if len(self.inputs) == 0:
-            raise ValueError('Transaction contains no inputs')
+            raise ValidationError('Transaction contains no inputs')
 
         input_conditions = []
         inputs_defined = all([input_.fulfills for input_ in self.inputs])
@@ -46,20 +37,20 @@ class Transaction(Transaction):
         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 TypeError(('`asset.data` must be a dict instance or '
+                raise ValidationError(('`asset.data` must be a dict instance or '
-                                 'None for `CREATE` transactions'))
+                                      'None for `CREATE` transactions'))
             # validate inputs
             if inputs_defined:
-                raise ValueError('A CREATE operation has no inputs')
+                raise ValidationError('A CREATE operation has no inputs')
         elif self.operation == Transaction.TRANSFER:
             # validate asset
             if not isinstance(self.asset['id'], str):
-                raise ValueError(('`asset.id` must be a string for '
+                raise ValidationError('`asset.id` must be a string for '
-                                  '`TRANSFER` transations'))
+                                      '`TRANSFER` transations')
             # check inputs
             if not inputs_defined:
-                raise ValueError('Only `CREATE` transactions can have null '
+                raise ValidationError('Only `CREATE` transactions can have '
-                                 'inputs')
+                                      'null inputs')
 
             # store the inputs so that we can check if the asset ids match
             input_txs = []
@@ -116,8 +107,8 @@ class Transaction(Transaction):
 
         else:
             allowed_operations = ', '.join(Transaction.ALLOWED_OPERATIONS)
-            raise TypeError('`operation`: `{}` must be either {}.'
+            raise ValidationError('`operation`: `{}` must be either {}.'
-                            .format(self.operation, allowed_operations))
+                                  .format(self.operation, allowed_operations))
 
         if not self.inputs_valid(input_conditions):
             raise InvalidSignature('Transaction signature is invalid.')
@@ -205,18 +196,8 @@ class Block(object):
             raised.
 
         Raises:
-            OperationError: If a non-federation node signed the Block.
+            ValidationError: If the block or any transaction in the block does
-            InvalidSignature: If a Block's signature is invalid or if the
+                             not validate
-                block contains a transaction with an invalid signature.
-            OperationError: if the transaction operation is not supported
-            TransactionDoesNotExist: if the input of the transaction is not
-                                     found
-            TransactionNotInValidBlock: if the input of the transaction is not
-                                        in a valid block
-            TransactionOwnerError: if the new transaction is using an input it
-                                   doesn't own
-            DoubleSpend: if the transaction is a double spend
-            InvalidHash: if the hash of the transaction is wrong
         """
 
         self._validate_block(bigchain)
@@ -232,13 +213,12 @@ class Block(object):
                 object.
 
         Raises:
-            OperationError: If a non-federation node signed the Block.
+            ValidationError: If there is a problem with the block
-            InvalidSignature: If a Block's signature is invalid.
         """
         # Check if the block was created by a federation node
         possible_voters = (bigchain.nodes_except_me + [bigchain.me])
         if self.node_pubkey not in possible_voters:
-            raise OperationError('Only federation nodes can create blocks')
+            raise SybilError('Only federation nodes can create blocks')
 
         # Check that the signature is valid
         if not self.is_signature_valid():
@@ -251,16 +231,7 @@ class Block(object):
             bigchain (Bigchain): an instantiated bigchaindb.Bigchain object.
 
         Raises:
-            OperationError: if the transaction operation is not supported
+            ValidationError: If an invalid transaction is found
-            TransactionDoesNotExist: if the input of the transaction is not
-                                     found
-            TransactionNotInValidBlock: if the input of the transaction is not
-                                        in a valid block
-            TransactionOwnerError: if the new transaction is using an input it
-                                   doesn't own
-            DoubleSpend: if the transaction is a double spend
-            InvalidHash: if the hash of the transaction is wrong
-            InvalidSignature: if the signature of the transaction is wrong
         """
         for tx in self.transactions:
             # If a transaction is not valid, `validate_transactions` will
@@ -341,10 +312,10 @@ class Block(object):
             dict: The Block as a dict.
 
         Raises:
-            OperationError: If the Block doesn't contain any transactions.
+            ValueError: If the Block doesn't contain any transactions.
         """
         if len(self.transactions) == 0:
-            raise OperationError('Empty block creation is not allowed')
+            raise ValueError('Empty block creation is not allowed')
 
         block = {
             'timestamp': self.timestamp,

bigchaindb/pipelines/block.py

@@ -13,8 +13,7 @@ import bigchaindb
 from bigchaindb import backend
 from bigchaindb.backend.changefeed import ChangeFeed
 from bigchaindb.models import Transaction
-from bigchaindb.common.exceptions import (SchemaValidationError, InvalidHash,
+from bigchaindb.common.exceptions import ValidationError
-                                          InvalidSignature, AmountError)
 from bigchaindb import Bigchain
 
 
@@ -63,8 +62,7 @@ class BlockPipeline:
         """
         try:
             tx = Transaction.from_dict(tx)
-        except (SchemaValidationError, InvalidHash, InvalidSignature,
+        except ValidationError:
-                AmountError):
             return None
 
         # If transaction is in any VALID or UNDECIDED block we
@@ -74,12 +72,14 @@ class BlockPipeline:
             return None
 
         # If transaction is not valid it should not be included
-        if not self.bigchain.is_valid_transaction(tx):
+        try:
+            tx.validate(self.bigchain)
+            return tx
+        except ValidationError as e:
+            # todo: log
             self.bigchain.delete_transaction(tx.id)
             return None
 
-        return tx
-
     def create(self, tx, timeout=False):
         """Create a block.
 

bigchaindb/pipelines/vote.py

@@ -61,7 +61,7 @@ class Vote:
                 return block['id'], [self.invalid_dummy_tx]
             try:
                 block._validate_block(self.bigchain)
-            except (exceptions.OperationError, exceptions.InvalidSignature):
+            except exceptions.ValidationError:
                 # 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
@@ -105,7 +105,13 @@ class Vote:
         if not new:
             return False, block_id, num_tx
 
-        valid = bool(self.bigchain.is_valid_transaction(tx))
+        try:
+            tx.validate(self.bigchain)
+            valid = True
+        except exceptions.ValidationError:
+            # TODO: log
+            valid = False
+
         return valid, block_id, num_tx
 
     def vote(self, tx_validity, block_id, num_tx):

bigchaindb/web/views/transactions.py

@@ -9,20 +9,7 @@ import logging
 from flask import current_app, request
 from flask_restful import Resource, reqparse
 
-
+from bigchaindb.common.exceptions import SchemaValidationError, ValidationError
-from bigchaindb.common.exceptions import (
-    AmountError,
-    DoubleSpend,
-    InvalidHash,
-    InvalidSignature,
-    SchemaValidationError,
-    OperationError,
-    TransactionDoesNotExist,
-    TransactionOwnerError,
-    TransactionNotInValidBlock,
-    ValidationError,
-)
-
 from bigchaindb.models import Transaction
 from bigchaindb.web.views.base import make_error
 from bigchaindb.web.views import parameters
@@ -84,7 +71,7 @@ class TransactionListApi(Resource):
                 message='Invalid transaction schema: {}'.format(
                     e.__cause__.message)
             )
-        except (ValidationError, InvalidSignature) as e:
+        except ValidationError as e:
             return make_error(
                 400,
                 'Invalid transaction ({}): {}'.format(type(e).__name__, e)
@@ -93,15 +80,7 @@ class TransactionListApi(Resource):
         with pool() as bigchain:
             try:
                 bigchain.validate_transaction(tx_obj)
-            except (ValueError,
+            except ValidationError as e:
-                    OperationError,
-                    TransactionDoesNotExist,
-                    TransactionOwnerError,
-                    DoubleSpend,
-                    InvalidHash,
-                    InvalidSignature,
-                    TransactionNotInValidBlock,
-                    AmountError) as e:
                 return make_error(
                     400,
                     'Invalid transaction ({}): {}'.format(type(e).__name__, e)

tests/assets/test_digital_assets.py

@@ -1,3 +1,4 @@
+from bigchaindb.common.exceptions import ValidationError
 import pytest
 import random
 
@@ -26,7 +27,7 @@ def test_validate_bad_asset_creation(b, user_pk):
     tx.asset['data'] = 'a'
     tx_signed = tx.sign([b.me_private])
 
-    with pytest.raises(TypeError):
+    with pytest.raises(ValidationError):
         b.validate_transaction(tx_signed)
 
 
@@ -108,4 +109,4 @@ def test_create_valid_divisible_asset(b, user_pk, user_sk):
 
     tx = Transaction.create([user_pk], [([user_pk], 2)])
     tx_signed = tx.sign([user_sk])
-    assert b.is_valid_transaction(tx_signed)
+    tx_signed.validate(b)

tests/db/test_bigchain_api.py

@@ -3,6 +3,8 @@ from time import sleep
 import pytest
 from unittest.mock import patch
 
+from bigchaindb.common.exceptions import ValidationError
+
 pytestmark = pytest.mark.bdb
 
 
@@ -565,14 +567,14 @@ class TestTransactionValidation(object):
         # 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(ValueError) as excinfo:
+        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(ValueError) as excinfo:
+        with pytest.raises(ValidationError) as excinfo:
             b.validate_transaction(signed_transfer_tx)
 
         assert excinfo.value.args[0] == 'Only `CREATE` transactions can have null inputs'
@@ -741,7 +743,7 @@ class TestBlockValidation(object):
             b.validate_block(block)
 
     def test_invalid_node_pubkey(self, b):
-        from bigchaindb.common.exceptions import OperationError
+        from bigchaindb.common.exceptions import SybilError
         from bigchaindb.common import crypto
 
         # blocks can only be created by a federation node
@@ -758,8 +760,8 @@ class TestBlockValidation(object):
         # from a non federation node
         block = block.sign(tmp_sk)
 
-        # check that validate_block raises an OperationError
+        # check that validate_block raises an SybilError
-        with pytest.raises(OperationError):
+        with pytest.raises(SybilError):
             b.validate_block(block)
 
 
@@ -778,7 +780,7 @@ class TestMultipleInputs(object):
         tx = tx.sign([user_sk])
 
         # validate transaction
-        assert b.is_valid_transaction(tx) == tx
+        tx.validate(b)
         assert len(tx.inputs) == 1
         assert len(tx.outputs) == 1
 
@@ -800,7 +802,7 @@ class TestMultipleInputs(object):
                                   asset_id=input_tx.id)
         tx = tx.sign([user_sk])
 
-        assert b.is_valid_transaction(tx) == tx
+        tx.validate(b)
         assert len(tx.inputs) == 1
         assert len(tx.outputs) == 1
 
@@ -832,7 +834,7 @@ class TestMultipleInputs(object):
         transfer_tx = transfer_tx.sign([user_sk, user2_sk])
 
         # validate transaction
-        assert b.is_valid_transaction(transfer_tx) == transfer_tx
+        transfer_tx.validate(b)
         assert len(transfer_tx.inputs) == 1
         assert len(transfer_tx.outputs) == 1
 
@@ -865,7 +867,7 @@ class TestMultipleInputs(object):
                                   asset_id=tx_input.id)
         tx = tx.sign([user_sk, user2_sk])
 
-        assert b.is_valid_transaction(tx) == tx
+        tx.validate(b)
         assert len(tx.inputs) == 1
         assert len(tx.outputs) == 1
 
@@ -1219,7 +1221,6 @@ def test_cant_spend_same_input_twice_in_tx(b, genesis_block):
     tx_transfer = Transaction.transfer(dup_inputs, [([b.me], 200)],
                                        asset_id=tx_create.id)
     tx_transfer_signed = tx_transfer.sign([b.me_private])
-    assert b.is_valid_transaction(tx_transfer_signed) is False
     with pytest.raises(DoubleSpend):
         tx_transfer_signed.validate(b)
 

tests/pipelines/test_block_creation.py

@@ -46,28 +46,19 @@ def test_validate_transaction_handles_exceptions(b, signed_create_tx):
     """
     from bigchaindb.pipelines.block import BlockPipeline
     block_maker = BlockPipeline()
+    from bigchaindb.common.exceptions import ValidationError
 
-    # Test SchemaValidationError
     tx_dict = signed_create_tx.to_dict()
-    tx_dict['invalid_key'] = 'schema validation gonna getcha!'
-    assert block_maker.validate_tx(tx_dict) is None
 
-    # Test InvalidHash
+    with patch('bigchaindb.models.Transaction.validate') as validate:
-    tx_dict = signed_create_tx.to_dict()
+        # Assert that validationerror gets caught
-    tx_dict['id'] = 'a' * 64
+        validate.side_effect = ValidationError()
-    assert block_maker.validate_tx(tx_dict) is None
+        assert block_maker.validate_tx(tx_dict) is None
 
-    # Test InvalidSignature when we pass a bad fulfillment
+        # Assert that another error doesnt
-    tx_dict = signed_create_tx.to_dict()
+        validate.side_effect = IOError()
-    tx_dict['inputs'][0]['fulfillment'] = 'cf:0:aaaaaaaaaaaaaaaaaaaaaaaaa'
+        with pytest.raises(IOError):
-    assert block_maker.validate_tx(tx_dict) is None
+            block_maker.validate_tx(tx_dict)
-
-    # Test AmountError
-    signed_create_tx.outputs[0].amount = 0
-    tx_dict = signed_create_tx.to_dict()
-    # set the correct value back so that we can continue using it
-    signed_create_tx.outputs[0].amount = 1
-    assert block_maker.validate_tx(tx_dict) is None
 
 
 def test_create_block(b, user_pk):

tests/pipelines/test_vote.py

@@ -135,10 +135,17 @@ def test_vote_validate_transaction(b):
     validation = vote_obj.validate_tx(tx, 123, 1)
     assert validation == (True, 123, 1)
 
-    # NOTE: Submit unsigned transaction to `validate_tx` yields `False`.
+    with patch('bigchaindb.models.Transaction.validate') as validate:
-    tx = Transaction.create([b.me], [([b.me], 1)])
+        # Assert that validationerror gets caught
-    validation = vote_obj.validate_tx(tx, 456, 10)
+        validate.side_effect = ValidationError()
-    assert validation == (False, 456, 10)
+        validation = vote_obj.validate_tx(tx, 456, 10)
+        assert validation == (False, 456, 10)
+
+        # Assert that another error doesnt
+        validate.side_effect = IOError()
+        with pytest.raises(IOError):
+            validation = vote_obj.validate_tx(tx, 456, 10)
+
 
 
 @pytest.mark.genesis

tests/test_models.py

@@ -1,4 +1,5 @@
 from pytest import raises
+from bigchaindb.common.exceptions import ValidationError
 
 
 class TestTransactionModel(object):
@@ -8,12 +9,12 @@ class TestTransactionModel(object):
         tx = Transaction.create([b.me], [([b.me], 1)])
         tx.operation = 'something invalid'
 
-        with raises(TypeError):
+        with raises(ValidationError):
             tx.validate(b)
 
         tx.operation = 'CREATE'
         tx.inputs = []
-        with raises(ValueError):
+        with raises(ValidationError):
             tx.validate(b)
 
 
@@ -61,11 +62,10 @@ class TestBlockModel(object):
         assert block.to_dict() == expected
 
     def test_block_invalid_serializaton(self):
-        from bigchaindb.common.exceptions import OperationError
         from bigchaindb.models import Block
 
         block = Block([])
-        with raises(OperationError):
+        with raises(ValueError):
             block.to_dict()
 
     def test_block_deserialization(self, b):

tests/web/test_transactions.py

@@ -1,4 +1,3 @@
-import builtins
 import json
 from unittest.mock import patch
 
@@ -113,18 +112,15 @@ def test_post_create_transaction_with_invalid_schema(client, caplog):
     ('DoubleSpend', 'Nope! It is gone now!'),
     ('InvalidHash', 'Do not smoke that!'),
     ('InvalidSignature', 'Falsche Unterschrift!'),
-    ('OperationError', 'Create and transfer!'),
+    ('ValidationError', 'Create and transfer!'),
     ('TransactionDoesNotExist', 'Hallucinations?'),
     ('TransactionOwnerError', 'Not yours!'),
     ('TransactionNotInValidBlock', 'Wait, maybe?'),
-    ('ValueError', '?'),
+    ('ValidationError', '?'),
 ))
 def test_post_invalid_transaction(client, exc, msg, monkeypatch, caplog):
     from bigchaindb.common import exceptions
-    try:
+    exc_cls = getattr(exceptions, exc)
-        exc_cls = getattr(exceptions, exc)
-    except AttributeError:
-        exc_cls = getattr(builtins, 'ValueError')
 
     def mock_validation(self_, tx):
         raise exc_cls(msg)