Merge pull request #205 from thenewboston-developers/BC-272-add-block-to-blockchain-once-it-gets-enough-confirmations

Fix for later confirmations

Author: dmugtasimov

node/blockchain/tests/base.py

@@ -6,10 +6,8 @@
 from node.core.utils.cryptography import get_node_identifier
 
 
-@contextmanager
-def as_role(node_role: NodeRole):
+def make_node_as_role(node_identifier, node_role):
     next_block_number = BlockchainFacade.get_instance().get_next_block_number()
-    node_identifier = get_node_identifier()
     if node_role == NodeRole.PRIMARY_VALIDATOR:
         Schedule.objects.update_or_create(_id=next_block_number, defaults={'node_identifier': node_identifier})
     elif node_role == NodeRole.CONFIRMATION_VALIDATOR:
@@ -20,4 +18,8 @@ def as_role(node_role: NodeRole):
         assert node_role == NodeRole.REGULAR_NODE
         Schedule.objects.filter(node_identifier=node_identifier).delete()
 
+
+@contextmanager
+def as_role(node_role: NodeRole):
+    make_node_as_role(get_node_identifier(), node_role)
     yield

node/blockchain/tests/fixtures/account.py

@@ -36,6 +36,14 @@ def confirmation_validator_key_pair_2() -> KeyPair:
     )
 
 
+@pytest.fixture
+def confirmation_validator_key_pair_3() -> KeyPair:
+    return KeyPair(
+        public=AccountNumber('5db9a262236cc148fd2adf841dbe0967e9bfe77e5e482dc7e0ef0c59d7fb56cf'),
+        private=SigningKey('67eb1838664e12d0773988618a1c69d90757dae5b9ea4e7c7e543b118f31aa55'),
+    )
+
+
 @pytest.fixture
 def regular_node_key_pair() -> KeyPair:
     return KeyPair(

node/blockchain/tests/fixtures/node.py

@@ -37,6 +37,14 @@ def confirmation_validator_node_2(confirmation_validator_key_pair_2):
     )
 
 
+@pytest.fixture
+def confirmation_validator_node_3(confirmation_validator_key_pair_3):
+    return make_node(
+        confirmation_validator_key_pair_3,
+        ['http://not-existing-confirmation-validator-3-address-674898923.com:8555/']
+    )
+
+
 @pytest.fixture
 def self_node(self_node_key_pair, test_server_address):
     return make_node(self_node_key_pair, ['http://not-existing-self-address-674898923.com:8555/', test_server_address])

node/blockchain/tests/test_rest_api/test_block_confirmation.py

@@ -9,12 +9,11 @@
 
 
 @pytest.mark.usefixtures('rich_blockchain', 'as_confirmation_validator')
-def test_send_confirmation_to_cv(confirmation_validator_key_pair_2, api_client):
+def test_send_confirmation_to_cv(next_block, confirmation_validator_key_pair_2, api_client):
     assert not ORMBlockConfirmation.objects.exists()
 
-    facade = BlockchainFacade.get_instance()
-    assert facade.get_next_block_number() >= 4
-    block = facade.get_block_by_number(4)
+    block = next_block
+    assert BlockchainFacade.get_instance().get_next_block_number() >= block.get_block_number()
 
     hash_ = block.make_hash()
     block_confirmation = BlockConfirmation.create(

node/blockchain/tests/test_tasks/test_process_block_confirmations.py

@@ -10,6 +10,7 @@
     get_consensus_block_hash_with_confirmations, get_next_block_confirmations, is_valid_consensus,
     process_block_confirmations_task, process_next_block
 )
+from node.blockchain.tests.base import make_node_as_role
 from node.blockchain.types import NodeRole
 
 
@@ -117,8 +118,11 @@ def test_process_block_confirmations_task():
 
 @pytest.mark.usefixtures('rich_blockchain', 'as_confirmation_validator')
 def test_process_block_confirmations_integration(
-    next_block, self_node_key_pair, confirmation_validator_key_pair, confirmation_validator_key_pair_2, api_client
+    next_block, self_node_key_pair, confirmation_validator_key_pair, confirmation_validator_key_pair_2,
+    confirmation_validator_key_pair_3, api_client
 ):
+    make_node_as_role(confirmation_validator_key_pair_3.public, NodeRole.CONFIRMATION_VALIDATOR)
+
     facade = BlockchainFacade.get_instance()
     block_number = facade.get_next_block_number()
 
@@ -137,17 +141,34 @@ def test_process_block_confirmations_integration(
     assert pending_block
     assert pending_block.body == payload
 
+    assert not BlockConfirmation.objects.all().exists()
     # TODO(dmu) CRITICAL: Remove artificial own confirmation
     #                     https://thenewboston.atlassian.net/browse/BC-263
     confirmation = PydanticBlockConfirmation.create_from_block(block, self_node_key_pair.private)
     BlockConfirmation.objects.create_from_block_confirmation(confirmation)
+    assert BlockConfirmation.objects.all().exists()
 
     # Create confirmations from other CVs
-    for private_key in (confirmation_validator_key_pair.private, confirmation_validator_key_pair_2.private):
+    counter = 1
+    cv_keys = (
+        confirmation_validator_key_pair.private, confirmation_validator_key_pair_2.private,
+        confirmation_validator_key_pair_3.private
+    )
+    for private_key in cv_keys:
         confirmation = PydanticBlockConfirmation.create_from_block(block, private_key)
         payload = confirmation.json()
         response = api_client.post('/api/block-confirmations/', payload, content_type='application/json')
-        assert response.status_code == 201
+
+        counter += 1
+        if counter < 3:
+            assert response.status_code == 201
+            assert BlockConfirmation.objects.all().exists()
+        elif counter == 3:
+            assert response.status_code == 201
+            assert not BlockConfirmation.objects.all().exists()  # because of always eager
+        else:
+            assert response.status_code == 204
+            assert not BlockConfirmation.objects.all().exists()
 
     # Assert that block was added to the blockchain
     assert facade.get_next_block_number() == block_number + 1

node/blockchain/views/block_confirmation.py

@@ -19,15 +19,18 @@ def create(self, request, *args, **kwargs):
 
         facade = BlockchainFacade.get_instance()
         next_block_number = facade.get_next_block_number()
-        is_next_block_number = block_confirmation.get_number() == next_block_number
-        if is_next_block_number:
+        block_confirmation_number = block_confirmation.get_number()
+        if block_confirmation_number < next_block_number:
+            return Response(status=status.HTTP_204_NO_CONTENT)  # Block is already confirmed
+
+        if block_confirmation_number == next_block_number:
             block_confirmation.validate_all(facade)
         else:
             block_confirmation.validate_business_logic()
 
         ORMBlockConfirmation.objects.update_or_create_from_block_confirmation(block_confirmation)
 
-        if is_next_block_number and (
+        if block_confirmation_number == next_block_number and (
             ORMBlockConfirmation.objects.filter(number=next_block_number).count() >= facade.get_minimum_consensus()
         ):
             apply_on_commit(start_process_block_confirmations_task)

node/core/tests/test_node_client.py

@@ -116,12 +116,13 @@ def test_send_block_to_address_integration(
 
 
 @pytest.mark.usefixtures('rich_blockchain', 'as_confirmation_validator')
-def test_send_confirmation_to_cv(test_server_address, confirmation_validator_key_pair_2, smart_mocked_node_client):
+def test_send_confirmation_to_cv(
+    next_block, test_server_address, confirmation_validator_key_pair_2, smart_mocked_node_client
+):
     assert not ORMBlockConfirmation.objects.exists()
 
-    facade = BlockchainFacade.get_instance()
-    assert facade.get_next_block_number() >= 4
-    block = facade.get_block_by_number(4)
+    block = next_block
+    assert BlockchainFacade.get_instance().get_next_block_number() >= block.get_block_number()
 
     hash_ = block.make_hash()
     block_confirmation = BlockConfirmation.create(