Problem: Need logic to allow for only one election that chages the validator set, multiple other elections, per block

Solution:
- created a boolean flag to track which elections update the validator set
- implemented a function to compute the updated validator set
- reworked `approved_electios` to use the new logic
- updated tests

bigchaindb/core.py

@@ -219,12 +219,10 @@ class App(BaseApplication):
         else:
             self.block_txn_hash = block['app_hash']
 
-        # Check the current block to see if any elections have concluded.
+        # Process all concluded elections in the current block and get any update to the validator set
-        concluded_elections = Election.approved_elections(self.bigchaindb,
+        update = Election.approved_elections(self.bigchaindb,
-                                                          self.new_height,
+                                             self.new_height,
-                                                          self.block_transactions)
+                                             self.block_transactions)
-        validator_update = concluded_elections.get('VALIDATOR_ELECTION')
-        update = [validator_update] if validator_update else []
 
         # Store pre-commit state to recover in case there is a crash  during `commit`
         pre_commit_state = PreCommitState(commit_id=PRE_COMMIT_ID,

bigchaindb/elections/election.py

@@ -1,6 +1,7 @@
 # Copyright BigchainDB GmbH and BigchainDB contributors
 # SPDX-License-Identifier: (Apache-2.0 AND CC-BY-4.0)
 # Code is Apache-2.0 and docs are CC-BY-4.0
+from collections import defaultdict
 
 import base58
 from uuid import uuid4
@@ -33,6 +34,7 @@ class Election(Transaction):
     INCONCLUSIVE = 'inconclusive'
     # Vote ratio to approve an election
     ELECTION_THRESHOLD = 2 / 3
+    CHANGES_VALIDATOR_SET = True
 
     @classmethod
     def get_validator_change(cls, bigchain):
@@ -244,31 +246,40 @@ class Election(Transaction):
 
     @classmethod
     def approved_elections(cls, bigchain, new_height, txns):
-        elections = {}
+        elections = defaultdict(list)
-        for txn in txns:
+        for tx in txns:
-            if not isinstance(txn, Vote):
+            if not isinstance(tx, Vote):
+                continue
+            election_id = tx.asset['id']
+            elections[election_id].append(tx)
+
+        validator_set_updated = False
+        validator_set_change = []
+        for election_id, votes in elections.items():
+            election = bigchain.get_transaction(election_id)
+
+            if not election.has_concluded(bigchain, election_id, votes, new_height):
                 continue
 
-            election_id = txn.asset['id']
+            if election.makes_validator_set_change():
-            e = bigchain.get_transaction(election_id)
+                if validator_set_updated:
-            election_operation = e.OPERATION
+                    continue
-            election_operation_votes = elections.get(election_operation, {})
+                validator_set_change.append(election.get_validator_set_change(bigchain, new_height))
-            # Once we conclude an election of a given type, we stop looking at that election class
+                validator_set_updated = True
-            if type(election_operation_votes) != dict:
-                continue
-            election_votes = election_operation_votes.get(election_id, [])
-            election_votes.append(txn)
-            election_operation_votes[election_id] = election_votes
-            elections[election_operation] = election_operation_votes
 
-            election = cls.has_concluded(bigchain, election_id, election_votes, new_height)
+            election.on_approval(bigchain, election, new_height)
-            if election:
+
-                # Once we conclude an election, we store the result in the db
+        return validator_set_change
-                cls.store_election_results(bigchain, election, new_height)
+
-                # And keep the transaction filed under the election_type
+    def makes_validator_set_change(self):
-                elections[election_operation] = election.on_approval(bigchain, election, new_height)
+        return self.CHANGES_VALIDATOR_SET
-        approved_elections = {k: v for (k, v) in elections.items() if type(v) != dict}
+
-        return approved_elections
+    def get_validator_set_change(self, bigchain, new_height):
+        if self.makes_validator_set_change():
+            return self.change_validator_set(bigchain, new_height)
+
+    def change_validator_set(self, bigchain, new_height):
+        raise NotImplementedError
 
     @classmethod
     def on_approval(cls, bigchain, election, new_height):

bigchaindb/migrations/migration_election.py

@@ -8,6 +8,7 @@ class MigrationElection(Election):
     CREATE = OPERATION
     ALLOWED_OPERATIONS = (OPERATION,)
     TX_SCHEMA_CUSTOM = TX_SCHEMA_MIGRATION_ELECTION
+    CHANGES_VALIDATOR_SET = False
 
     @classmethod
     def on_approval(cls, bigchain, election, new_height):

bigchaindb/upsert_validator/validator_election.py

@@ -36,14 +36,18 @@ class ValidatorElection(Election):
         super(ValidatorElection, cls).validate_schema(tx)
         validate_asset_public_key(tx['asset']['data']['public_key'])
 
-    @classmethod
+    def change_validator_set(self, bigchain, new_height):
-    def on_approval(cls, bigchain, election, new_height):
         # The new validator set comes into effect from height = new_height+1
-        validator_updates = [election.asset['data']]
+        # (upcoming changes to Tendermint will change this to height = new_height+2)
+        validator_updates = [self.asset['data']]
         curr_validator_set = bigchain.get_validators(new_height)
         updated_validator_set = new_validator_set(curr_validator_set,
                                                   validator_updates)
 
         updated_validator_set = [v for v in updated_validator_set if v['voting_power'] > 0]
         bigchain.store_validator_set(new_height+1, updated_validator_set)
-        return encode_validator(election.asset['data'])
+        return encode_validator(self.asset['data'])
+
+    @classmethod
+    def on_approval(cls, bigchain, election, new_height):
+        pass

tests/elections/test_election.py

@@ -17,15 +17,13 @@ def test_approved_elections_one_migration_one_upsert(b,
 
 
 @pytest.mark.bdb
-def test_approved_elections_two_migrations_two_upsert(b,
+def test_approved_elections_one_migration_two_upsert(b,
-                                                      ongoing_validator_election, validator_election_votes,
+                                                     ongoing_validator_election, validator_election_votes,
-                                                      ongoing_validator_election_2, validator_election_votes_2,
+                                                     ongoing_validator_election_2, validator_election_votes_2,
-                                                      ongoing_migration_election, migration_election_votes,
+                                                     ongoing_migration_election, migration_election_votes):
-                                                      ongoing_migration_election_2, migration_election_votes_2):
     txns = validator_election_votes + \
            validator_election_votes_2 + \
-           migration_election_votes + \
+           migration_election_votes
-           migration_election_votes_2
     mock_chain_migration, mock_store_validator = run_approved_elections(b, txns)
     mock_chain_migration.assert_called_once()
     mock_store_validator.assert_called_once()
@@ -40,7 +38,7 @@ def test_approved_elections_two_migrations_one_upsert(b,
            migration_election_votes + \
            migration_election_votes_2
     mock_chain_migration, mock_store_validator = run_approved_elections(b, txns)
-    mock_chain_migration.assert_called_once()
+    assert mock_chain_migration.call_count == 2
     mock_store_validator.assert_called_once()
 
 

tests/upsert_validator/test_upsert_validator_vote.py

@@ -294,19 +294,19 @@ def test_get_validator_update(b, node_keys, node_key, ed25519_node_keys):
     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.approved_elections(b, 4, [tx_vote0]).get('VALIDATOR_ELECTION')
+    assert Election.approved_elections(b, 4, [tx_vote0]) == []
-    assert not Election.approved_elections(b, 4, [tx_vote0, tx_vote1]).get('VALIDATOR_ELECTION')
+    assert Election.approved_elections(b, 4, [tx_vote0, tx_vote1]) == []
 
-    update = Election.approved_elections(b, 4, [tx_vote0, tx_vote1, tx_vote2]).get('VALIDATOR_ELECTION')
+    update = Election.approved_elections(b, 4, [tx_vote0, tx_vote1, tx_vote2])
-    assert update
+    assert len(update) == 1
-    update_public_key = codecs.encode(update.pub_key.data, 'base64').decode().rstrip('\n')
+    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]).get('VALIDATOR_ELECTION')
+    update = Election.approved_elections(b, 4, [tx_vote2])
-    assert update
+    assert len(update) == 1
-    update_public_key = codecs.encode(update.pub_key.data, 'base64').decode().rstrip('\n')
+    update_public_key = codecs.encode(update[0].pub_key.data, 'base64').decode().rstrip('\n')
     assert update_public_key == public_key64
 
     # remove validator
@@ -327,10 +327,9 @@ def test_get_validator_update(b, node_keys, node_key, ed25519_node_keys):
 
     b.store_bulk_transactions([tx_vote0, tx_vote1])
 
-    update = Election.approved_elections(b, 9, [tx_vote2]).get('VALIDATOR_ELECTION')
+    update = Election.approved_elections(b, 9, [tx_vote2])
-    if update:
+    assert len(update) == 1
-        update_public_key = codecs.encode(update.pub_key.data, 'base64').decode().rstrip('\n')
+    update_public_key = codecs.encode(update[0].pub_key.data, 'base64').decode().rstrip('\n')
-    assert update
     assert update_public_key == public_key64
 
     # assert that the public key is not a part of the current validator set

tests/upsert_validator/test_validator_election.py

@@ -124,6 +124,9 @@ def test_get_status_concluded(b, concluded_election, new_validator):
 
 
 def test_get_status_inconclusive(b, inconclusive_election, new_validator):
+    def set_block_height_to_3():
+        return {'height': 3}
+
     def custom_mock_get_validators(height):
         if height >= 3:
             return [{'pub_key': {'data': 'zL/DasvKulXZzhSNFwx4cLRXKkSM9GPK7Y0nZ4FEylM=',
@@ -153,6 +156,7 @@ def test_get_status_inconclusive(b, inconclusive_election, new_validator):
                      'voting_power': 8}]
 
     b.get_validators = custom_mock_get_validators
+    b.get_latest_block = set_block_height_to_3
     status = ValidatorElection.INCONCLUSIVE
     resp = inconclusive_election.get_status(b)
     assert resp == status