diff --git a/node/blockchain/migrations/0004_pendingblock.py b/node/blockchain/migrations/0004_pendingblock.py index 1d2414cb..3d83491f 100644 --- a/node/blockchain/migrations/0004_pendingblock.py +++ b/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')}, }, ), diff --git a/node/blockchain/models/block_confirmation.py b/node/blockchain/models/block_confirmation.py index 5e7fcd54..c1505c57 100644 --- a/node/blockchain/models/block_confirmation.py +++ b/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() diff --git a/node/blockchain/models/pending_block.py b/node/blockchain/models/pending_block.py index 0e5143a5..2efec50b 100644 --- a/node/blockchain/models/pending_block.py +++ b/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): diff --git a/node/blockchain/serializers/block.py b/node/blockchain/serializers/block.py index 294a8608..5dba63ca 100644 --- a/node/blockchain/serializers/block.py +++ b/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 diff --git a/node/blockchain/serializers/block_confirmation.py b/node/blockchain/serializers/block_confirmation.py index d915c800..cfd7ec89 100644 --- a/node/blockchain/serializers/block_confirmation.py +++ b/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() diff --git a/node/blockchain/tasks/process_pending_blocks.py b/node/blockchain/tasks/process_pending_blocks.py index bc708ee7..9ed478bd 100644 --- a/node/blockchain/tasks/process_pending_blocks.py +++ b/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(): diff --git a/node/blockchain/tests/test_rest_api/test_block.py b/node/blockchain/tests/test_rest_api/test_block.py index 3d11096c..de2091c6 100644 --- a/node/blockchain/tests/test_rest_api/test_block.py +++ b/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()