From 73d6add36ffd5492fe7f0bd7bbaa14fd0b735c4d Mon Sep 17 00:00:00 2001 From: vrde Date: Wed, 13 Jul 2016 17:30:13 +0200 Subject: [PATCH 01/18] Remove block_number [pair prog w/ @rhsimplex] --- bigchaindb/core.py | 34 ++++++++++++------------- bigchaindb/db/utils.py | 2 -- bigchaindb/util.py | 15 +++++++++++ bigchaindb/voter.py | 17 +++++-------- tests/conftest.py | 1 + tests/db/test_bigchain_api.py | 48 ++++++++++++++++++----------------- tests/db/test_utils.py | 3 +-- tests/db/test_voter.py | 5 +--- tests/test_util.py | 6 +++++ 9 files changed, 71 insertions(+), 60 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 3c9e3d3f..956c8924 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -492,6 +492,16 @@ class Bigchain(object): response = r.table('bigchain').get_all(transaction_id, index='transaction_id').run(self.conn) return True if len(response.items) > 0 else False + def prepare_genesis_block(self): + """Prepare a genesis block.""" + + payload = {'message': 'Hello World from the BigchainDB'} + transaction = self.create_transaction([self.me], [self.me], None, 'GENESIS', payload=payload) + transaction_signed = self.sign_transaction(transaction, self.me_private) + + # create the block + return self.create_block([transaction_signed]) + # TODO: Unless we prescribe the signature of create_transaction, this will # also need to be moved into the plugin API. def create_genesis_block(self): @@ -511,14 +521,7 @@ class Bigchain(object): if blocks_count: raise exceptions.GenesisBlockAlreadyExistsError('Cannot create the Genesis block') - payload = {'message': 'Hello World from the BigchainDB'} - transaction = self.create_transaction([self.me], [self.me], None, 'GENESIS', payload=payload) - transaction_signed = self.sign_transaction(transaction, self.me_private) - - # create the block - block = self.create_block([transaction_signed]) - # add block number before writing - block['block_number'] = 0 + block = self.prepare_genesis_block() self.write_block(block, durability='hard') return block @@ -553,18 +556,13 @@ class Bigchain(object): return vote_signed - def write_vote(self, block, vote, block_number): + def write_vote(self, block, vote): """Write the vote to the database.""" # First, make sure this block doesn't contain a vote from this node if self.has_previous_vote(block): return None - # We need to *not* override the existing block_number, if any - # FIXME: MIGHT HAVE RACE CONDITIONS WITH THE OTHER NODES IN THE FEDERATION - if 'block_number' not in vote: - vote['block_number'] = block_number # maybe this should be in the signed part...or better yet, removed.. - r.table('votes') \ .insert(vote) \ .run(self.conn) @@ -574,14 +572,14 @@ class Bigchain(object): last_voted = r.table('votes') \ .filter(r.row['node_pubkey'] == self.me) \ - .order_by(r.desc('block_number')) \ + .order_by(r.desc(r.row['vote']['timestamp'])) \ .limit(1) \ .run(self.conn) # return last vote if last vote exists else return Genesis block if not last_voted: return list(r.table('bigchain') - .filter(r.row['block_number'] == 0) + .filter(util.is_genesis_block) .run(self.conn))[0] res = r.table('bigchain').get(last_voted[0]['vote']['voting_for_block']).run(self.conn) @@ -597,10 +595,10 @@ class Bigchain(object): unvoted = r.table('bigchain') \ .filter(lambda block: r.table('votes').get_all([block['id'], self.me], index='block_and_voter') .is_empty()) \ - .order_by(r.desc('block_number')) \ + .order_by(r.desc(r.row['block']['timestamp'])) \ .run(self.conn) - if unvoted and unvoted[0].get('block_number') == 0: + if unvoted and util.is_genesis_block(unvoted[0]): unvoted.pop(0) return unvoted diff --git a/bigchaindb/db/utils.py b/bigchaindb/db/utils.py index 482d27fc..252527d7 100644 --- a/bigchaindb/db/utils.py +++ b/bigchaindb/db/utils.py @@ -42,8 +42,6 @@ def init(): # create the secondary indexes # to order blocks by timestamp r.db(dbname).table('bigchain').index_create('block_timestamp', r.row['block']['timestamp']).run(conn) - # to order blocks by block number - r.db(dbname).table('bigchain').index_create('block_number', r.row['block']['block_number']).run(conn) # to order transactions by timestamp r.db(dbname).table('backlog').index_create('transaction_timestamp', r.row['transaction']['timestamp']).run(conn) # to query the bigchain for a transaction id diff --git a/bigchaindb/util.py b/bigchaindb/util.py index b16cdd78..8492cc81 100644 --- a/bigchaindb/util.py +++ b/bigchaindb/util.py @@ -615,3 +615,18 @@ def transform_create(tx): new_tx = create_tx(b.me, transaction['fulfillments'][0]['current_owners'], None, 'CREATE', payload=payload) return new_tx + +def is_genesis_block(block): + """Check if the block is the genesis block. + + Args: + block (dict): the block to check + + Returns: + bool: True if the block is the genesis block, False otherwise. + """ + + # we cannot have empty blocks, there will always be at least one + # element in the list so we can safely refer to it + return block['block']['transactions'][0]['transaction']['operation'] == 'GENESIS' + diff --git a/bigchaindb/voter.py b/bigchaindb/voter.py index 73e08fb9..60a029db 100644 --- a/bigchaindb/voter.py +++ b/bigchaindb/voter.py @@ -65,7 +65,6 @@ class Voter(object): self.q_validated_block = mp.Queue() self.q_voted_block = mp.Queue() self.v_previous_block_id = mp.Value(ctypes.c_char_p) - self.v_previous_block_number = mp.Value(ctypes.c_uint64) self.initialized = mp.Event() def feed_blocks(self): @@ -105,18 +104,15 @@ class Voter(object): return logger.info('new_block arrived to voter') - block_number = self.v_previous_block_number.value + 1 with self.monitor.timer('validate_block'): validity = b.is_valid_block(new_block) self.q_validated_block.put((new_block, self.v_previous_block_id.value.decode(), - block_number, validity)) self.v_previous_block_id.value = new_block['id'].encode() - self.v_previous_block_number.value = block_number def vote(self): """ @@ -134,9 +130,9 @@ class Voter(object): self.q_voted_block.put('stop') return - validated_block, previous_block_id, block_number, decision = elem + validated_block, previous_block_id, decision = elem vote = b.vote(validated_block, previous_block_id, decision) - self.q_voted_block.put((validated_block, vote, block_number)) + self.q_voted_block.put((validated_block, vote)) def update_block(self): """ @@ -154,22 +150,21 @@ class Voter(object): logger.info('clean exit') return - block, vote, block_number = elem - logger.info('updating block %s with number %s and with vote %s', block['id'], block_number, vote) - b.write_vote(block, vote, block_number) + block, vote = elem + logger.info('updating block %s and with vote %s', block['id'], vote) + b.write_vote(block, vote) def bootstrap(self): """ Before starting handling the new blocks received by the changefeed we need to handle unvoted blocks added to the bigchain while the process was down - We also need to set the previous_block_id and the previous block_number + We also need to set the previous_block_id. """ b = Bigchain() last_voted = b.get_last_voted_block() - self.v_previous_block_number.value = last_voted['block_number'] self.v_previous_block_id.value = last_voted['id'].encode() def kill(self): diff --git a/tests/conftest.py b/tests/conftest.py index 7e3f5c56..bbf705a4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,7 @@ Tasks: import os import copy +from functools import partial import pytest diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index a416e730..61d58e1e 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -140,7 +140,7 @@ class TestBigchainApi(object): # vote the block invalid vote = b.vote(block, b.get_last_voted_block()['id'], False) - b.write_vote(block, vote, 3) + b.write_vote(block, vote) response = b.get_transaction(tx_signed["id"]) # should be None, because invalid blocks are ignored @@ -181,10 +181,9 @@ class TestBigchainApi(object): @pytest.mark.usefixtures('inputs') def test_genesis_block(self, b): response = list(r.table('bigchain') - .filter(r.row['block_number'] == 0) + .filter(util.is_genesis_block) .run(b.conn))[0] - assert response['block_number'] == 0 assert len(response['block']['transactions']) == 1 assert response['block']['transactions'][0]['transaction']['operation'] == 'GENESIS' assert response['block']['transactions'][0]['transaction']['fulfillments'][0]['input'] is None @@ -196,7 +195,7 @@ class TestBigchainApi(object): b.create_genesis_block() genesis_blocks = list(r.table('bigchain') - .filter(r.row['block_number'] == 0) + .filter(util.is_genesis_block) .run(b.conn)) assert len(genesis_blocks) == 1 @@ -257,11 +256,11 @@ class TestBigchainApi(object): def test_get_last_voted_block_returns_genesis_if_no_votes_has_been_casted(self, b): b.create_genesis_block() genesis = list(r.table('bigchain') - .filter(r.row['block_number'] == 0) + .filter(util.is_genesis_block) .run(b.conn))[0] assert b.get_last_voted_block() == genesis - def test_get_last_voted_block_returns_the_correct_block(self, b): + def test_get_last_voted_block_returns_the_correct_block(self, b, monkeypatch): genesis = b.create_genesis_block() assert b.get_last_voted_block() == genesis @@ -274,13 +273,16 @@ class TestBigchainApi(object): b.write_block(block_2, durability='hard') b.write_block(block_3, durability='hard') - b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block(), True), 1) + monkeypatch.setattr(util, 'timestamp', lambda: 1) + b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block()['id'], True)) assert b.get_last_voted_block()['id'] == block_1['id'] - b.write_vote(block_2, b.vote(block_2, b.get_last_voted_block(), True), 2) + monkeypatch.setattr(util, 'timestamp', lambda: 2) + b.write_vote(block_2, b.vote(block_2, b.get_last_voted_block()['id'], True)) assert b.get_last_voted_block()['id'] == block_2['id'] - b.write_vote(block_3, b.vote(block_3, b.get_last_voted_block(), True), 3) + monkeypatch.setattr(util, 'timestamp', lambda: 3) + b.write_vote(block_3, b.vote(block_3, b.get_last_voted_block()['id'], True)) assert b.get_last_voted_block()['id'] == block_3['id'] def test_no_vote_written_if_block_already_has_vote(self, b): @@ -290,11 +292,11 @@ class TestBigchainApi(object): b.write_block(block_1, durability='hard') - b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block(), True), 1) + b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block()['id'], True)) retrieved_block_1 = r.table('bigchain').get(block_1['id']).run(b.conn) # try to vote again on the retrieved block, should do nothing - b.write_vote(retrieved_block_1, b.vote(retrieved_block_1, b.get_last_voted_block(), True), 1) + b.write_vote(retrieved_block_1, b.vote(retrieved_block_1, b.get_last_voted_block()['id'], True)) retrieved_block_2 = r.table('bigchain').get(block_1['id']).run(b.conn) assert retrieved_block_1 == retrieved_block_2 @@ -304,8 +306,8 @@ class TestBigchainApi(object): block_1 = dummy_block() b.write_block(block_1, durability='hard') # insert duplicate votes - vote_1 = b.vote(block_1, b.get_last_voted_block(), True) - vote_2 = b.vote(block_1, b.get_last_voted_block(), True) + vote_1 = b.vote(block_1, b.get_last_voted_block()['id'], True) + vote_2 = b.vote(block_1, b.get_last_voted_block()['id'], True) vote_2['node_pubkey'] = 'aaaaaaa' r.table('votes').insert(vote_1).run(b.conn) r.table('votes').insert(vote_2).run(b.conn) @@ -322,7 +324,7 @@ class TestBigchainApi(object): b.write_block(block_1, durability='hard') # insert duplicate votes for i in range(2): - r.table('votes').insert(b.vote(block_1, b.get_last_voted_block(), True)).run(b.conn) + r.table('votes').insert(b.vote(block_1, b.get_last_voted_block()['id'], True)).run(b.conn) from bigchaindb.exceptions import MultipleVotesError with pytest.raises(MultipleVotesError) as excinfo: @@ -339,9 +341,9 @@ class TestBigchainApi(object): b.create_genesis_block() block_1 = dummy_block() b.write_block(block_1, durability='hard') - vote_1 = b.vote(block_1, b.get_last_voted_block(), True) + vote_1 = b.vote(block_1, b.get_last_voted_block()['id'], True) # mangle the signature - vote_1['signature'] = vote_1['signature'][2:] + vote_1['signature'][:1] + vote_1['signature'] = 'a' * 87 r.table('votes').insert(vote_1).run(b.conn) from bigchaindb.exceptions import ImproperVoteError with pytest.raises(ImproperVoteError) as excinfo: @@ -896,9 +898,9 @@ class TestBigchainBlock(object): b.write_block(block_2, durability='hard') b.write_block(block_3, durability='hard') - b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block(), True), 1) - b.write_vote(block_2, b.vote(block_2, b.get_last_voted_block(), True), 2) - b.write_vote(block_3, b.vote(block_3, b.get_last_voted_block(), True), 3) + b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block()['id'], True)) + b.write_vote(block_2, b.vote(block_2, b.get_last_voted_block()['id'], True)) + b.write_vote(block_3, b.vote(block_3, b.get_last_voted_block()['id'], True)) q_revert_delete = mp.Queue() @@ -1195,7 +1197,7 @@ class TestMultipleInputs(object): # vote the block VALID vote = b.vote(block, b.get_unvoted_blocks()[0]['id'], True) - b.write_vote(block, vote, 2) + b.write_vote(block, vote) # get input owned_inputs_user1 = b.get_owned_ids(user_vk) @@ -1211,7 +1213,7 @@ class TestMultipleInputs(object): # vote the block invalid vote = b.vote(block, b.get_last_voted_block()['id'], False) - b.write_vote(block, vote, 3) + b.write_vote(block, vote) owned_inputs_user1 = b.get_owned_ids(user_vk) owned_inputs_user2 = b.get_owned_ids(user2_vk) @@ -1323,7 +1325,7 @@ class TestMultipleInputs(object): # vote the block VALID vote = b.vote(block, b.get_unvoted_blocks()[0]['id'], True) - b.write_vote(block, vote, 2) + b.write_vote(block, vote) # get input owned_inputs_user1 = b.get_owned_ids(user_vk) @@ -1340,7 +1342,7 @@ class TestMultipleInputs(object): # vote the block invalid vote = b.vote(block, b.get_last_voted_block()['id'], False) - b.write_vote(block, vote, 2) + b.write_vote(block, vote) response = b.get_transaction(tx_signed["id"]) spent_inputs_user1 = b.get_spent(owned_inputs_user1[0]) diff --git a/tests/db/test_utils.py b/tests/db/test_utils.py index 9e032b25..d521fa21 100644 --- a/tests/db/test_utils.py +++ b/tests/db/test_utils.py @@ -30,8 +30,7 @@ def test_init_creates_db_tables_and_indexes(): assert r.db(dbname).table_list().contains('backlog', 'bigchain').run(conn) is True assert r.db(dbname).table('bigchain').index_list().contains( - 'block_timestamp', - 'block_number').run(conn) is True + 'block_timestamp').run(conn) is True assert r.db(dbname).table('backlog').index_list().contains( 'transaction_timestamp', diff --git a/tests/db/test_voter.py b/tests/db/test_voter.py index fb0331ca..d87c7eaa 100644 --- a/tests/db/test_voter.py +++ b/tests/db/test_voter.py @@ -325,11 +325,8 @@ class TestBigchainVoter(object): .run(b.conn)) # retrieve votes - votes = r.table('votes')\ - .order_by(r.asc((r.row['block_number'])))\ - .run(b.conn) + votes = list(r.table('votes').run(b.conn)) - assert blocks[0]['block_number'] == 0 # genesis block assert votes[0]['vote']['voting_for_block'] in (blocks[1]['id'], blocks[2]['id']) assert votes[1]['vote']['voting_for_block'] in (blocks[1]['id'], blocks[2]['id']) diff --git a/tests/test_util.py b/tests/test_util.py index a8bb21ec..4a440d60 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -210,3 +210,9 @@ def test_check_hash_and_signature_invalid_signature(monkeypatch): 'bigchaindb.util.validate_fulfillments', lambda tx: False) with pytest.raises(InvalidSignature): check_hash_and_signature(transaction) + + +def test_is_genesis_block_returns_true_if_genesis(b): + from bigchaindb.util import is_genesis_block + genesis_block = b.prepare_genesis_block() + assert is_genesis_block(genesis_block) From e9b4b9994650c05f7a7ec566299c5d1ed19e39d1 Mon Sep 17 00:00:00 2001 From: vrde Date: Wed, 13 Jul 2016 17:54:16 +0200 Subject: [PATCH 02/18] [wip] Attempt to remove genesis block --- bigchaindb/core.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 956c8924..0ab7dd86 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -598,8 +598,7 @@ class Bigchain(object): .order_by(r.desc(r.row['block']['timestamp'])) \ .run(self.conn) - if unvoted and util.is_genesis_block(unvoted[0]): - unvoted.pop(0) + unvoted = filter(lambda block: not util.is_genesis_block(block), unvoted) return unvoted From e0d0ab21d30f5b513749f00266ab35cea78fbb56 Mon Sep 17 00:00:00 2001 From: vrde Date: Thu, 14 Jul 2016 15:03:16 +0200 Subject: [PATCH 03/18] Add more blocks in inputs fixture --- bigchaindb/core.py | 4 +++- tests/db/conftest.py | 20 ++++++++++---------- tests/db/test_bigchain_api.py | 15 +++++++++------ 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 0ab7dd86..79d30baa 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -598,9 +598,11 @@ class Bigchain(object): .order_by(r.desc(r.row['block']['timestamp'])) \ .run(self.conn) + # FIXME: I (@vrde) don't like this solution. Filtering should be done at a + # database level. Solving issue #444 can help untangling the situation unvoted = filter(lambda block: not util.is_genesis_block(block), unvoted) - return unvoted + return list(unvoted) def block_election_status(self, block): """Tally the votes on a block, and return the status: valid, invalid, or undecided.""" diff --git a/tests/db/conftest.py b/tests/db/conftest.py index 900e7918..4ae4f0c3 100644 --- a/tests/db/conftest.py +++ b/tests/db/conftest.py @@ -98,7 +98,7 @@ def cleanup_tables(request, node_config): @pytest.fixture -def inputs(user_vk, amount=1, b=None): +def inputs(user_vk, amount=4, b=None, blocks=4): from bigchaindb.exceptions import GenesisBlockAlreadyExistsError # 1. create the genesis block b = b or Bigchain() @@ -108,13 +108,13 @@ def inputs(user_vk, amount=1, b=None): pass # 2. create block with transactions for `USER` to spend - transactions = [] - for i in range(amount): - tx = b.create_transaction(b.me, user_vk, None, 'CREATE') - tx_signed = b.sign_transaction(tx, b.me_private) - transactions.append(tx_signed) - b.write_transaction(tx_signed) + for block in range(blocks): + transactions = [] + for i in range(amount): + tx = b.create_transaction(b.me, user_vk, None, 'CREATE') + tx_signed = b.sign_transaction(tx, b.me_private) + transactions.append(tx_signed) + b.write_transaction(tx_signed) - block = b.create_block(transactions) - b.write_block(block, durability='hard') - return block + block = b.create_block(transactions) + b.write_block(block, durability='hard') diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index 61d58e1e..92d36857 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -182,11 +182,13 @@ class TestBigchainApi(object): def test_genesis_block(self, b): response = list(r.table('bigchain') .filter(util.is_genesis_block) - .run(b.conn))[0] + .run(b.conn)) - assert len(response['block']['transactions']) == 1 - assert response['block']['transactions'][0]['transaction']['operation'] == 'GENESIS' - assert response['block']['transactions'][0]['transaction']['fulfillments'][0]['input'] is None + assert len(response) == 1 + block = response[0] + assert len(block['block']['transactions']) == 1 + assert block['block']['transactions'][0]['transaction']['operation'] == 'GENESIS' + assert block['block']['transactions'][0]['transaction']['fulfillments'][0]['input'] is None def test_create_genesis_block_fails_if_table_not_empty(self, b): b.create_genesis_block() @@ -1988,6 +1990,7 @@ class TestCryptoconditions(object): @pytest.mark.usefixtures('inputs') def test_transfer_asset_with_hashlock_condition(self, b, user_vk, user_sk): + owned_count = len(b.get_owned_ids(user_vk)) first_input_tx = b.get_owned_ids(user_vk).pop() hashlock_tx = b.create_transaction(user_vk, None, first_input_tx, 'TRANSFER') @@ -2010,7 +2013,7 @@ class TestCryptoconditions(object): assert b.validate_transaction(hashlock_tx_signed) == hashlock_tx_signed assert b.is_valid_transaction(hashlock_tx_signed) == hashlock_tx_signed - assert len(b.get_owned_ids(user_vk)) == 1 + assert len(b.get_owned_ids(user_vk)) == owned_count b.write_transaction(hashlock_tx_signed) @@ -2018,7 +2021,7 @@ class TestCryptoconditions(object): block = b.create_block([hashlock_tx_signed]) b.write_block(block, durability='hard') - assert len(b.get_owned_ids(user_vk)) == 0 + assert len(b.get_owned_ids(user_vk)) == owned_count - 1 def test_create_and_fulfill_asset_with_hashlock_condition(self, b, user_vk): hashlock_tx = b.create_transaction(b.me, None, None, 'CREATE') From 559b6a4fe65fa41a826ce17858dd2d57d92de553 Mon Sep 17 00:00:00 2001 From: vrde Date: Thu, 14 Jul 2016 16:21:03 +0200 Subject: [PATCH 04/18] Remove stale code --- bigchaindb/core.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 79d30baa..5f97dfba 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -584,9 +584,6 @@ class Bigchain(object): res = r.table('bigchain').get(last_voted[0]['vote']['voting_for_block']).run(self.conn) - if 'block_number' in last_voted[0]: - res['block_number'] = last_voted[0]['block_number'] - return res def get_unvoted_blocks(self): From 8ac50bf0cc1448c7c2d3e1a13a21df02847162f5 Mon Sep 17 00:00:00 2001 From: vrde Date: Thu, 14 Jul 2016 17:10:55 +0200 Subject: [PATCH 05/18] Make sure we retrieve the last block in case timestamps collide --- bigchaindb/core.py | 18 ++++++++++++++++-- tests/db/test_bigchain_api.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 5f97dfba..814048cc 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -570,10 +570,10 @@ class Bigchain(object): def get_last_voted_block(self): """Returns the last block that this node voted on.""" + last_voted = r.table('votes') \ .filter(r.row['node_pubkey'] == self.me) \ .order_by(r.desc(r.row['vote']['timestamp'])) \ - .limit(1) \ .run(self.conn) # return last vote if last vote exists else return Genesis block @@ -582,7 +582,21 @@ class Bigchain(object): .filter(util.is_genesis_block) .run(self.conn))[0] - res = r.table('bigchain').get(last_voted[0]['vote']['voting_for_block']).run(self.conn) + # since the resolution of timestamp is a second, + # we might have more than one vote per timestamp + if len(last_voted) > 1: + mapping = {v['vote']['previous_block']: v['vote']['voting_for_block'] + for v in last_voted} + last_block_id = list(mapping.values())[0] + while True: + try: + last_block_id = mapping[last_block_id] + except KeyError: + break + else: + last_block_id = last_voted[0]['vote']['voting_for_block'] + + res = r.table('bigchain').get(last_block_id).run(self.conn) return res diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index 92d36857..e2d4c6e4 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -262,7 +262,7 @@ class TestBigchainApi(object): .run(b.conn))[0] assert b.get_last_voted_block() == genesis - def test_get_last_voted_block_returns_the_correct_block(self, b, monkeypatch): + def test_get_last_voted_block_returns_the_correct_block_same_timestamp(self, b, monkeypatch): genesis = b.create_genesis_block() assert b.get_last_voted_block() == genesis @@ -275,6 +275,33 @@ class TestBigchainApi(object): b.write_block(block_2, durability='hard') b.write_block(block_3, durability='hard') + # make sure all the blocks are written at the same time + monkeypatch.setattr(util, 'timestamp', lambda: 1) + + b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block()['id'], True)) + assert b.get_last_voted_block()['id'] == block_1['id'] + + b.write_vote(block_2, b.vote(block_2, b.get_last_voted_block()['id'], True)) + assert b.get_last_voted_block()['id'] == block_2['id'] + + b.write_vote(block_3, b.vote(block_3, b.get_last_voted_block()['id'], True)) + assert b.get_last_voted_block()['id'] == block_3['id'] + + + def test_get_last_voted_block_returns_the_correct_block_different_timestamps(self, b, monkeypatch): + genesis = b.create_genesis_block() + + assert b.get_last_voted_block() == genesis + + block_1 = dummy_block() + block_2 = dummy_block() + block_3 = dummy_block() + + b.write_block(block_1, durability='hard') + b.write_block(block_2, durability='hard') + b.write_block(block_3, durability='hard') + + # make sure all the blocks are written at different timestamps monkeypatch.setattr(util, 'timestamp', lambda: 1) b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block()['id'], True)) assert b.get_last_voted_block()['id'] == block_1['id'] From 7ce79c51ea1a9353fff38c4544c6698d2beb3da6 Mon Sep 17 00:00:00 2001 From: ryan Date: Fri, 15 Jul 2016 13:53:58 +0200 Subject: [PATCH 06/18] correct type for spoofed timestamps correct type for spoofed timestamps --- tests/db/test_bigchain_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index e2d4c6e4..9c5bcebd 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -276,7 +276,7 @@ class TestBigchainApi(object): b.write_block(block_3, durability='hard') # make sure all the blocks are written at the same time - monkeypatch.setattr(util, 'timestamp', lambda: 1) + monkeypatch.setattr(util, 'timestamp', lambda: '1') b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block()['id'], True)) assert b.get_last_voted_block()['id'] == block_1['id'] @@ -302,15 +302,15 @@ class TestBigchainApi(object): b.write_block(block_3, durability='hard') # make sure all the blocks are written at different timestamps - monkeypatch.setattr(util, 'timestamp', lambda: 1) + monkeypatch.setattr(util, 'timestamp', lambda: '1') b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block()['id'], True)) assert b.get_last_voted_block()['id'] == block_1['id'] - monkeypatch.setattr(util, 'timestamp', lambda: 2) + monkeypatch.setattr(util, 'timestamp', lambda: '2') b.write_vote(block_2, b.vote(block_2, b.get_last_voted_block()['id'], True)) assert b.get_last_voted_block()['id'] == block_2['id'] - monkeypatch.setattr(util, 'timestamp', lambda: 3) + monkeypatch.setattr(util, 'timestamp', lambda: '3') b.write_vote(block_3, b.vote(block_3, b.get_last_voted_block()['id'], True)) assert b.get_last_voted_block()['id'] == block_3['id'] From 5c11fc57fa2d739040a9c76f34316f6d3a3532e2 Mon Sep 17 00:00:00 2001 From: ryan Date: Fri, 15 Jul 2016 15:27:24 +0200 Subject: [PATCH 07/18] don't return all votes --- bigchaindb/core.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 814048cc..eabe7947 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -570,14 +570,20 @@ class Bigchain(object): def get_last_voted_block(self): """Returns the last block that this node voted on.""" + try: + # get the latest value for the vote timestamp (over all votes) + max_timestamp = r.table('votes') \ + .concat_map(lambda x: [x['vote']['timestamp']]) \ + .max() \ + .run(self.conn) - last_voted = r.table('votes') \ - .filter(r.row['node_pubkey'] == self.me) \ - .order_by(r.desc(r.row['vote']['timestamp'])) \ - .run(self.conn) + last_voted = list(r.table('votes') \ + .filter(r.row['node_pubkey'] == self.me) \ + .filter(lambda x: x['vote']['timestamp'] == max_timestamp) \ + .run(self.conn)) - # return last vote if last vote exists else return Genesis block - if not last_voted: + except r.ReqlNonExistenceError: + # return last vote if last vote exists else return Genesis block return list(r.table('bigchain') .filter(util.is_genesis_block) .run(self.conn))[0] From 87d332479b58d8852fe5c155b0e19c0359abdb96 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 18 Jul 2016 10:35:54 +0200 Subject: [PATCH 08/18] filter query --- bigchaindb/core.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index eabe7947..07970693 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -573,6 +573,7 @@ class Bigchain(object): try: # get the latest value for the vote timestamp (over all votes) max_timestamp = r.table('votes') \ + .filter(r.row['node_pubkey'] == self.me) \ .concat_map(lambda x: [x['vote']['timestamp']]) \ .max() \ .run(self.conn) From 3c545f8bda9637b80ed3e880fbc10954cd581a75 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 18 Jul 2016 10:39:58 +0200 Subject: [PATCH 09/18] sort unvoted ascending order --- bigchaindb/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 07970693..bc4d4c2b 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -613,7 +613,7 @@ class Bigchain(object): unvoted = r.table('bigchain') \ .filter(lambda block: r.table('votes').get_all([block['id'], self.me], index='block_and_voter') .is_empty()) \ - .order_by(r.desc(r.row['block']['timestamp'])) \ + .order_by(r.asc(r.row['block']['timestamp'])) \ .run(self.conn) # FIXME: I (@vrde) don't like this solution. Filtering should be done at a From 84945436a358ee2ff1caf68438a149394abb2c93 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 18 Jul 2016 10:54:07 +0200 Subject: [PATCH 10/18] unused import --- tests/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index bbf705a4..7e3f5c56 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,7 +8,6 @@ Tasks: import os import copy -from functools import partial import pytest From 6fa9d5f563caa0305f5b02c4f0aebdda8e5885ac Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 18 Jul 2016 11:02:02 +0200 Subject: [PATCH 11/18] fix fixture --- tests/db/conftest.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/db/conftest.py b/tests/db/conftest.py index 4ae4f0c3..d89ca004 100644 --- a/tests/db/conftest.py +++ b/tests/db/conftest.py @@ -98,23 +98,22 @@ def cleanup_tables(request, node_config): @pytest.fixture -def inputs(user_vk, amount=4, b=None, blocks=4): +def inputs(user_vk): from bigchaindb.exceptions import GenesisBlockAlreadyExistsError # 1. create the genesis block - b = b or Bigchain() + b = Bigchain() try: b.create_genesis_block() except GenesisBlockAlreadyExistsError: pass # 2. create block with transactions for `USER` to spend - for block in range(blocks): + for block in range(4): transactions = [] - for i in range(amount): + for i in range(10): tx = b.create_transaction(b.me, user_vk, None, 'CREATE') tx_signed = b.sign_transaction(tx, b.me_private) transactions.append(tx_signed) - b.write_transaction(tx_signed) block = b.create_block(transactions) b.write_block(block, durability='hard') From 553148c5e49411254ec0ba34c1689a94e7f3f2b7 Mon Sep 17 00:00:00 2001 From: vrde Date: Mon, 18 Jul 2016 13:49:42 +0200 Subject: [PATCH 12/18] Add CyclicBlockchainError exception, fix tests --- bigchaindb/core.py | 24 +++++++++++++----------- bigchaindb/exceptions.py | 4 ++++ tests/db/test_bigchain_api.py | 17 ++++++++++------- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index bc4d4c2b..a4afa583 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -537,6 +537,9 @@ class Bigchain(object): invalid_reason (Optional[str]): Reason the block is invalid """ + if block['id'] == previous_block_id: + raise exceptions.CyclicBlockchainError() + vote = { 'voting_for_block': block['id'], 'previous_block': previous_block_id, @@ -591,17 +594,16 @@ class Bigchain(object): # since the resolution of timestamp is a second, # we might have more than one vote per timestamp - if len(last_voted) > 1: - mapping = {v['vote']['previous_block']: v['vote']['voting_for_block'] - for v in last_voted} - last_block_id = list(mapping.values())[0] - while True: - try: - last_block_id = mapping[last_block_id] - except KeyError: - break - else: - last_block_id = last_voted[0]['vote']['voting_for_block'] + mapping = {v['vote']['previous_block']: v['vote']['voting_for_block'] + for v in last_voted} + last_block_id = list(mapping.values())[0] + while True: + try: + if last_block_id == mapping[last_block_id]: + raise exceptions.CyclicBlockchainError() + last_block_id = mapping[last_block_id] + except KeyError: + break res = r.table('bigchain').get(last_block_id).run(self.conn) diff --git a/bigchaindb/exceptions.py b/bigchaindb/exceptions.py index 9985d722..480a0bd8 100644 --- a/bigchaindb/exceptions.py +++ b/bigchaindb/exceptions.py @@ -56,3 +56,7 @@ class MultipleVotesError(Exception): class GenesisBlockAlreadyExistsError(Exception): """Raised when trying to create the already existing genesis block""" + + +class CyclicBlockchainError(Exception): + """Raised when there is a cycle in the blockchain""" diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index 9c5bcebd..d36902c3 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -315,17 +315,17 @@ class TestBigchainApi(object): assert b.get_last_voted_block()['id'] == block_3['id'] def test_no_vote_written_if_block_already_has_vote(self, b): - b.create_genesis_block() + genesis = b.create_genesis_block() block_1 = dummy_block() b.write_block(block_1, durability='hard') - b.write_vote(block_1, b.vote(block_1, b.get_last_voted_block()['id'], True)) + b.write_vote(block_1, b.vote(block_1, genesis['id'], True)) retrieved_block_1 = r.table('bigchain').get(block_1['id']).run(b.conn) # try to vote again on the retrieved block, should do nothing - b.write_vote(retrieved_block_1, b.vote(retrieved_block_1, b.get_last_voted_block()['id'], True)) + b.write_vote(retrieved_block_1, b.vote(retrieved_block_1, genesis['id'], True)) retrieved_block_2 = r.table('bigchain').get(block_1['id']).run(b.conn) assert retrieved_block_1 == retrieved_block_2 @@ -348,12 +348,12 @@ class TestBigchainApi(object): .format(block_id=block_1['id'], n_votes=str(2), n_voters=str(1)) def test_multiple_votes_single_node(self, b): - b.create_genesis_block() + genesis = b.create_genesis_block() block_1 = dummy_block() b.write_block(block_1, durability='hard') # insert duplicate votes for i in range(2): - r.table('votes').insert(b.vote(block_1, b.get_last_voted_block()['id'], True)).run(b.conn) + r.table('votes').insert(b.vote(block_1, genesis['id'], True)).run(b.conn) from bigchaindb.exceptions import MultipleVotesError with pytest.raises(MultipleVotesError) as excinfo: @@ -1215,6 +1215,7 @@ class TestMultipleInputs(object): assert owned_inputs_user2 == [{'cid': 0, 'txid': tx['id']}] def test_get_owned_ids_single_tx_single_output_invalid_block(self, b, user_sk, user_vk): + genesis = b.create_genesis_block() # create a new users user2_sk, user2_vk = crypto.generate_key_pair() @@ -1225,7 +1226,7 @@ class TestMultipleInputs(object): b.write_block(block, durability='hard') # vote the block VALID - vote = b.vote(block, b.get_unvoted_blocks()[0]['id'], True) + vote = b.vote(block, genesis['id'], True) b.write_vote(block, vote) # get input @@ -1343,6 +1344,8 @@ class TestMultipleInputs(object): assert spent_inputs_user1 == tx_signed def test_get_spent_single_tx_single_output_invalid_block(self, b, user_sk, user_vk): + genesis = b.create_genesis_block() + # create a new users user2_sk, user2_vk = crypto.generate_key_pair() @@ -1353,7 +1356,7 @@ class TestMultipleInputs(object): b.write_block(block, durability='hard') # vote the block VALID - vote = b.vote(block, b.get_unvoted_blocks()[0]['id'], True) + vote = b.vote(block, genesis['id'], True) b.write_vote(block, vote) # get input From 429d299b232e02dc1eca839b72887f01d300120c Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 18 Jul 2016 15:07:10 +0200 Subject: [PATCH 13/18] simplify query --- bigchaindb/core.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index a4afa583..3ee5541d 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -577,9 +577,8 @@ class Bigchain(object): # get the latest value for the vote timestamp (over all votes) max_timestamp = r.table('votes') \ .filter(r.row['node_pubkey'] == self.me) \ - .concat_map(lambda x: [x['vote']['timestamp']]) \ - .max() \ - .run(self.conn) + .max(lambda x: x['vote']['timestamp']) \ + .run(self.conn)['vote']['timestamp'] last_voted = list(r.table('votes') \ .filter(r.row['node_pubkey'] == self.me) \ From 1f7cf3df2e87764429ab9b8e3792e0abc953c511 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 18 Jul 2016 15:09:33 +0200 Subject: [PATCH 14/18] optimize query --- bigchaindb/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 3ee5541d..5cad533b 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -581,8 +581,8 @@ class Bigchain(object): .run(self.conn)['vote']['timestamp'] last_voted = list(r.table('votes') \ - .filter(r.row['node_pubkey'] == self.me) \ .filter(lambda x: x['vote']['timestamp'] == max_timestamp) \ + .filter(r.row['node_pubkey'] == self.me) \ .run(self.conn)) except r.ReqlNonExistenceError: From 9c4314ffe7277977ceb53fe5aa82a35faacd7124 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 18 Jul 2016 15:11:28 +0200 Subject: [PATCH 15/18] uniform notation --- bigchaindb/core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 5cad533b..8fd33e2b 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -577,11 +577,11 @@ class Bigchain(object): # get the latest value for the vote timestamp (over all votes) max_timestamp = r.table('votes') \ .filter(r.row['node_pubkey'] == self.me) \ - .max(lambda x: x['vote']['timestamp']) \ + .max(r.row['vote']['timestamp']) \ .run(self.conn)['vote']['timestamp'] last_voted = list(r.table('votes') \ - .filter(lambda x: x['vote']['timestamp'] == max_timestamp) \ + .filter(r.row['vote']['timestamp'] == max_timestamp) \ .filter(r.row['node_pubkey'] == self.me) \ .run(self.conn)) From 86466725cf3c06e52743ae1a20241c2edeb688aa Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 18 Jul 2016 15:18:31 +0200 Subject: [PATCH 16/18] remove block number reference from docs --- docs/source/topic-guides/models.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/source/topic-guides/models.md b/docs/source/topic-guides/models.md index deda24d5..3ec49981 100644 --- a/docs/source/topic-guides/models.md +++ b/docs/source/topic-guides/models.md @@ -260,7 +260,6 @@ Each node must generate a vote for each block, to be appended the `votes` table. "timestamp": "" }, "signature": "", - "block_number": "" } ``` From fe2245daab3c70556ef7ffc6648153eb7e9a7b34 Mon Sep 17 00:00:00 2001 From: vrde Date: Mon, 18 Jul 2016 16:38:59 +0200 Subject: [PATCH 17/18] Add better detection of loops, more comments --- bigchaindb/core.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 8fd33e2b..5171aab8 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -591,15 +591,32 @@ class Bigchain(object): .filter(util.is_genesis_block) .run(self.conn))[0] - # since the resolution of timestamp is a second, - # we might have more than one vote per timestamp + # Now the fun starts. Since the resolution of timestamp is a second, + # we might have more than one vote per timestamp. If this is the case + # then we need to rebuild the chain for the blocks that have been retrieved + # to get the last one. + + # Given a block_id, mapping returns the id of the block pointing at it. mapping = {v['vote']['previous_block']: v['vote']['voting_for_block'] for v in last_voted} + + # Since we follow the chain backwards, we can start from a random + # point of the chain and "move up" from it. last_block_id = list(mapping.values())[0] + + # We must be sure to break the infinite loop. This happens when: + # - the block we are currenty iterating is the one we are looking for. + # This will trigger a KeyError, breaking the loop + # - we are visiting again a node we already explored, hence there is + # a loop. This might happen if a vote points both `previous_block` + # and `voting_for_block` to the same `block_id` + explored = set() + while True: try: - if last_block_id == mapping[last_block_id]: + if last_block_id in explored: raise exceptions.CyclicBlockchainError() + explored.add(last_block_id) last_block_id = mapping[last_block_id] except KeyError: break From c4c35a25aac9bc2fcb2778a51f5dcdeea54dc579 Mon Sep 17 00:00:00 2001 From: Sylvain Bellemare Date: Tue, 19 Jul 2016 12:07:47 +0200 Subject: [PATCH 18/18] Fix #443 (#445) --- tests/utils/test_config_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/utils/test_config_utils.py b/tests/utils/test_config_utils.py index 89572183..8c8e22d4 100644 --- a/tests/utils/test_config_utils.py +++ b/tests/utils/test_config_utils.py @@ -197,7 +197,7 @@ def test_update_config(monkeypatch): 'database': {'host': 'test-host', 'name': 'bigchaindb', 'port': 28015} } monkeypatch.setattr('bigchaindb.config_utils.file_config', lambda *args, **kwargs: file_config) - config_utils.autoconfigure() + config_utils.autoconfigure(config=file_config) # update configuration, retaining previous changes config_utils.update_config({'database': {'port': 28016, 'name': 'bigchaindb_other'}}) @@ -205,4 +205,3 @@ def test_update_config(monkeypatch): assert bigchaindb.config['database']['host'] == 'test-host' assert bigchaindb.config['database']['name'] == 'bigchaindb_other' assert bigchaindb.config['database']['port'] == 28016 -