From 113065b9f6b8d6e417233c4ebae6b5a5c84c5220 Mon Sep 17 00:00:00 2001 From: Lev Berman Date: Tue, 18 Sep 2018 11:18:59 +0200 Subject: [PATCH] Adjust multiple elections conclusion. - Do not conclude migration election if there is a migration in progress. - Rewrite election tests to not use mocks and assert many different things. - Record concluded elections in the `election` collection. --- bigchaindb/backend/localmongodb/query.py | 5 +- bigchaindb/elections/election.py | 42 ++-- .../migrations/chain_migration_election.py | 9 + tests/conftest.py | 51 +---- tests/elections/test_election.py | 179 +++++++++++++----- .../test_upsert_validator_vote.py | 33 ++-- tests/utils.py | 64 ++++++- 7 files changed, 236 insertions(+), 147 deletions(-) diff --git a/bigchaindb/backend/localmongodb/query.py b/bigchaindb/backend/localmongodb/query.py index 75fa7aa5..6b0793ba 100644 --- a/bigchaindb/backend/localmongodb/query.py +++ b/bigchaindb/backend/localmongodb/query.py @@ -283,12 +283,11 @@ def store_validator_set(conn, validators_update): @register_query(LocalMongoDBConnection) def store_election_results(conn, election): - height = election['height'] return conn.run( conn.collection('elections').replace_one( - {'height': height}, + {'election_id': election['election_id']}, election, - upsert=True + upsert=True, ) ) diff --git a/bigchaindb/elections/election.py b/bigchaindb/elections/election.py index ca0a4be3..40861a52 100644 --- a/bigchaindb/elections/election.py +++ b/bigchaindb/elections/election.py @@ -186,27 +186,26 @@ class Election(Transaction): election_pk)) return self.count_votes(election_pk, txns, dict.get) - @classmethod - def has_concluded(cls, bigchain, election_id, current_votes=[], height=None): - """Check if the given `election_id` can be concluded or not - NOTE: - * Election is concluded iff the current validator set is exactly equal - to the validator set encoded in election outputs - * Election can concluded only if the current votes achieves a supermajority + def has_concluded(self, bigchain, current_votes=[], height=None): + """Check if the election can be concluded or not. + + * Elections can only be concluded if the current validator set + is exactly equal to the validator set encoded in the election outputs. + * Elections can be concluded only if the current votes form a supermajority. + + Custom elections may override this function and introduce additional checks. """ - election = bigchain.get_transaction(election_id) - if election: - election_pk = election.to_public_key(election.id) - votes_committed = election.get_commited_votes(bigchain, election_pk) - votes_current = election.count_votes(election_pk, current_votes) - current_validators = election.get_validators(bigchain, height) + election_pk = self.to_public_key(self.id) + votes_committed = self.get_commited_votes(bigchain, election_pk) + votes_current = self.count_votes(election_pk, current_votes) + current_validators = self.get_validators(bigchain, height) - if election.is_same_topology(current_validators, election.outputs): - total_votes = sum(current_validators.values()) - if (votes_committed < (2/3)*total_votes) and \ - (votes_committed + votes_current >= (2/3)*total_votes): - return election + if self.is_same_topology(current_validators, self.outputs): + total_votes = sum(current_validators.values()) + if (votes_committed < (2/3) * total_votes) and \ + (votes_committed + votes_current >= (2/3)*total_votes): + return True return False def get_status(self, bigchain): @@ -255,9 +254,11 @@ class Election(Transaction): validator_set_updated = False validator_set_change = [] for election_id, votes in elections.items(): - election = Election.has_concluded(bigchain, election_id, votes, new_height) + election = bigchain.get_transaction(election_id) + if election is None: + continue - if not election: + if not election.has_concluded(bigchain, votes, new_height): continue if election.makes_validator_set_change(): @@ -267,6 +268,7 @@ class Election(Transaction): validator_set_updated = True election.on_approval(bigchain, election, new_height) + election.store_election_results(bigchain, election, new_height) return validator_set_change diff --git a/bigchaindb/migrations/chain_migration_election.py b/bigchaindb/migrations/chain_migration_election.py index e1e783d0..8e80b979 100644 --- a/bigchaindb/migrations/chain_migration_election.py +++ b/bigchaindb/migrations/chain_migration_election.py @@ -10,6 +10,15 @@ class ChainMigrationElection(Election): TX_SCHEMA_CUSTOM = TX_SCHEMA_CHAIN_MIGRATION_ELECTION CHANGES_VALIDATOR_SET = False + def has_concluded(self, bigchaindb, *args, **kwargs): + chain = bigchaindb.get_latest_abci_chain() + if chain is not None and not chain['is_synced']: + # do not conclude the migration election if + # there is another migration in progress + return False + + return super().has_concluded(bigchaindb, *args, **kwargs) + @classmethod def on_approval(cls, bigchain, election, new_height): bigchain.migrate_abci_chain() diff --git a/tests/conftest.py b/tests/conftest.py index 4c271aad..90d54dea 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -23,7 +23,6 @@ from pymongo import MongoClient from bigchaindb import ValidatorElection from bigchaindb.common import crypto from bigchaindb.log import setup_logging -from bigchaindb.migrations.chain_migration_election import ChainMigrationElection from bigchaindb.tendermint_utils import key_from_base64 from bigchaindb.backend import schema, query from bigchaindb.common.crypto import (key_pair_from_ed25519_key, @@ -714,22 +713,6 @@ def valid_upsert_validator_election_2(b_mock, node_key, new_validator): new_validator, None).sign([node_key.private_key]) -@pytest.fixture -def valid_chain_migration_election(b_mock, node_key): - voters = ChainMigrationElection.recipients(b_mock) - return ChainMigrationElection.generate([node_key.public_key], - voters, - {}, None).sign([node_key.private_key]) - - -@pytest.fixture -def valid_chain_migration_election_2(b_mock, node_key): - voters = ChainMigrationElection.recipients(b_mock) - return ChainMigrationElection.generate([node_key.public_key], - voters, - {}, None).sign([node_key.private_key]) - - @pytest.fixture def ongoing_validator_election(b, valid_upsert_validator_election, ed25519_node_keys): validators = b.get_validators(height=1) @@ -758,24 +741,6 @@ def ongoing_validator_election_2(b, valid_upsert_validator_election_2, ed25519_n return valid_upsert_validator_election_2 -@pytest.fixture -def ongoing_chain_migration_election(b, valid_chain_migration_election, ed25519_node_keys): - - b.store_bulk_transactions([valid_chain_migration_election]) - block_1 = Block(app_hash='hash_1', height=1, transactions=[valid_chain_migration_election.id]) - b.store_block(block_1._asdict()) - return valid_chain_migration_election - - -@pytest.fixture -def ongoing_chain_migration_election_2(b, valid_chain_migration_election_2, ed25519_node_keys): - - b.store_bulk_transactions([valid_chain_migration_election_2]) - block_1 = Block(app_hash='hash_2', height=1, transactions=[valid_chain_migration_election_2.id]) - b.store_block(block_1._asdict()) - return valid_chain_migration_election_2 - - @pytest.fixture def validator_election_votes(b_mock, ongoing_validator_election, ed25519_node_keys): voters = ValidatorElection.recipients(b_mock) @@ -790,23 +755,9 @@ def validator_election_votes_2(b_mock, ongoing_validator_election_2, ed25519_nod return votes -@pytest.fixture -def chain_migration_election_votes(b_mock, ongoing_chain_migration_election, ed25519_node_keys): - voters = ChainMigrationElection.recipients(b_mock) - votes = generate_votes(ongoing_chain_migration_election, voters, ed25519_node_keys) - return votes - - -@pytest.fixture -def chain_migration_election_votes_2(b_mock, ongoing_chain_migration_election_2, ed25519_node_keys): - voters = ChainMigrationElection.recipients(b_mock) - votes = generate_votes(ongoing_chain_migration_election_2, voters, ed25519_node_keys) - return votes - - def generate_votes(election, voters, keys): votes = [] - for voter in range(len(voters)): + for voter, _ in enumerate(voters): v = gen_vote(election, voter, keys) votes.append(v) return votes diff --git a/tests/elections/test_election.py b/tests/elections/test_election.py index 0e9da188..0aed1816 100644 --- a/tests/elections/test_election.py +++ b/tests/elections/test_election.py @@ -1,64 +1,143 @@ -from unittest.mock import MagicMock - import pytest +from tests.utils import generate_election, generate_validators + +from bigchaindb.lib import Block from bigchaindb.elections.election import Election +from bigchaindb.migrations.chain_migration_election import ChainMigrationElection +from bigchaindb.upsert_validator.validator_election import ValidatorElection @pytest.mark.bdb -def test_approved_elections_one_migration_one_upsert( - b, - ongoing_validator_election, validator_election_votes, - ongoing_chain_migration_election, chain_migration_election_votes -): - txns = validator_election_votes + \ - chain_migration_election_votes - mock_chain_migration, mock_store_validator = run_approved_elections(b, txns) - mock_chain_migration.assert_called_once() - mock_store_validator.assert_called_once() +def test_approved_elections_concludes_all_elections(b): + validators = generate_validators([1] * 4) + b.store_validator_set(1, [v['storage'] for v in validators]) + + new_validator = generate_validators([1])[0] + + public_key = validators[0]['public_key'] + private_key = validators[0]['private_key'] + election, votes = generate_election(b, + ValidatorElection, + public_key, private_key, + new_validator['election']) + txs = [election] + total_votes = votes + + election, votes = generate_election(b, + ChainMigrationElection, + public_key, private_key, + {}) + + txs += [election] + total_votes += votes + + b.store_abci_chain(1, 'chain-X') + b.store_block(Block(height=1, + transactions=[tx.id for tx in txs], + app_hash='')._asdict()) + b.store_bulk_transactions(txs) + + Election.approved_elections(b, 1, total_votes) + + validators = b.get_validators() + assert len(validators) == 5 + assert new_validator['storage'] in validators + + chain = b.get_latest_abci_chain() + assert chain + assert chain == { + 'height': 2, + 'is_synced': False, + 'chain_id': 'chain-X-migrated-at-height-1', + } + + for tx in txs: + election = b.get_election(tx.id) + assert election @pytest.mark.bdb -def test_approved_elections_one_migration_two_upsert( - b, - ongoing_validator_election, validator_election_votes, - ongoing_validator_election_2, validator_election_votes_2, - ongoing_chain_migration_election, chain_migration_election_votes -): - txns = validator_election_votes + \ - validator_election_votes_2 + \ - chain_migration_election_votes - mock_chain_migration, mock_store_validator = run_approved_elections(b, txns) - mock_chain_migration.assert_called_once() - mock_store_validator.assert_called_once() +def test_approved_elections_applies_only_one_validator_update(b): + validators = generate_validators([1] * 4) + b.store_validator_set(1, [v['storage'] for v in validators]) + + new_validator = generate_validators([1])[0] + + public_key = validators[0]['public_key'] + private_key = validators[0]['private_key'] + election, votes = generate_election(b, + ValidatorElection, + public_key, private_key, + new_validator['election']) + txs = [election] + total_votes = votes + + another_validator = generate_validators([1])[0] + + election, votes = generate_election(b, + ValidatorElection, + public_key, private_key, + another_validator['election']) + txs += [election] + total_votes += votes + + b.store_block(Block(height=1, + transactions=[tx.id for tx in txs], + app_hash='')._asdict()) + b.store_bulk_transactions(txs) + + Election.approved_elections(b, 1, total_votes) + + validators = b.get_validators() + assert len(validators) == 5 + assert new_validator['storage'] in validators + assert another_validator['storage'] not in validators + + assert b.get_election(txs[0].id) + assert not b.get_election(txs[1].id) @pytest.mark.bdb -def test_approved_elections_two_migrations_one_upsert( - b, - ongoing_validator_election, validator_election_votes, - ongoing_chain_migration_election, chain_migration_election_votes, - ongoing_chain_migration_election_2, chain_migration_election_votes_2 -): - txns = validator_election_votes + \ - chain_migration_election_votes + \ - chain_migration_election_votes_2 - mock_chain_migration, mock_store_validator = run_approved_elections(b, txns) - assert mock_chain_migration.call_count == 2 - mock_store_validator.assert_called_once() +def test_approved_elections_applies_only_one_migration(b): + validators = generate_validators([1] * 4) + b.store_validator_set(1, [v['storage'] for v in validators]) + + public_key = validators[0]['public_key'] + private_key = validators[0]['private_key'] + election, votes = generate_election(b, + ChainMigrationElection, + public_key, private_key, + {}) + txs = [election] + total_votes = votes + + election, votes = generate_election(b, + ChainMigrationElection, + public_key, private_key, + {}) + + txs += [election] + total_votes += votes + + b.store_abci_chain(1, 'chain-X') + b.store_block(Block(height=1, + transactions=[tx.id for tx in txs], + app_hash='')._asdict()) + b.store_bulk_transactions(txs) + + Election.approved_elections(b, 1, total_votes) + chain = b.get_latest_abci_chain() + assert chain + assert chain == { + 'height': 2, + 'is_synced': False, + 'chain_id': 'chain-X-migrated-at-height-1', + } + + assert b.get_election(txs[0].id) + assert not b.get_election(txs[1].id) -def test_approved_elections_no_elections(b): - txns = [] - mock_chain_migration, mock_store_validator = run_approved_elections(b, txns) - mock_chain_migration.assert_not_called() - mock_store_validator.assert_not_called() - - -def run_approved_elections(bigchain, txns): - mock_chain_migration = MagicMock() - mock_store_validator = MagicMock() - bigchain.migrate_abci_chain = mock_chain_migration - bigchain.store_validator_set = mock_store_validator - Election.approved_elections(bigchain, 1, txns) - return mock_chain_migration, mock_store_validator +def test_approved_elections_gracefully_handles_empty_block(b): + Election.approved_elections(b, 1, []) diff --git a/tests/upsert_validator/test_upsert_validator_vote.py b/tests/upsert_validator/test_upsert_validator_vote.py index 3c3512d0..7c49bd77 100644 --- a/tests/upsert_validator/test_upsert_validator_vote.py +++ b/tests/upsert_validator/test_upsert_validator_vote.py @@ -168,14 +168,14 @@ def test_valid_election_conclude(b_mock, valid_upsert_validator_election, ed2551 # store election b_mock.store_bulk_transactions([valid_upsert_validator_election]) # cannot conclude election as not votes exist - assert not ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id) + assert not valid_upsert_validator_election.has_concluded(b_mock) # validate vote assert tx_vote0.validate(b_mock) - assert not ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id, [tx_vote0]) + assert not valid_upsert_validator_election.has_concluded(b_mock, [tx_vote0]) b_mock.store_bulk_transactions([tx_vote0]) - assert not ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id) + assert not valid_upsert_validator_election.has_concluded(b_mock) # Node 1: cast vote tx_vote1 = gen_vote(valid_upsert_validator_election, 1, ed25519_node_keys) @@ -187,31 +187,31 @@ def test_valid_election_conclude(b_mock, valid_upsert_validator_election, ed2551 tx_vote3 = gen_vote(valid_upsert_validator_election, 3, ed25519_node_keys) assert tx_vote1.validate(b_mock) - assert not ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id, [tx_vote1]) + assert not valid_upsert_validator_election.has_concluded(b_mock, [tx_vote1]) # 2/3 is achieved in the same block so the election can be.has_concludedd - assert ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id, [tx_vote1, tx_vote2]) + assert valid_upsert_validator_election.has_concluded(b_mock, [tx_vote1, tx_vote2]) b_mock.store_bulk_transactions([tx_vote1]) - assert not ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id) + assert not valid_upsert_validator_election.has_concluded(b_mock) assert tx_vote2.validate(b_mock) assert tx_vote3.validate(b_mock) # conclusion can be triggered my different votes in the same block - assert ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id, [tx_vote2]) - assert ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id, [tx_vote2, tx_vote3]) + assert valid_upsert_validator_election.has_concluded(b_mock, [tx_vote2]) + assert valid_upsert_validator_election.has_concluded(b_mock, [tx_vote2, tx_vote3]) b_mock.store_bulk_transactions([tx_vote2]) # Once the blockchain records >2/3 of the votes the election is assumed to be.has_concludedd # so any invocation of `.has_concluded` for that election should return False - assert not ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id) + assert not valid_upsert_validator_election.has_concluded(b_mock) # Vote is still valid but the election cannot be.has_concludedd as it it assmed that it has # been.has_concludedd before assert tx_vote3.validate(b_mock) - assert not ValidatorElection.has_concluded(b_mock, valid_upsert_validator_election.id, [tx_vote3]) + assert not valid_upsert_validator_election.has_concluded(b_mock, [tx_vote3]) @pytest.mark.abci @@ -285,9 +285,9 @@ def test_get_validator_update(b, node_keys, node_key, ed25519_node_keys): tx_vote1 = gen_vote(election, 1, ed25519_node_keys) tx_vote2 = gen_vote(election, 2, ed25519_node_keys) - assert not ValidatorElection.has_concluded(b, election.id, [tx_vote0]) - assert not ValidatorElection.has_concluded(b, election.id, [tx_vote0, tx_vote1]) - assert ValidatorElection.has_concluded(b, election.id, [tx_vote0, tx_vote1, tx_vote2]) + assert not election.has_concluded(b, [tx_vote0]) + assert not election.has_concluded(b, [tx_vote0, tx_vote1]) + assert election.has_concluded(b, [tx_vote0, tx_vote1, tx_vote2]) assert Election.approved_elections(b, 4, [tx_vote0]) == [] assert Election.approved_elections(b, 4, [tx_vote0, tx_vote1]) == [] @@ -297,13 +297,6 @@ def test_get_validator_update(b, node_keys, node_key, ed25519_node_keys): update_public_key = codecs.encode(update[0].pub_key.data, 'base64').decode().rstrip('\n') assert update_public_key == public_key64 - b.store_bulk_transactions([tx_vote0, tx_vote1]) - - update = Election.approved_elections(b, 4, [tx_vote2]) - assert len(update) == 1 - update_public_key = codecs.encode(update[0].pub_key.data, 'base64').decode().rstrip('\n') - assert update_public_key == public_key64 - # remove validator power = 0 new_validator = {'public_key': {'value': public_key, 'type': 'ed25519-base16'}, diff --git a/tests/utils.py b/tests/utils.py index 852be5e6..5b2e1ea6 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -2,12 +2,17 @@ # SPDX-License-Identifier: (Apache-2.0 AND CC-BY-4.0) # Code is Apache-2.0 and docs are CC-BY-4.0 +import base58 +import base64 +import random + from functools import singledispatch -from bigchaindb import Vote from bigchaindb.backend.localmongodb.connection import LocalMongoDBConnection from bigchaindb.backend.schema import TABLES -from bigchaindb.elections.election import Election +from bigchaindb.common import crypto +from bigchaindb.elections.election import Election, Vote +from bigchaindb.tendermint_utils import key_to_base64 @singledispatch @@ -24,7 +29,6 @@ def flush_localmongo_db(connection, dbname): def generate_block(bigchain): from bigchaindb.common.crypto import generate_key_pair from bigchaindb.models import Transaction - import time alice = generate_key_pair() tx = Transaction.create([alice.public_key], @@ -34,7 +38,6 @@ def generate_block(bigchain): code, message = bigchain.write_transaction(tx, 'broadcast_tx_commit') assert code == 202 - time.sleep(2) def to_inputs(election, i, ed25519_node_keys): @@ -52,3 +55,56 @@ def gen_vote(election, i, ed25519_node_keys): [([election_pub_key], votes_i)], election_id=election.id)\ .sign([key_i.private_key]) + + +def generate_validators(powers): + """Generates an arbitrary number of validators with random public keys. + + The object under the `storage` key is in the format expected by DB. + + The object under the `eleciton` key is in the format expected by + the upsert validator election. + + `public_key`, `private_key` are in the format used for signing transactions. + + Args: + powers: A list of intergers representing the voting power to + assign to the corresponding validators. + """ + validators = [] + for power in powers: + kp = crypto.generate_key_pair() + validators.append({ + 'storage': { + 'public_key': { + 'value': key_to_base64(base58.b58decode(kp.public_key).hex()), + 'type': 'ed25519-base64', + }, + 'voting_power': power, + }, + 'election': { + 'node_id': f'node-{random.choice(range(100))}', + 'power': power, + 'public_key': { + 'value': base64.b16encode(base58.b58decode(kp.public_key)).decode('utf-8'), + 'type': 'ed25519-base16', + }, + }, + 'public_key': kp.public_key, + 'private_key': kp.private_key, + }) + return validators + + +def generate_election(b, cls, public_key, private_key, asset_data): + voters = cls.recipients(b) + election = cls.generate([public_key], + voters, + asset_data, + None).sign([private_key]) + + votes = [Vote.generate([election.to_inputs()[i]], + [([Election.to_public_key(election.id)], power)], + election.id) for i, (_, power) in enumerate(voters)] + + return election, votes