Process pending blocks

dmugtasimov · dmugtasimov · commit 8194295ff238 · 2022-03-25T20:48:47.000+03:00
diff --git a/node/blockchain/facade.py b/node/blockchain/facade.py
@@ -68,6 +68,7 @@ def add_block(self, block: Block, *, validate=True) -> 'ORMBlock':
             block.validate_business_logic()
 
         # Make blockchain state specific validations
+        # We need `bypass_lock_validation=True` because we have already validated it
         block.validate_blockchain_state_dependent(self, bypass_lock_validation=True)
 
         from node.blockchain.models import Block as ORMBlock
diff --git a/node/blockchain/inner_models/block.py b/node/blockchain/inner_models/block.py
@@ -60,7 +60,7 @@ def validate_signer(self, blockchain_facade):
 
     @lock(BLOCK_LOCK, expect_locked=True)
     def validate_blockchain_state_dependent(self, blockchain_facade):
-        self.message.validate_blockchain_state_dependent(blockchain_facade)
+        self.message.validate_blockchain_state_dependent(blockchain_facade, bypass_lock_validation=True)
         self.validate_signer(blockchain_facade)
 
 
diff --git a/node/blockchain/inner_models/block_message/base.py b/node/blockchain/inner_models/block_message/base.py
@@ -109,7 +109,7 @@ def validate_update(self, blockchain_facade):
 
     @lock(BLOCK_LOCK, expect_locked=True)
     def validate_blockchain_state_dependent(self, blockchain_facade):
-        self.request.validate_blockchain_state_dependent(blockchain_facade)
+        self.request.validate_blockchain_state_dependent(blockchain_facade, bypass_lock_validation=True)
         self.validate_number(blockchain_facade)
         self.validate_identifier(blockchain_facade)
         self.validate_update(blockchain_facade)
diff --git a/node/blockchain/inner_models/block_message/genesis.py b/node/blockchain/inner_models/block_message/genesis.py
@@ -5,9 +5,11 @@
 
 from pydantic import Field
 
+from node.blockchain.constants import BLOCK_LOCK
 from node.blockchain.inner_models.account_state import AccountState
 from node.blockchain.inner_models.node import Node
 from node.blockchain.inner_models.signed_change_request import GenesisSignedChangeRequest
+from node.blockchain.utils.lock import lock
 from node.core.exceptions import ValidationError
 
 from ...types import BlockIdentifier, Type
@@ -76,7 +78,8 @@ def validate_update(self, blockchain_facade):
         # TODO(dmu) MEDIUM: Is it critical not to be able to validate genesis block?
         pass
 
+    @lock(BLOCK_LOCK, expect_locked=True)
     def validate_blockchain_state_dependent(self, blockchain_facade):
-        super().validate_blockchain_state_dependent(blockchain_facade)
+        super().validate_blockchain_state_dependent(blockchain_facade, bypass_lock_validation=True)
         if blockchain_facade.has_blocks():
             raise ValidationError('Blockchain must not have blocks')
diff --git a/node/blockchain/inner_models/signed_change_request/base.py b/node/blockchain/inner_models/signed_change_request/base.py
@@ -66,7 +66,7 @@ def validate_account_lock(self, blockchain_facade):
 
     @lock(BLOCK_LOCK, expect_locked=True)
     def validate_blockchain_state_dependent(self, blockchain_facade):
-        self.message.validate_blockchain_state_dependent(blockchain_facade)
+        self.message.validate_blockchain_state_dependent(blockchain_facade, bypass_lock_validation=True)
         self.validate_account_lock(blockchain_facade)
 
     def get_type(self):
diff --git a/node/blockchain/inner_models/signed_change_request/coin_transfer.py b/node/blockchain/inner_models/signed_change_request/coin_transfer.py
@@ -1,3 +1,5 @@
+from node.blockchain.constants import BLOCK_LOCK
+from node.blockchain.utils.lock import lock
 from node.core.exceptions import ValidationError
 
 from ..signed_change_request_message import CoinTransferSignedChangeRequestMessage
@@ -11,8 +13,9 @@ def validate_business_logic(self):
         super().validate_business_logic()
         self.validate_circular_transactions()
 
+    @lock(BLOCK_LOCK, expect_locked=True)
     def validate_blockchain_state_dependent(self, blockchain_facade):
-        super().validate_blockchain_state_dependent(blockchain_facade)
+        super().validate_blockchain_state_dependent(blockchain_facade, bypass_lock_validation=True)
         self.validate_amount(blockchain_facade)
 
     def validate_circular_transactions(self):
diff --git a/node/blockchain/inner_models/signed_change_request_message/pv_schedule_update.py b/node/blockchain/inner_models/signed_change_request_message/pv_schedule_update.py
@@ -3,6 +3,8 @@
 from django.conf import settings
 from pydantic import Field, validator
 
+from node.blockchain.constants import BLOCK_LOCK
+from node.blockchain.utils.lock import lock
 from node.core.exceptions import ValidationError
 from node.core.utils.types import non_negative_intstr
 
@@ -40,6 +42,7 @@ def validate_block_numbers(self, blockchain_facade):
         if insertion_point > 1:
             raise ValidationError('Outdated block numbers detected in schedule')
 
+    @lock(BLOCK_LOCK, expect_locked=True)
     def validate_blockchain_state_dependent(self, blockchain_facade):
         super().validate_blockchain_state_dependent(blockchain_facade)
         self.validate_nodes_are_declared(blockchain_facade)
diff --git a/node/blockchain/mixins/validatable.py b/node/blockchain/mixins/validatable.py
@@ -3,9 +3,9 @@ class ValidatableMixin:
     def validate_business_logic(self):  # validate() is used by pydantic there we use a longer name
         raise NotImplementedError('Must be implemented in child class')
 
-    def validate_blockchain_state_dependent(self, blockchain_facade):
+    def validate_blockchain_state_dependent(self, blockchain_facade, **kwargs):
         raise NotImplementedError('Must be implemented in child class')
 
-    def validate_all(self, blockchain_facade):
+    def validate_all(self, blockchain_facade, **kwargs):
         self.validate_business_logic()
-        self.validate_blockchain_state_dependent(blockchain_facade)
+        self.validate_blockchain_state_dependent(blockchain_facade, **kwargs)
diff --git a/node/blockchain/serializers/block.py b/node/blockchain/serializers/block.py
@@ -8,6 +8,8 @@
 from node.blockchain.inner_models import Block as PydanticBlock
 from node.blockchain.models import Block as ORMBlock
 from node.blockchain.models import PendingBlock
+from node.blockchain.tasks.process_pending_blocks import start_process_pending_blocks_task
+from node.core.utils.misc import apply_on_commit
 
 logger = logging.getLogger(__name__)
 
@@ -22,16 +24,21 @@ def to_representation(self, instance):
         # This "hack" is needed to reduce deserialization / serialization overhead when reading blocks
         return OrderedDict(body=instance.body)
 
-    def validate_message(self, message):
-        is_invalid_number = ((block_number := message.get('number')) is None or
-                             block_number < BlockchainFacade.get_instance().get_next_block_number())
-        if is_invalid_number:
-            raise ValidationError('Invalid number')
-
-        return message
-
     def create(self, validated_data):
         block = PydanticBlock.parse_obj(validated_data)
+
+        facade = BlockchainFacade.get_instance()
+        block_number = block.get_block_number()
+        next_block_number = facade.get_next_block_number()
+        if block_number < next_block_number:
+            raise ValidationError('Invalid block number')
+
+        if block_number == next_block_number:
+            # Preliminary validation (we will revalidate it later) therefore providing `bypass_lock_validation=True`
+            block.validate_all(facade, bypass_lock_validation=True)
+        else:
+            block.validate_business_logic()
+
         instance, _ = PendingBlock.objects.update_or_create(
             number=block.get_block_number(),
             signer=block.signer,
@@ -42,6 +49,9 @@ def create(self, validated_data):
             },
         )
 
+        if block_number == next_block_number:
+            apply_on_commit(start_process_pending_blocks_task)
+
         return instance
 
     class Meta:
diff --git a/node/blockchain/tasks/process_block_confirmations.py b/node/blockchain/tasks/process_block_confirmations.py
@@ -97,10 +97,11 @@ def process_next_block() -> bool:
         raise ValidationError('Pending block body hash is not valid')  # we should never get here
 
     with transaction.atomic():
-        facade.add_block_from_json(block_body, expect_locked=True)
+        orm_block = facade.add_block_from_json(block_body, expect_locked=True)
         # There may be blocks with other hashes therefore we delete all of them
-        PendingBlock.objects.filter(number__lte=next_block_number).delete()
-        BlockConfirmation.objects.filter(number__lte=next_block_number).delete()
+        block_number = orm_block._id
+        PendingBlock.objects.filter(number__lte=block_number).delete()
+        BlockConfirmation.objects.filter(number__lte=block_number).delete()
 
     return True
 
diff --git a/node/blockchain/tasks/process_pending_blocks.py b/node/blockchain/tasks/process_pending_blocks.py
@@ -1,34 +1,43 @@
 import logging
 
 from celery import shared_task
+from pydantic.error_wrappers import ValidationError as PydanticValidationError
 
 from node.blockchain.facade import BlockchainFacade
-from node.blockchain.models import PendingBlock
+from node.blockchain.inner_models import BlockConfirmation as PydanticBlockConfirmation
+from node.blockchain.models import BlockConfirmation, PendingBlock
+from node.core.exceptions import ValidationError
+from node.core.utils.cryptography import get_signing_key
 
 logger = logging.getLogger(__name__)
 
 
-def validate_pending_block(orm_pending_block: PendingBlock):
-    pending_block = orm_pending_block.get_block()
-    return pending_block
-
-
 def process_next_block() -> bool:
-    # TODO(dmu) CRITICAL: Process pending blocks. To be implemented in
-    #                     https://thenewboston.atlassian.net/browse/BC-263
     facade = BlockchainFacade.get_instance()
     next_block_number = facade.get_next_block_number()
-
-    # There may be more than one pending block, but at most one of them can be valid
-    orm_pending_blocks = list(PendingBlock.objects.filter(number=next_block_number))
-    for orm_pending_block in orm_pending_blocks:
-        try:
-            validate_pending_block(orm_pending_block)
-            return False
-        except Exception:
-            logger.warning('Error while trying to validate pending block: %s', orm_pending_block, exc_info=True)
-
-    return False
+    own_signing_key = get_signing_key()
+    orm_block_confirmation = BlockConfirmation.objects.get_or_none(number=next_block_number, signer=own_signing_key)
+    if orm_block_confirmation:
+        return False  # block is already confirmed
+
+    primary_validator = facade.get_primary_validator()
+    assert primary_validator is not None
+    pending_block = PendingBlock.objects.get_or_none(number=next_block_number, signer=primary_validator.identifier)
+    if not pending_block:
+        return False
+
+    try:
+        block = pending_block.get_block()
+        block.validate_all(facade)
+    except (PydanticValidationError, ValidationError):
+        logger.warning('Block did not pass validation: %s', pending_block, exc_info=True)
+        return False
+
+    block_confirmation = PydanticBlockConfirmation.create_from_block(block, own_signing_key)
+    BlockConfirmation.objects.update_or_create_from_block_confirmation(block_confirmation)
+    # TODO(dmu) CRITICAL: Send out the confirmation to other CVs
+    #                     https://thenewboston.atlassian.net/browse/BC-291
+    return True
 
 
 @shared_task
diff --git a/node/blockchain/tests/test_rest_api/test_block.py b/node/blockchain/tests/test_rest_api/test_block.py
@@ -103,7 +103,7 @@ def test_create_pending_block(regular_node, regular_node_key_pair, primary_valid
     block = make_block(block_message, primary_validator_key_pair.private)
 
     payload = block.json()
-    with patch('node.blockchain.views.block.start_process_pending_blocks_task') as mock:
+    with patch('node.blockchain.serializers.block.start_process_pending_blocks_task') as mock:
         response = api_client.post('/api/blocks/', payload, content_type='application/json')
 
     assert response.status_code == 204
@@ -113,6 +113,30 @@ def test_create_pending_block(regular_node, regular_node_key_pair, primary_valid
     assert pending_block.body == payload
 
 
+@pytest.mark.usefixtures('base_blockchain')
+def test_create_future_block(regular_node, regular_node_key_pair, primary_validator_key_pair, api_client):
+    assert not PendingBlock.objects.exists()
+
+    facade = BlockchainFacade.get_instance()
+    block_message = make_node_declaration_block_message(regular_node, regular_node_key_pair, facade)
+    block_message_dict = block_message.dict()
+    block_message_dict['number'] = facade.get_next_block_number() + 1
+    block_message = NodeDeclarationBlockMessage.parse_obj(block_message_dict)
+
+    assert facade.get_primary_validator().identifier == primary_validator_key_pair.public
+    block = make_block(block_message, primary_validator_key_pair.private)
+
+    payload = block.json()
+    with patch('node.blockchain.serializers.block.start_process_pending_blocks_task') as mock:
+        response = api_client.post('/api/blocks/', payload, content_type='application/json')
+
+    assert response.status_code == 204
+    mock.assert_not_called()
+    pending_block = PendingBlock.objects.get_or_none(number=block.get_block_number(), hash=block.make_hash())
+    assert pending_block
+    assert pending_block.body == payload
+
+
 @pytest.mark.usefixtures('rich_blockchain')
 def test_try_to_create_outdated_block(regular_node, regular_node_key_pair, primary_validator_key_pair, api_client):
     assert not PendingBlock.objects.exists()
@@ -127,11 +151,40 @@ def test_try_to_create_outdated_block(regular_node, regular_node_key_pair, prima
     block = make_block(block_message, primary_validator_key_pair.private)
 
     payload = block.json()
-    with patch('node.blockchain.views.block.start_process_pending_blocks_task') as mock:
+    with patch('node.blockchain.serializers.block.start_process_pending_blocks_task') as mock:
+        response = api_client.post('/api/blocks/', payload, content_type='application/json')
+
+    assert response.status_code == 400
+    assert response.json() == [{'code': 'invalid', 'message': 'Invalid block number'}]
+    mock.assert_not_called()
+
+    # This is because we have queried the database and nested transactions (save points) are not supported
+    assert connection.needs_rollback
+    connection.set_rollback(False)
+
+    assert not PendingBlock.objects.exists()
+
+
+@pytest.mark.usefixtures('rich_blockchain')
+def test_try_to_create_invalid_signature_block(
+    regular_node, regular_node_key_pair, primary_validator_key_pair, api_client
+):
+    assert not PendingBlock.objects.exists()
+
+    facade = BlockchainFacade.get_instance()
+    block_message = make_node_declaration_block_message(regular_node, regular_node_key_pair, facade)
+
+    assert facade.get_primary_validator().identifier == primary_validator_key_pair.public
+    block = make_block(block_message, primary_validator_key_pair.private)
+
+    payload_dict = json.loads(block.json())
+    payload_dict['signature'] = '0' * 128
+    payload = json.dumps(payload_dict)
+    with patch('node.blockchain.serializers.block.start_process_pending_blocks_task') as mock:
         response = api_client.post('/api/blocks/', payload, content_type='application/json')
 
     assert response.status_code == 400
-    assert response.json() == {'message': [{'code': 'invalid', 'message': 'Invalid number'}]}
+    assert response.json() == {'non_field_errors': [{'code': 'invalid', 'message': 'Invalid signature'}]}
     mock.assert_not_called()
 
     # This is because we have queried the database and nested transactions (save points) are not supported
diff --git a/node/blockchain/views/block.py b/node/blockchain/views/block.py
@@ -2,15 +2,13 @@
 from django.http import HttpResponse
 from django_filters.rest_framework import DjangoFilterBackend, RangeFilter
 from rest_framework import status
-from rest_framework.mixins import ListModelMixin, RetrieveModelMixin
+from rest_framework.mixins import CreateModelMixin, ListModelMixin, RetrieveModelMixin
 from rest_framework.response import Response
 from rest_framework.viewsets import GenericViewSet
 
 from node.blockchain.models import Block
 from node.blockchain.serializers.block import BlockSerializer
-from node.blockchain.tasks.process_pending_blocks import start_process_pending_blocks_task
 from node.core.pagination import CustomLimitOffsetNoCountPagination
-from node.core.utils.misc import apply_on_commit
 
 from ..constants import LAST_BLOCK_ID
 
@@ -23,7 +21,7 @@ class Meta:
         fields = ('block_number',)
 
 
-class BlockViewSet(RetrieveModelMixin, ListModelMixin, GenericViewSet):
+class BlockViewSet(CreateModelMixin, RetrieveModelMixin, ListModelMixin, GenericViewSet):
     serializer_class = BlockSerializer
     # TODO(dmu) MEDIUM: We might need to add dynamic ordering later
     queryset = BlockSerializer.Meta.model.objects.order_by('_id')
@@ -32,10 +30,7 @@ class BlockViewSet(RetrieveModelMixin, ListModelMixin, GenericViewSet):
     filterset_class = BlockFilterSet
 
     def create(self, request, *args, **kwargs):
-        serializer = self.get_serializer(data=request.data)
-        serializer.is_valid(raise_exception=True)
-        serializer.save()
-        apply_on_commit(start_process_pending_blocks_task)
+        super().create(request, *args, **kwargs)
         return Response(status=status.HTTP_204_NO_CONTENT)
 
     def list(self, request, *args, **kwargs):  # noqa: A003
diff --git a/node/core/tests/test_node_client.py b/node/core/tests/test_node_client.py
@@ -104,7 +104,7 @@ def test_send_block_to_address_integration(
     block = make_block(block_message, primary_validator_key_pair.private)
 
     for block_arg in (block, block.json()):
-        with patch('node.blockchain.views.block.start_process_pending_blocks_task') as mock:
+        with patch('node.blockchain.serializers.block.start_process_pending_blocks_task') as mock:
             response = smart_mocked_node_client.send_block(test_server_address, block_arg)
 
         assert response.status_code == 204
