From 7c461e47d7c6fb77ac3f70eb307fa979d111c083 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Tue, 14 Mar 2017 12:39:57 +0100 Subject: [PATCH] remove structural and schematic validations from Transaction.validate which is validating the spend --- .../common/schema/transaction_transfer.yaml | 11 +++++ bigchaindb/models.py | 40 ++----------------- tests/assets/test_digital_assets.py | 8 ++-- tests/assets/test_divisible_assets.py | 3 ++ tests/common/test_transaction.py | 9 +++++ tests/db/test_bigchain_api.py | 27 ------------- 6 files changed, 30 insertions(+), 68 deletions(-) diff --git a/bigchaindb/common/schema/transaction_transfer.yaml b/bigchaindb/common/schema/transaction_transfer.yaml index 80abbf95..09a5aa1b 100644 --- a/bigchaindb/common/schema/transaction_transfer.yaml +++ b/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}" diff --git a/bigchaindb/models.py b/bigchaindb/models.py index a46f2b73..43313a2b 100644 --- a/bigchaindb/models.py +++ b/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.') diff --git a/tests/assets/test_digital_assets.py b/tests/assets/test_digital_assets.py index d44bc52c..c31ec3da 100644 --- a/tests/assets/test_digital_assets.py +++ b/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): - b.validate_transaction(tx) + with pytest.raises(ValidationError): + Transaction.from_dict(tx.to_dict()) def test_create_valid_divisible_asset(b, user_pk, user_sk): diff --git a/tests/assets/test_divisible_assets.py b/tests/assets/test_divisible_assets.py index 31e7890f..87a29c2b 100644 --- a/tests/assets/test_divisible_assets.py +++ b/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): diff --git a/tests/common/test_transaction.py b/tests/common/test_transaction.py index 5b06f801..45cadc3b 100644 --- a/tests/common/test_transaction.py +++ b/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) diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index c39a104f..f1062777 100644 --- a/tests/db/test_bigchain_api.py +++ b/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)