Merge pull request #206 from thenewboston-developers/BC-263-validate-pending-blocks

Some refactoring along with drafted implementation

Author: dmugtasimov

node/blockchain/migrations/0004_pendingblock.py

@@ -18,10 +18,11 @@ class Migration(migrations.Migration):
                 ('_id', models.UUIDField(default=uuid.uuid4, primary_key=True, serialize=False)),
                 ('number', models.PositiveBigIntegerField()),
                 ('hash', models.CharField(max_length=128)),
+                ('signer', models.CharField(max_length=64)),
                 ('body', models.BinaryField()),
             ],
             options={
-                'ordering': ('number', 'hash'),
+                'ordering': ('number', 'signer'),
                 'unique_together': {('number', 'hash')},
             },
         ),

node/blockchain/models/block_confirmation.py

@@ -30,10 +30,10 @@ def update_or_create_from_block_confirmation(self, block_confirmation: PydanticB
 
 class BlockConfirmation(CustomModel):
 
-    _id = models.UUIDField(primary_key=True, default=uuid.uuid4)  # noqa: A003
+    _id = models.UUIDField(primary_key=True, default=uuid.uuid4)
     number = models.PositiveBigIntegerField()
     hash = models.CharField(max_length=128)  # noqa: A003
-    signer = models.CharField(max_length=64)  # noqa: A003
+    signer = models.CharField(max_length=64)
     body = models.BinaryField()
 
     objects = BlockConfirmationManager()

node/blockchain/models/pending_block.py

@@ -8,16 +8,17 @@
 
 class PendingBlock(CustomModel):
 
-    _id = models.UUIDField(primary_key=True, default=uuid.uuid4)  # noqa: A003
+    _id = models.UUIDField(primary_key=True, default=uuid.uuid4)
     number = models.PositiveBigIntegerField()
     hash = models.CharField(max_length=128)  # noqa: A003
+    signer = models.CharField(max_length=64)
     body = models.BinaryField()
 
     def get_block(self) -> PydanticBlock:
         return PydanticBlock.parse_raw(self.body)
 
     class Meta:
-        unique_together = ('number', 'hash')
+        unique_together = ('number', 'signer')
         ordering = unique_together
 
     def __str__(self):

node/blockchain/serializers/block.py

@@ -2,7 +2,9 @@
 from collections import OrderedDict
 
 from rest_framework import serializers
+from rest_framework.exceptions import ValidationError
 
+from node.blockchain.facade import BlockchainFacade
 from node.blockchain.inner_models import Block as PydanticBlock
 from node.blockchain.models import Block as ORMBlock
 from node.blockchain.models import PendingBlock
@@ -20,19 +22,25 @@ 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)
-
-        block_number = block.get_block_number()
-        block_hash = block.make_hash()
-        instance, is_created = PendingBlock.objects.get_or_create(
-            number=block_number,
-            hash=block_hash,
+        instance, _ = PendingBlock.objects.update_or_create(
+            number=block.get_block_number(),
+            signer=block.signer,
             # TODO(dmu) MEDIUM: It would be more effective to save original request body instead of serializing again
-            defaults={'body': block.json()},
+            defaults={
+                'hash': block.make_hash(),
+                'body': block.json(),
+            },
         )
-        if not is_created:
-            logger.warning('Block number %s, hash %s appeared more than once')
 
         return instance
 

node/blockchain/serializers/block_confirmation.py

@@ -5,6 +5,8 @@
 from node.core.serializers import ValidateUnknownFieldsMixin
 
 
+# TODO(dmu) MEDIUM: Consider implementing BlockConfirmationSerializer as ModelSerializer similar to
+#                   node.blockchain.serializers.block.BlockSerializer
 class BlockConfirmationSerializer(serializers.Serializer, ValidateUnknownFieldsMixin):
     signer = serializers.CharField()
     signature = serializers.CharField()

node/blockchain/tasks/process_pending_blocks.py

@@ -1,11 +1,41 @@
+import logging
+
 from celery import shared_task
 
+from node.blockchain.facade import BlockchainFacade
+from node.blockchain.models import PendingBlock
 
-@shared_task
-def process_pending_blocks_task():
+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
-    pass
+    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
+
+
+@shared_task
+def process_pending_blocks_task():
+    should_process_next_block = True
+    while should_process_next_block:
+        should_process_next_block = process_next_block()
 
 
 def start_process_pending_blocks_task():

node/blockchain/tests/test_rest_api/test_block.py

@@ -2,8 +2,10 @@
 from unittest.mock import patch
 
 import pytest
+from django.db import connection
 
 from node.blockchain.facade import BlockchainFacade
+from node.blockchain.inner_models import NodeDeclarationBlockMessage
 from node.blockchain.models import Block, PendingBlock
 from node.blockchain.tests.factories.block import make_block
 from node.blockchain.tests.factories.block_message.node_declaration import make_node_declaration_block_message
@@ -109,3 +111,31 @@ def test_create_pending_block(regular_node, regular_node_key_pair, primary_valid
     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()
+
+    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.views.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'}]}
+    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()