From 843d65d2335de9de5797ec5b27e5674a1432a5f8 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Wed, 22 Feb 2017 13:26:23 +0100 Subject: [PATCH 1/3] bigchain.is_new_transaction method --- bigchaindb/core.py | 16 +++++++++++++++ tests/db/test_bigchain_api.py | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index a7ed93f0..a4a65482 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -178,6 +178,22 @@ class Bigchain(object): 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 + VALID or UNDECIDED block. Return False otherwise. + + Args: + txid (str): Transaction ID + exclude_block_id (str): Exclude block from search + """ + block_statuses = self.get_blocks_status_containing_tx(txid) + block_statuses.pop(exclude_block_id, None) + for status in block_statuses.values(): + if status != self.BLOCK_INVALID: + return False + return True + def get_block(self, block_id, include_status=False): """Get the block with the specified `block_id` (and optionally its status) diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index 31abe176..c5b61c88 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -1240,3 +1240,40 @@ def test_transaction_unicode(b): assert b.get_block(block.id) == block.to_dict() assert block.validate(b) == block assert beer_json in serialize(block.to_dict()) + + +@pytest.mark.bdb +def test_is_new_transaction(b, genesis_block): + from bigchaindb.models import Transaction + + def write_tx(n): + tx = Transaction.create([b.me], [([b.me], n)]) + tx = tx.sign([b.me_private]) + # Tx is new because it's not in any block + assert b.is_new_transaction(tx.id) + + block = b.create_block([tx]) + b.write_block(block) + return tx, block + + # test VALID case + tx, block = write_tx(1) + # Tx is now in undecided block + assert not b.is_new_transaction(tx.id) + assert b.is_new_transaction(tx.id, exclude_block_id=block.id) + # After voting valid, should not be new + vote = b.vote(block.id, genesis_block.id, True) + b.write_vote(vote) + assert not b.is_new_transaction(tx.id) + assert b.is_new_transaction(tx.id, exclude_block_id=block.id) + + # test INVALID case + tx, block = write_tx(2) + # Tx is now in undecided block + assert not b.is_new_transaction(tx.id) + assert b.is_new_transaction(tx.id, exclude_block_id=block.id) + vote = b.vote(block.id, genesis_block.id, False) + b.write_vote(vote) + # 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) From fc2b684f32d8476aba34ccd2c9fdb93c5e15131f Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Wed, 22 Feb 2017 13:52:53 +0100 Subject: [PATCH 2/3] use is_new_transaction in pipelines --- bigchaindb/pipelines/block.py | 29 ++++++++++------------------- bigchaindb/pipelines/vote.py | 10 ++++++++-- tests/pipelines/test_vote.py | 14 ++++++++++++++ 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/bigchaindb/pipelines/block.py b/bigchaindb/pipelines/block.py index 5f372384..1f2e9017 100644 --- a/bigchaindb/pipelines/block.py +++ b/bigchaindb/pipelines/block.py @@ -67,28 +67,19 @@ class BlockPipeline: AmountError): return None - if self.bigchain.transaction_exists(tx.id): - # if the transaction already exists, we must check whether - # it's in a valid or undecided block - tx, status = self.bigchain.get_transaction(tx.id, - include_status=True) - if status == self.bigchain.TX_VALID \ - or status == self.bigchain.TX_UNDECIDED: - # if the tx is already in a valid or undecided block, - # then it no longer should be in the backlog, or added - # to a new block. We can delete and drop it. - self.bigchain.delete_transaction(tx.id) - return None - - tx_validated = self.bigchain.is_valid_transaction(tx) - if tx_validated: - return tx - else: - # if the transaction is not valid, remove it from the - # backlog + # If transaction is in any VALID or UNDECIDED block we + # should not include it again + if not self.bigchain.is_new_transaction(tx.id): self.bigchain.delete_transaction(tx.id) return None + # If transaction is not valid it should not be included + if not self.bigchain.is_valid_transaction(tx): + self.bigchain.delete_transaction(tx.id) + return None + + return tx + def create(self, tx, timeout=False): """Create a block. diff --git a/bigchaindb/pipelines/vote.py b/bigchaindb/pipelines/vote.py index 5f385f53..958e5b9c 100644 --- a/bigchaindb/pipelines/vote.py +++ b/bigchaindb/pipelines/vote.py @@ -90,7 +90,8 @@ class Vote: yield tx, block_id, num_tx def validate_tx(self, tx, block_id, num_tx): - """Validate a transaction. + """Validate a transaction. Transaction must also not be in any VALID + block. Args: tx (dict): the transaction to validate @@ -101,7 +102,12 @@ class Vote: Three values are returned, the validity of the transaction, ``block_id``, ``num_tx``. """ - return bool(self.bigchain.is_valid_transaction(tx)), block_id, num_tx + new = self.bigchain.is_new_transaction(tx.id, exclude_block_id=block_id) + if not new: + return False, block_id, num_tx + + valid = bool(self.bigchain.is_valid_transaction(tx)) + return valid, block_id, num_tx def vote(self, tx_validity, block_id, num_tx): """Collect the validity of transactions and cast a vote when ready. diff --git a/tests/pipelines/test_vote.py b/tests/pipelines/test_vote.py index e0b27f50..20beac1e 100644 --- a/tests/pipelines/test_vote.py +++ b/tests/pipelines/test_vote.py @@ -629,3 +629,17 @@ def test_start(mock_start, b): from bigchaindb.pipelines import vote vote.start() mock_start.assert_called_with() + + +@pytest.mark.genesis +def test_vote_no_double_inclusion(b): + from bigchaindb.pipelines import vote + + tx = dummy_tx(b) + block = b.create_block([tx]) + r = vote.Vote().validate_tx(tx, block.id, 1) + assert r == (True, block.id, 1) + + b.write_block(block) + r = vote.Vote().validate_tx(tx, 'other_block_id', 1) + assert r == (False, 'other_block_id', 1) From 6e7534d3c23f5bcf486b767dca1dede3e3ec3cfc Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Wed, 22 Feb 2017 13:55:26 +0100 Subject: [PATCH 3/3] cleanup; remove transaction_exists and has_transaction --- bigchaindb/backend/mongodb/query.py | 7 ------- bigchaindb/backend/query.py | 14 -------------- bigchaindb/backend/rethinkdb/query.py | 7 ------- bigchaindb/core.py | 3 --- tests/backend/mongodb/test_queries.py | 13 ------------- tests/backend/test_generics.py | 1 - tests/test_core.py | 9 --------- 7 files changed, 54 deletions(-) diff --git a/bigchaindb/backend/mongodb/query.py b/bigchaindb/backend/mongodb/query.py index 6a241a78..1988db04 100644 --- a/bigchaindb/backend/mongodb/query.py +++ b/bigchaindb/backend/mongodb/query.py @@ -212,13 +212,6 @@ def get_block(conn, block_id): projection={'_id': False})) -@register_query(MongoDBConnection) -def has_transaction(conn, transaction_id): - return bool(conn.run( - conn.collection('bigchain') - .find_one({'block.transactions.id': transaction_id}))) - - @register_query(MongoDBConnection) def count_blocks(conn): return conn.run( diff --git a/bigchaindb/backend/query.py b/bigchaindb/backend/query.py index bdfb9e28..9aa653d7 100644 --- a/bigchaindb/backend/query.py +++ b/bigchaindb/backend/query.py @@ -211,20 +211,6 @@ def get_block(connection, block_id): raise NotImplementedError -@singledispatch -def has_transaction(connection, transaction_id): - """Check if a transaction exists in the bigchain table. - - Args: - transaction_id (str): the id of the transaction to check. - - Returns: - ``True`` if the transaction exists, ``False`` otherwise. - """ - - raise NotImplementedError - - @singledispatch def count_blocks(connection): """Count the number of blocks in the bigchain table. diff --git a/bigchaindb/backend/rethinkdb/query.py b/bigchaindb/backend/rethinkdb/query.py index 99346984..6011cc8c 100644 --- a/bigchaindb/backend/rethinkdb/query.py +++ b/bigchaindb/backend/rethinkdb/query.py @@ -158,13 +158,6 @@ def get_block(connection, block_id): return connection.run(r.table('bigchain').get(block_id)) -@register_query(RethinkDBConnection) -def has_transaction(connection, transaction_id): - return bool(connection.run( - r.table('bigchain', read_mode=READ_MODE) - .get_all(transaction_id, index='transaction_id').count())) - - @register_query(RethinkDBConnection) def count_blocks(connection): return connection.run( diff --git a/bigchaindb/core.py b/bigchaindb/core.py index a4a65482..b5465ef5 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -542,9 +542,6 @@ class Bigchain(object): return backend.query.write_block(self.connection, block) - def transaction_exists(self, transaction_id): - return backend.query.has_transaction(self.connection, transaction_id) - def prepare_genesis_block(self): """Prepare a genesis block.""" diff --git a/tests/backend/mongodb/test_queries.py b/tests/backend/mongodb/test_queries.py index 48089805..80e3cc91 100644 --- a/tests/backend/mongodb/test_queries.py +++ b/tests/backend/mongodb/test_queries.py @@ -248,19 +248,6 @@ def test_get_block(signed_create_tx): assert block_db == block.to_dict() -def test_has_transaction(signed_create_tx): - from bigchaindb.backend import connect, query - from bigchaindb.models import Block - conn = connect() - - # create and insert block - block = Block(transactions=[signed_create_tx]) - conn.db.bigchain.insert_one(block.to_dict()) - - assert query.has_transaction(conn, signed_create_tx.id) - assert query.has_transaction(conn, 'aaa') is False - - def test_count_blocks(signed_create_tx): from bigchaindb.backend import connect, query from bigchaindb.models import Block diff --git a/tests/backend/test_generics.py b/tests/backend/test_generics.py index 2f86d417..57a644ee 100644 --- a/tests/backend/test_generics.py +++ b/tests/backend/test_generics.py @@ -29,7 +29,6 @@ def test_schema(schema_func_name, args_qty): ('get_votes_by_block_id', 1), ('write_block', 1), ('get_block', 1), - ('has_transaction', 1), ('write_vote', 1), ('get_last_voted_block', 1), ('get_unvoted_blocks', 1), diff --git a/tests/test_core.py b/tests/test_core.py index 6bcabdc9..977db065 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -88,12 +88,3 @@ def test_has_previous_vote(monkeypatch): block = {'votes': ({'node_pubkey': 'pubkey'},)} with pytest.raises(Exception): bigchain.has_previous_vote(block) - - -@pytest.mark.parametrize('exists', (True, False)) -def test_transaction_exists(monkeypatch, exists): - from bigchaindb.core import Bigchain - monkeypatch.setattr( - 'bigchaindb.backend.query.has_transaction', lambda x, y: exists) - bigchain = Bigchain(public_key='pubkey', private_key='privkey') - assert bigchain.transaction_exists('txid') is exists