Adjust multiple elections conclusion. (#2553)

- 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

@@ -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,
         )
     )
 

bigchaindb/elections/election.py

@@ -186,27 +186,26 @@ def get_commited_votes(self, bigchain, election_pk=None):
                                                                   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)
-
-            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
+
+        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 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 @@ def approved_elections(cls, bigchain, new_height, txns):
         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 @@ def approved_elections(cls, bigchain, new_height, txns):
                 validator_set_updated = True
 
             election.on_approval(bigchain, election, new_height)
+            election.store_election_results(bigchain, election, new_height)
 
         return validator_set_change
 

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()

tests/conftest.py

@@ -23,7 +23,6 @@
 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

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_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_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_gracefully_handles_empty_block(b):
+    Election.approved_elections(b, 1, [])