Problem: Invalid transaction posted to Tendermint (#2270)

* Problem: Amount error is not tested on the HTTP level.

Solution: A failed web test to reproduce the problem.

* Problem: Invalid transaction posted to Tendermint

Solution: Pass the exception to HTTP POST handler function

* Problem: DoubleSpend and CriticalDoubleSpend not differentiated

Solution: Handle these exceptions differently in `get_spent`

* Problem: No test for checking exception DoubleSpend and CriticalDoubleSpend

Solution: Add necessary tests

* Problem: find doesn't raise IndexError

Solution: Remove exception handling for IndexError

bigchaindb/backend/localmongodb/query.py

@@ -95,14 +95,11 @@ def get_asset(conn, asset_id):
 
 @register_query(LocalMongoDBConnection)
 def get_spent(conn, transaction_id, output):
-    try:
-        return conn.run(
-            conn.collection('transactions')
-            .find_one({'inputs.fulfills.transaction_id': transaction_id,
-                       'inputs.fulfills.output_index': output},
-                      {'_id': 0}))
-    except IndexError:
-        pass
+    return conn.run(
+        conn.collection('transactions')
+        .find({'inputs.fulfills.transaction_id': transaction_id,
+               'inputs.fulfills.output_index': output},
+              {'_id': 0}))
 
 
 @register_query(LocalMongoDBConnection)

bigchaindb/tendermint/core.py

@@ -57,7 +57,7 @@ class App(BaseApplication):
             raw_tx: a raw string (in bytes) transaction."""
         logger.debug('check_tx: %s', raw_transaction)
         transaction = decode_transaction(raw_transaction)
-        if self.bigchaindb.validate_transaction(transaction):
+        if self.bigchaindb.is_valid_transaction(transaction):
             logger.debug('check_tx: VALID')
             return Result.ok()
         else:
@@ -80,7 +80,7 @@ class App(BaseApplication):
         Args:
             raw_tx: a raw string (in bytes) transaction."""
         logger.debug('deliver_tx: %s', raw_transaction)
-        transaction = self.bigchaindb.validate_transaction(
+        transaction = self.bigchaindb.is_valid_transaction(
             decode_transaction(raw_transaction), self.block_transactions)
 
         if not transaction:

bigchaindb/tendermint/lib.py

@@ -19,7 +19,9 @@ import requests
 from bigchaindb import backend
 from bigchaindb import Bigchain
 from bigchaindb.models import Transaction
-from bigchaindb.common.exceptions import SchemaValidationError, ValidationError
+from bigchaindb.common.exceptions import (SchemaValidationError,
+                                          ValidationError,
+                                          DoubleSpend)
 from bigchaindb.tendermint.utils import encode_transaction, merkleroot
 from bigchaindb.tendermint import fastquery
 from bigchaindb import exceptions as core_exceptions
@@ -242,6 +244,10 @@ class BigchainDB(Bigchain):
         transactions = backend.query.get_spent(self.connection, txid,
                                                output)
         transactions = list(transactions) if transactions else []
+        if len(transactions) > 1:
+            raise core_exceptions.CriticalDoubleSpend(
+                '`{}` was spent more than once. There is a problem'
+                ' with the chain'.format(txid))
 
         for ctxn in current_transactions:
             for ctxn_input in ctxn.inputs:
@@ -251,25 +257,11 @@ class BigchainDB(Bigchain):
 
         transaction = None
         if len(transactions) > 1:
-            raise core_exceptions.CriticalDoubleSpend(
-                '`{}` was spent more than once. There is a problem'
-                ' with the chain'.format(txid))
+            raise DoubleSpend('tx "{}" spends inputs twice'.format(txid))
         elif transactions:
-            transaction = transactions[0]
+            transaction = Transaction.from_db(self, transactions[0])
 
-        if transaction and transaction['operation'] == 'CREATE':
-            asset = backend.query.get_asset(self.connection, transaction['id'])
-
-            if asset:
-                transaction['asset'] = asset
-            else:
-                transaction['asset'] = {'data': None}
-
-            return Transaction.from_dict(transaction)
-        elif transaction and transaction['operation'] == 'TRANSFER':
-            return Transaction.from_dict(transaction)
-        else:
-            return None
+        return transaction
 
     def store_block(self, block):
         """Create a new block."""
@@ -338,6 +330,9 @@ class BigchainDB(Bigchain):
 
         transaction = tx
 
+        # CLEANUP: The conditional below checks for transaction in dict format.
+        # It would be better to only have a single format for the transaction
+        # throught the code base.
         if not isinstance(transaction, Transaction):
             try:
                 transaction = Transaction.from_dict(tx)
@@ -347,12 +342,16 @@ class BigchainDB(Bigchain):
             except ValidationError as e:
                 logger.warning('Invalid transaction (%s): %s', type(e).__name__, e)
                 return False
+        return transaction.validate(self, current_transactions)
+
+    def is_valid_transaction(self, tx, current_transactions=[]):
+        # NOTE: the function returns the Transaction object in case
+        # the transaction is valid
         try:
-            return transaction.validate(self, current_transactions)
+            return self.validate_transaction(tx, current_transactions)
         except ValidationError as e:
             logger.warning('Invalid transaction (%s): %s', type(e).__name__, e)
             return False
-        return transaction
 
     @property
     def fastquery(self):

tests/tendermint/test_lib.py

@@ -350,3 +350,43 @@ def test_get_utxoset_merkle_root(b, utxoset):
         '86d311c03115bf4d287f8449ca5828505432d69b82762d47077b1c00fe426eac')
     merkle_root = b.get_utxoset_merkle_root()
     assert merkle_root == expected_merkle_root
+
+
+@pytest.mark.bdb
+def test_get_spent_transaction_critical_double_spend(b, alice, bob, carol):
+    from bigchaindb.models import Transaction
+    from bigchaindb.exceptions import CriticalDoubleSpend
+    from bigchaindb.common.exceptions import DoubleSpend
+
+    asset = {'test': 'asset'}
+
+    tx = Transaction.create([alice.public_key],
+                            [([alice.public_key], 1)],
+                            asset=asset)\
+                    .sign([alice.private_key])
+
+    tx_transfer = Transaction.transfer(tx.to_inputs(),
+                                       [([bob.public_key], 1)],
+                                       asset_id=tx.id)\
+                             .sign([alice.private_key])
+
+    double_spend = Transaction.transfer(tx.to_inputs(),
+                                        [([carol.public_key], 1)],
+                                        asset_id=tx.id)\
+                              .sign([alice.private_key])
+
+    b.store_bulk_transactions([tx])
+
+    with pytest.raises(DoubleSpend):
+        b.get_spent(tx.id, tx_transfer.inputs[0].fulfills.output,
+                    [tx_transfer, double_spend])
+
+    b.store_bulk_transactions([tx_transfer])
+
+    with pytest.raises(DoubleSpend):
+        b.get_spent(tx.id, tx_transfer.inputs[0].fulfills.output, [double_spend])
+
+    b.store_bulk_transactions([double_spend])
+
+    with pytest.raises(CriticalDoubleSpend):
+        b.get_spent(tx.id, tx_transfer.inputs[0].fulfills.output)

tests/web/test_transactions.py

@@ -342,6 +342,30 @@ def test_post_invalid_transfer_transaction_returns_400(b, client, user_pk):
     assert res.json['message'] == expected_error_message
 
 
+@pytest.mark.tendermint
+def test_post_wrong_asset_division_transfer_returns_400(b, client, user_pk):
+    from bigchaindb.models import Transaction
+    from bigchaindb.common.exceptions import InputDoesNotExist
+
+    priv_key, pub_key = crypto.generate_key_pair()
+
+    create_tx = Transaction.create([pub_key],
+                                   [([pub_key], 10)],
+                                   asset={'test': 'asset'}).sign([priv_key])
+    res = client.post(TX_ENDPOINT + '?mode=commit', data=json.dumps(create_tx.to_dict()))
+    assert res.status_code == 202
+
+    transfer_tx = Transaction.transfer(create_tx.to_inputs(),
+                                       [([pub_key], 20)],  # 20 > 10
+                                       asset_id=create_tx.id).sign([priv_key])
+    res = client.post(TX_ENDPOINT + '?mode=commit', data=json.dumps(transfer_tx.to_dict()))
+    expected_error_message = 'Invalid transaction ({}): input `{}` doesn\'t exist'.format(
+        InputDoesNotExist.__name__, create_tx.id)
+
+    assert res.status_code == 400
+    assert res.json['message'] == expected_error_message
+
+
 @pytest.mark.tendermint
 def test_transactions_get_list_good(client):
     from functools import partial