Merge pull request #1286 from bigchaindb/tx-amount-string

transaction.outputs.amount as string

bigchaindb/common/schema/transaction.yaml

@@ -132,7 +132,8 @@ definitions:
     - public_keys
     properties:
       amount:
-        type: integer
+        type: string
+        pattern: "^[0-9]{1,20}$"
         description: |
             Integral amount of the asset represented by this output.
             In the case of a non divisible asset, this will always be 1.
@@ -158,10 +159,6 @@ definitions:
         "$ref": "#/definitions/public_keys"
         description: |
             List of public keys associated with the conditions on an output.
-      amount:
-        type: integer
-        description: |
-            Integral amount of the asset represented by this condition.
   input:
     type: "object"
     description:

bigchaindb/common/transaction.py

@@ -209,6 +209,8 @@ class Output(object):
                 owners before a Transaction was confirmed.
     """
 
+    MAX_AMOUNT = 9 * 10 ** 18
+
     def __init__(self, fulfillment, public_keys=None, amount=1):
         """Create an instance of a :class:`~.Output`.
 
@@ -229,6 +231,8 @@ class Output(object):
             raise TypeError('`amount` must be an int')
         if amount < 1:
             raise AmountError('`amount` must be greater than 0')
+        if amount > self.MAX_AMOUNT:
+            raise AmountError('`amount` must be <= %s' % self.MAX_AMOUNT)
 
         self.fulfillment = fulfillment
         self.amount = amount
@@ -264,7 +268,7 @@ class Output(object):
         output = {
             'public_keys': self.public_keys,
             'condition': condition,
-            'amount': self.amount
+            'amount': str(self.amount),
         }
         return output
 
@@ -381,7 +385,11 @@ class Output(object):
         except KeyError:
             # NOTE: Hashlock condition case
             fulfillment = data['condition']['uri']
-        return cls(fulfillment, data['public_keys'], data['amount'])
+        try:
+            amount = int(data['amount'])
+        except ValueError:
+            raise AmountError('Invalid amount: %s' % data['amount'])
+        return cls(fulfillment, data['public_keys'], amount)
 
 
 class Transaction(object):

tests/assets/test_digital_assets.py

@@ -1,4 +1,3 @@
-from bigchaindb.common.exceptions import ValidationError
 import pytest
 import random
 
@@ -19,18 +18,6 @@ def test_asset_transfer(b, user_pk, user_sk):
     assert tx_transfer_signed.asset['id'] == tx_create.id
 
 
-def test_validate_bad_asset_creation(b, user_pk):
-    from bigchaindb.models import Transaction
-
-    # `data` needs to be a dictionary
-    tx = Transaction.create([b.me], [([user_pk], 1)])
-    tx.asset['data'] = 'a'
-    tx_signed = tx.sign([b.me_private])
-
-    with pytest.raises(ValidationError):
-        Transaction.from_dict(tx_signed.to_dict())
-
-
 @pytest.mark.bdb
 @pytest.mark.usefixtures('inputs')
 def test_validate_transfer_asset_id_mismatch(b, user_pk, user_sk):
@@ -91,19 +78,6 @@ def test_asset_id_mismatch(b, user_pk):
         Transaction.get_asset_id([tx1, tx2])
 
 
-def test_create_invalid_divisible_asset(b, user_pk, user_sk):
-    from bigchaindb.models import Transaction
-    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(ValidationError):
-        Transaction.from_dict(tx.to_dict())
-
-
 def test_create_valid_divisible_asset(b, user_pk, user_sk):
     from bigchaindb.models import Transaction
 

tests/assets/test_divisible_assets.py

@@ -635,88 +635,3 @@ def test_divide(b, user_pk, user_sk):
     assert len(tx_transfer_signed.outputs) == 3
     for output in tx_transfer_signed.outputs:
         assert output.amount == 1
-
-
-# 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):
-    from bigchaindb.models import Transaction
-    from bigchaindb.common.exceptions import AmountError
-
-    # CREATE divisible asset with 1 output with amount 3
-    tx_create = Transaction.create([b.me], [([user_pk], 3)])
-    tx_create_signed = tx_create.sign([b.me_private])
-    # create block
-    block = b.create_block([tx_create_signed])
-    assert block.validate(b) == block
-    b.write_block(block)
-    # vote
-    vote = b.vote(block.id, b.get_last_voted_block().id, True)
-    b.write_vote(vote)
-
-    with pytest.raises(AmountError):
-        Transaction.transfer(tx_create.to_inputs(),
-                             [([b.me], 4), ([b.me], -1)],
-                             asset_id=tx_create.id)
-
-
-# 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):
-    from bigchaindb.models import Transaction
-    from bigchaindb.common.exceptions import AmountError
-
-    # CREATE divisible asset with 1 output with amount 3
-    tx_create = Transaction.create([b.me], [([user_pk], 3)])
-    tx_create_signed = tx_create.sign([b.me_private])
-    # create block
-    block = b.create_block([tx_create_signed])
-    assert block.validate(b) == block
-    b.write_block(block)
-    # vote
-    vote = b.vote(block.id, b.get_last_voted_block().id, True)
-    b.write_vote(vote)
-
-    # create a transfer transaction with 3 outputs and check if the amount
-    # of each output is 1
-    tx_transfer = Transaction.transfer(tx_create.to_inputs(),
-                                       [([b.me], 4), ([b.me], 1)],
-                                       asset_id=tx_create.id)
-    tx_transfer.outputs[1].amount = -1
-    tx_transfer_signed = tx_transfer.sign([user_sk])
-
-    with pytest.raises(AmountError):
-        tx_transfer_signed.validate(b)
-
-
-# Check that negative inputs are caught when creating a CREATE transaction
-@pytest.mark.bdb
-@pytest.mark.usefixtures('inputs')
-def test_non_positive_amounts_on_create(b, user_pk):
-    from bigchaindb.models import Transaction
-    from bigchaindb.common.exceptions import AmountError
-
-    # CREATE divisible asset with 1 output with amount 3
-    with pytest.raises(AmountError):
-        Transaction.create([b.me], [([user_pk], -3)])
-
-
-# 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):
-    from bigchaindb.models import Transaction
-    from bigchaindb.common.exceptions import AmountError
-
-    # CREATE divisible asset with 1 output with amount 3
-    tx_create = Transaction.create([b.me], [([user_pk], 3)])
-    tx_create.outputs[0].amount = -3
-    tx_create_signed = tx_create.sign([b.me_private])
-
-    with pytest.raises(AmountError):
-        tx_create_signed.validate(b)

tests/common/schema/test_transaction_schema.py

@@ -1,60 +0,0 @@
-from pytest import raises
-
-from bigchaindb.common.exceptions import SchemaValidationError
-from bigchaindb.common.schema import validate_transaction_schema
-
-
-def test_validate_transaction_create(create_tx):
-    validate_transaction_schema(create_tx.to_dict())
-
-
-def test_validate_transaction_signed_create(signed_create_tx):
-    validate_transaction_schema(signed_create_tx.to_dict())
-
-
-def test_validate_transaction_signed_transfer(signed_transfer_tx):
-    validate_transaction_schema(signed_transfer_tx.to_dict())
-
-
-def test_validate_transaction_fails():
-    with raises(SchemaValidationError):
-        validate_transaction_schema({})
-
-
-def test_validate_fails_metadata_empty_dict(create_tx):
-    create_tx.metadata = {'a': 1}
-    validate_transaction_schema(create_tx.to_dict())
-    create_tx.metadata = None
-    validate_transaction_schema(create_tx.to_dict())
-    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/schema/test_vote_schema.py

@@ -1,13 +0,0 @@
-from pytest import raises
-
-from bigchaindb.common.exceptions import SchemaValidationError
-from bigchaindb.common.schema import validate_vote_schema
-
-
-def test_validate_vote(structurally_valid_vote):
-    validate_vote_schema(structurally_valid_vote)
-
-
-def test_validate_vote_fails():
-    with raises(SchemaValidationError):
-        validate_vote_schema({})

tests/common/test_schema.py

@@ -1,6 +1,18 @@
-from bigchaindb.common.schema import (
+"""
-    TX_SCHEMA_COMMON, VOTE_SCHEMA, drop_schema_descriptions)
+This module is tests related to schema checking, but _not_ of granular schematic
+properties related to validation.
+"""
 
+from pytest import raises
+
+from bigchaindb.common.exceptions import SchemaValidationError
+from bigchaindb.common.schema import (
+    TX_SCHEMA_COMMON, VOTE_SCHEMA, drop_schema_descriptions,
+    validate_transaction_schema, validate_vote_schema)
+
+
+################################################################################
+# Test of schema utils
 
 def _test_additionalproperties(node, path=''):
     """
@@ -67,3 +79,37 @@ def test_drop_descriptions():
     }
     drop_schema_descriptions(node)
     assert node == expected
+
+
+################################################################################
+# Test call transaction schema
+
+
+def test_validate_transaction_create(create_tx):
+    validate_transaction_schema(create_tx.to_dict())
+
+
+def test_validate_transaction_signed_create(signed_create_tx):
+    validate_transaction_schema(signed_create_tx.to_dict())
+
+
+def test_validate_transaction_signed_transfer(signed_transfer_tx):
+    validate_transaction_schema(signed_transfer_tx.to_dict())
+
+
+def test_validate_transaction_fails():
+    with raises(SchemaValidationError):
+        validate_transaction_schema({})
+
+
+################################################################################
+# Test call vote schema
+
+
+def test_validate_vote(structurally_valid_vote):
+    validate_vote_schema(structurally_valid_vote)
+
+
+def test_validate_vote_fails():
+    with raises(SchemaValidationError):
+        validate_vote_schema({})

tests/common/test_transaction.py

@@ -1,3 +1,8 @@
+"""
+These are tests of the API of the Transaction class and associated classes.
+Tests for transaction validation are separate.
+"""
+
 from pytest import raises
 
 
@@ -78,7 +83,7 @@ def test_output_serialization(user_Ed25519, user_pub):
             'details': user_Ed25519.to_dict(),
         },
         'public_keys': [user_pub],
-        'amount': 1,
+        'amount': '1',
     }
 
     cond = Output(user_Ed25519, [user_pub], 1)
@@ -96,7 +101,7 @@ def test_output_deserialization(user_Ed25519, user_pub):
             'details': user_Ed25519.to_dict()
         },
         'public_keys': [user_pub],
-        'amount': 1,
+        'amount': '1',
     }
     cond = Output.from_dict(cond)
 
@@ -115,7 +120,7 @@ def test_output_hashlock_serialization():
             'uri': hashlock,
         },
         'public_keys': None,
-        'amount': 1,
+        'amount': '1',
     }
     cond = Output(hashlock, amount=1)
 
@@ -135,7 +140,7 @@ def test_output_hashlock_deserialization():
             'uri': hashlock
         },
         'public_keys': None,
-        'amount': 1,
+        'amount': '1',
     }
     cond = Output.from_dict(cond)
 
@@ -341,28 +346,6 @@ def test_transaction_deserialization(user_input, user_output, data):
     validate_transaction_model(tx)
 
 
-def test_tx_serialization_with_incorrect_hash(utx):
-    from bigchaindb.common.transaction import Transaction
-    from bigchaindb.common.exceptions import InvalidHash
-
-    utx_dict = utx.to_dict()
-    utx_dict['id'] = 'a' * 64
-    with raises(InvalidHash):
-        Transaction.from_dict(utx_dict)
-    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
 
@@ -984,35 +967,11 @@ def test_cant_add_empty_input():
         tx.add_input(None)
 
 
-def test_validate_version(utx):
+def test_output_from_dict_invalid_amount(user_output):
-    import re
+    from bigchaindb.common.transaction import Output
-    import bigchaindb.version
+    from bigchaindb.common.exceptions import AmountError
-    from .utils import validate_transaction_model
-    from bigchaindb.common.exceptions import SchemaValidationError
 
-    short_ver = bigchaindb.version.__short_version__
+    out = user_output.to_dict()
-    assert utx.version == re.match(r'^(.*\d)', short_ver).group(1)
+    out['amount'] = 'a'
-
+    with raises(AmountError):
-    validate_transaction_model(utx)
+        Output.from_dict(out)
-
-    # At version 1, transaction version will break step with server version.
-    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/validation/test_transaction_structure.py

@@ -0,0 +1,159 @@
+"""
+All tests of transaction structure. The concern here is that transaction
+structural / schematic issues are caught when reading a transaction
+(ie going from dict -> transaction).
+"""
+
+import pytest
+
+from bigchaindb.common.exceptions import (AmountError, InvalidHash,
+                                          SchemaValidationError)
+from bigchaindb.models import Transaction
+
+
+################################################################################
+# Helper functions
+
+
+def validate(tx):
+    if isinstance(tx, Transaction):
+        tx = tx.to_dict()
+    Transaction.from_dict(tx)
+
+
+def validate_raises(tx, exc=SchemaValidationError):
+    with pytest.raises(exc):
+        validate(tx)
+
+
+# We should test that validation works when we expect it to
+def test_validation_passes(create_tx):
+    validate(create_tx)
+
+
+################################################################################
+# ID
+
+
+def test_tx_serialization_hash_function(create_tx):
+    import sha3
+    import json
+    tx = create_tx.to_dict()
+    tx['inputs'][0]['fulfillment'] = None
+    del tx['id']
+    payload = json.dumps(tx, skipkeys=False, sort_keys=True,
+                         separators=(',', ':'))
+    assert sha3.sha3_256(payload.encode()).hexdigest() == create_tx.id
+
+
+def test_tx_serialization_with_incorrect_hash(create_tx):
+    tx = create_tx.to_dict()
+    tx['id'] = 'a' * 64
+    validate_raises(tx, InvalidHash)
+
+
+################################################################################
+# Operation
+
+def test_validate_invalid_operation(create_tx):
+    create_tx.operation = 'something invalid'
+    validate_raises(create_tx)
+
+
+################################################################################
+# Metadata
+
+def test_validate_fails_metadata_empty_dict(create_tx):
+    create_tx.metadata = {'a': 1}
+    validate(create_tx)
+    create_tx.metadata = None
+    validate(create_tx)
+    create_tx.metadata = {}
+    validate_raises(create_tx)
+
+
+################################################################################
+# Asset
+
+def test_transfer_asset_schema(signed_transfer_tx):
+    tx = signed_transfer_tx.to_dict()
+    validate(tx)
+    tx['asset']['data'] = {}
+    validate_raises(tx)
+    del tx['asset']['data']
+    tx['asset']['id'] = 'b' * 63
+    validate_raises(tx)
+
+
+def test_create_tx_no_asset_id(create_tx):
+    create_tx.asset['id'] = 'b' * 64
+    validate_raises(create_tx)
+
+
+def test_create_tx_asset_type(create_tx):
+    create_tx.asset['data'] = 'a'
+    validate_raises(create_tx)
+
+
+################################################################################
+# Inputs
+
+def test_no_inputs(create_tx):
+    create_tx.inputs = []
+    validate_raises(create_tx)
+
+
+def test_create_single_input(create_tx):
+    tx = create_tx.to_dict()
+    tx['inputs'] += tx['inputs']
+    validate_raises(tx)
+    tx['inputs'] = []
+    validate_raises(tx)
+
+
+def test_create_tx_no_fulfills(create_tx):
+    tx = create_tx.to_dict()
+    tx['inputs'][0]['fulfills'] = {'tx': 'a' * 64, 'output': 0}
+    validate_raises(tx)
+
+
+################################################################################
+# Outputs
+
+
+def test_low_amounts(create_tx, signed_transfer_tx):
+    for tx in [create_tx, signed_transfer_tx]:
+        tx.outputs[0].amount = 0
+        validate_raises(tx, AmountError)
+        tx.outputs[0].amount = -1
+        validate_raises(tx)
+
+
+def test_high_amounts(create_tx):
+    # Should raise a SchemaValidationError - don't want to allow ridiculously
+    # large numbers to get converted to int
+    create_tx.outputs[0].amount = 10 ** 21
+    validate_raises(create_tx)
+    # Should raise AmountError
+    create_tx.outputs[0].amount = 9 * 10 ** 18 + 1
+    validate_raises(create_tx, AmountError)
+    # Should pass
+    create_tx.outputs[0].amount -= 1
+    validate(create_tx)
+
+
+################################################################################
+# Version
+
+def test_validate_version(create_tx):
+    import re
+    import bigchaindb.version
+
+    short_ver = bigchaindb.version.__short_version__
+    assert create_tx.version == re.match(r'^(.*\d)', short_ver).group(1)
+
+    validate(create_tx)
+
+    # At version 1, transaction version will break step with server version.
+    create_tx.version = '1.0.0'
+    validate_raises(create_tx)