[PAN-1917] Ensure blockchain nonce uniqueness

pantos/validatornode/blockchains/ethereum.py

@@ -486,6 +486,17 @@ def __get_blockchain_nodes_domains(
             blockchain_nodes_domains.append(blockchain_node_domain)
         return ', '.join(blockchain_nodes_domains)
 
+    def __get_nonce(self, node_connections: NodeConnections,
+                    internal_transfer_id: int) -> int:
+        transaction_count = node_connections.eth.get_transaction_count(
+            self.__address).get_maximum_result()
+        database_access.update_transfer_nonce(internal_transfer_id,
+                                              self.get_blockchain(),
+                                              transaction_count)
+        nonce = database_access.read_transfer_nonce(internal_transfer_id)
+        assert nonce is not None
+        return nonce
+
     def __submit_transfer_to_request(
             self, node_connections: NodeConnections, internal_transfer_id: int,
             on_chain_request: _OnChainTransferToRequest,
@@ -499,14 +510,7 @@ def __submit_transfer_to_request(
             self._get_config()['min_adaptable_fee_per_gas']
         max_total_fee_per_gas = self._get_config().get('max_total_fee_per_gas')
         amount = None
-        transaction_count = typing.cast(
-            int,
-            node_connections.eth.get_transaction_count(
-                self.__address).get_maximum_result())
-        database_access.update_transfer_nonce(internal_transfer_id,
-                                              self.get_blockchain(),
-                                              transaction_count)
-        nonce = database_access.read_transfer_nonce(internal_transfer_id)
+        nonce = self.__get_nonce(node_connections, internal_transfer_id)
         adaptable_fee_increase_factor = \
             self._get_config()['adaptable_fee_increase_factor']
         blocks_until_resubmission = \

pantos/validatornode/business/transfers.py

@@ -303,6 +303,12 @@ def validate_transfer(self, internal_transfer_id: int,
                 if transfer.is_reversal_transfer else
                 'outgoing token transfer confirmed', extra=vars(transfer)
                 | {'interal_transfer_id': internal_transfer_id})
+            if transfer.is_reversal_transfer:
+                database_access.update_reversal_transfer(
+                    internal_transfer_id,
+                    transfer.eventual_destination_blockchain,
+                    transfer.eventual_recipient_address,
+                    transfer.eventual_destination_token_address)
             validator_nonce = \
                 database_access.read_validator_nonce_by_internal_transfer_id(
                     internal_transfer_id)
@@ -312,9 +318,7 @@ def validate_transfer(self, internal_transfer_id: int,
                 source_blockchain_client if transfer.is_reversal_transfer else
                 destination_blockchain_client)
             database_access.update_transfer_submitted_destination_transaction(
-                internal_transfer_id, transfer.eventual_destination_blockchain,
-                transfer.eventual_recipient_address,
-                transfer.eventual_destination_token_address,
+                internal_transfer_id,
                 submission_response.destination_hub_address,
                 submission_response.destination_forwarder_address)
             database_access.update_transfer_status(

pantos/validatornode/database/access.py

@@ -241,9 +241,9 @@ def read_transfer_id(source_blockchain: Blockchain,
         return session.execute(statement).scalar_one_or_none()
 
 
-def read_transfer_nonce(internal_transfer_id: int) -> int:
-    """Read the nonce for a transfer submitted to the destination
-    blockchain.
+def read_transfer_nonce(internal_transfer_id: int) -> int | None:
+    """Read the nonce for a transfer transaction submitted to the
+    destination blockchain.
 
     Parameters
     ----------
@@ -252,15 +252,16 @@ def read_transfer_nonce(internal_transfer_id: int) -> int:
 
     Returns
     -------
-    int
-        The nonce for the transfer on the destination blockchain.
+    int or None
+        The nonce for the transfer transaction on the destination
+        blockchain, or None if no blockchain nonce has been assigned to
+        the transfer.
 
     """
     statement = sqlalchemy.select(
         Transfer.nonce).filter(Transfer.id == internal_transfer_id)
     with get_session() as session:
-        result = session.execute(statement).fetchall()
-        return result[0][0]
+        return session.execute(statement).scalar_one_or_none()
 
 
 @dataclasses.dataclass
@@ -506,6 +507,40 @@ def update_blockchain_last_block_number(blockchain: Blockchain,
                 sqlalchemy.Column, last_block_number)
 
 
+def update_reversal_transfer(
+        internal_transfer_id: int, destination_blockchain: Blockchain,
+        recipient_address: BlockchainAddress,
+        destination_token_address: BlockchainAddress) -> None:
+    """Update a reversal transfer's destination blockchain attributes.
+
+    Parameters
+    ----------
+    internal_transfer_id : int
+        The unique internal ID of the transfer.
+    destination_blockchain : Blockchain
+        The transfer's destination blockchain.
+    recipient_address : BlockchainAddress
+        The transfer's recipient address.
+    destination_token_address : BlockchainAddress
+        The address of the transferred token on the destination
+        blockchain.
+
+    """
+    with get_session_maker().begin() as session:
+        destination_token_contract_id = _read_token_contract_id(
+            session, destination_blockchain, destination_token_address)
+        if destination_token_contract_id is None:
+            destination_token_contract_id = _create_token_contract(
+                session, destination_blockchain, destination_token_address)
+        statement = sqlalchemy.update(Transfer).where(
+            Transfer.id == internal_transfer_id).values(
+                destination_blockchain_id=destination_blockchain.value,
+                recipient_address=recipient_address,
+                destination_token_contract_id=destination_token_contract_id,
+                updated=datetime.datetime.utcnow())
+        session.execute(statement)
+
+
 def update_transfer_confirmed_destination_transaction(
         internal_transfer_id: int, destination_transfer_id: int,
         destination_transaction_id: str,
@@ -536,10 +571,7 @@ def update_transfer_confirmed_destination_transaction(
 
 
 def update_transfer_submitted_destination_transaction(
-        internal_transfer_id: int, destination_blockchain: Blockchain,
-        recipient_address: BlockchainAddress,
-        destination_token_address: BlockchainAddress,
-        destination_hub_address: BlockchainAddress,
+        internal_transfer_id: int, destination_hub_address: BlockchainAddress,
         destination_forwarder_address: BlockchainAddress) -> None:
     """Update a transfer's attributes for its submitted destination
     blockchain transaction.
@@ -548,13 +580,6 @@ def update_transfer_submitted_destination_transaction(
     ----------
     internal_transfer_id : int
         The unique internal ID of the transfer.
-    destination_blockchain : Blockchain
-        The transfer's destination blockchain.
-    recipient_address : BlockchainAddress
-        The transfer's recipient address.
-    destination_token_address : BlockchainAddress
-        The address of the transferred token on the destination
-        blockchain.
     destination_hub_address : BlockchainAddress
         The address of the hub contract on the destination blockchain.
     destination_forwarder_address : BlockchainAddress
@@ -563,14 +588,12 @@ def update_transfer_submitted_destination_transaction(
 
     """
     with get_session_maker().begin() as session:
-        transfer = typing.cast(Transfer,
-                               session.get(Transfer, internal_transfer_id))
-        assert transfer is not None
-        destination_token_contract_id = _read_token_contract_id(
-            session, destination_blockchain, destination_token_address)
-        if destination_token_contract_id is None:
-            destination_token_contract_id = _create_token_contract(
-                session, destination_blockchain, destination_token_address)
+        select_statement = sqlalchemy.select(
+            Transfer.destination_blockchain_id).where(
+                Transfer.id == internal_transfer_id)
+        destination_blockchain_id = session.execute(
+            select_statement).scalar_one()
+        destination_blockchain = Blockchain(destination_blockchain_id)
         destination_hub_contract_id = _read_hub_contract_id(
             session, destination_blockchain, destination_hub_address)
         if destination_hub_contract_id is None:
@@ -581,18 +604,13 @@ def update_transfer_submitted_destination_transaction(
         if destination_forwarder_contract_id is None:
             destination_forwarder_contract_id = _create_forwarder_contract(
                 session, destination_blockchain, destination_forwarder_address)
-        transfer.destination_blockchain_id = typing.cast(
-            sqlalchemy.Column, destination_blockchain.value)
-        transfer.recipient_address = typing.cast(sqlalchemy.Column,
-                                                 recipient_address)
-        transfer.destination_token_contract_id = typing.cast(
-            sqlalchemy.Column, destination_token_contract_id)
-        transfer.destination_hub_contract_id = typing.cast(
-            sqlalchemy.Column, destination_hub_contract_id)
-        transfer.destination_forwarder_contract_id = typing.cast(
-            sqlalchemy.Column, destination_forwarder_contract_id)
-        transfer.updated = typing.cast(sqlalchemy.Column,
-                                       datetime.datetime.utcnow())
+        update_statement = sqlalchemy.update(Transfer).where(
+            Transfer.id == internal_transfer_id).values(
+                destination_hub_contract_id=destination_hub_contract_id,
+                destination_forwarder_contract_id=  # noqa: E251
+                destination_forwarder_contract_id,
+                updated=datetime.datetime.utcnow())
+        session.execute(update_statement)
 
 
 def update_transfer_nonce(internal_transfer_id: int,

pantos/validatornode/database/migrations/versions/e648dd961dfc_unique_blockchain_nonce_constraint.py

@@ -0,0 +1,26 @@
+"""unique_blockchain_nonce_constraint
+
+Revision ID: e648dd961dfc
+Revises: 536e50b24e1f
+Create Date: 2024-06-06 04:13:50.740664
+
+"""
+import alembic
+
+# revision identifiers, used by Alembic.
+revision = 'e648dd961dfc'
+down_revision = '536e50b24e1f'
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    alembic.op.execute('UPDATE transfers SET nonce = NULL')
+    alembic.op.create_unique_constraint('unique_blockchain_nonce', 'transfers',
+                                        ['destination_blockchain_id', 'nonce'],
+                                        deferrable='True')
+
+
+def downgrade() -> None:
+    alembic.op.drop_constraint('unique_blockchain_nonce', 'transfers',
+                               type_='unique')

pantos/validatornode/database/models.py

@@ -7,8 +7,8 @@
 import sqlalchemy
 import sqlalchemy.orm
 
+UNIQUE_BLOCKCHAIN_NONCE_CONSTRAINT = 'unique_blockchain_nonce'
 UNIQUE_VALIDATOR_NONCE_CONSTRAINT = 'unique_validator_nonce'
-"""Name of the unique validator nonce constraint."""
 
 Base: typing.Any = sqlalchemy.orm.declarative_base()
 """SQLAlchemy base class for declarative class definitions."""
@@ -366,6 +366,9 @@ class Transfer(Base):
     __table_args__ = (sqlalchemy.UniqueConstraint(
         destination_forwarder_contract_id, validator_nonce,
         name=UNIQUE_VALIDATOR_NONCE_CONSTRAINT),
+                      sqlalchemy.UniqueConstraint(
+                          destination_blockchain_id, nonce, deferrable=True,
+                          name=UNIQUE_BLOCKCHAIN_NONCE_CONSTRAINT),
                       sqlalchemy.UniqueConstraint(source_blockchain_id,
                                                   source_transfer_id),
                       sqlalchemy.UniqueConstraint(destination_blockchain_id,

tests/business/transfers/test_validate_transfer.py

@@ -78,12 +78,15 @@ def test_validate_transfer_confirmed_correct(
         transfer_to_submission_start_response.destination_hub_address
     destination_forwarder_address = \
         transfer_to_submission_start_response.destination_forwarder_address
-    mock_database_access.update_transfer_submitted_destination_transaction.\
-        assert_called_once_with(
+    if is_reversal_transfer:
+        mock_database_access.update_reversal_transfer.assert_called_once_with(
             internal_transfer_id,
             cross_chain_transfer.eventual_destination_blockchain,
             cross_chain_transfer.eventual_recipient_address,
-            cross_chain_transfer.eventual_destination_token_address,
+            cross_chain_transfer.eventual_destination_token_address)
+    mock_database_access.update_transfer_submitted_destination_transaction.\
+        assert_called_once_with(
+            internal_transfer_id,
             destination_hub_address,
             destination_forwarder_address)
     mock_database_access.update_transfer_status.assert_called_once_with(

tests/database/sqlite/access/test_update_reversal_transfer.py

@@ -0,0 +1,39 @@
+import unittest.mock
+
+import pytest
+from pantos.common.blockchains.enums import Blockchain
+
+from pantos.validatornode.database.access import update_reversal_transfer
+from tests.database.utilities import modify_model_instance
+
+
+@pytest.mark.parametrize('destination_token_contract_existent', [True, False])
+@unittest.mock.patch('pantos.validatornode.database.access.get_session_maker')
+@unittest.mock.patch('pantos.validatornode.database.access.get_session')
+def test_update_reversal_transfer_correct(
+        mock_get_session, mock_get_session_maker,
+        destination_token_contract_existent,
+        initialized_database_session_maker, transfer,
+        other_destination_blockchain, other_recipient_address,
+        other_destination_token_contract):
+    initialized_database_session = initialized_database_session_maker()
+    mock_get_session.return_value = initialized_database_session
+    mock_get_session_maker.return_value = initialized_database_session_maker
+    transfer = modify_model_instance(transfer, destination_hub_contract=None,
+                                     destination_forwarder_contract=None)
+    initialized_database_session.add(transfer)
+    if destination_token_contract_existent:
+        initialized_database_session.add(other_destination_token_contract)
+    initialized_database_session.commit()
+    update_reversal_transfer(transfer.id,
+                             Blockchain(other_destination_blockchain.id),
+                             other_recipient_address,
+                             other_destination_token_contract.address)
+    initialized_database_session.refresh(transfer)
+    assert (
+        transfer.destination_blockchain_id == other_destination_blockchain.id)
+    assert (transfer.recipient_address == other_recipient_address)
+    assert (transfer.destination_token_contract.blockchain_id ==
+            transfer.destination_blockchain_id)
+    assert (transfer.destination_token_contract.address ==
+            other_destination_token_contract.address)

tests/database/sqlite/access/test_update_transfer_submitted_destination_transaction.py

@@ -1,7 +1,6 @@
 import unittest.mock
 
 import pytest
-from pantos.common.blockchains.enums import Blockchain
 
 from pantos.validatornode.database.access import \
     update_transfer_submitted_destination_transaction
@@ -11,43 +10,31 @@
 @pytest.mark.parametrize('destination_forwarder_contract_existent',
                          [True, False])
 @pytest.mark.parametrize('destination_hub_contract_existent', [True, False])
-@pytest.mark.parametrize('destination_token_contract_existent', [True, False])
 @unittest.mock.patch('pantos.validatornode.database.access.get_session_maker')
 @unittest.mock.patch('pantos.validatornode.database.access.get_session')
 def test_update_transfer_submitted_destination_transaction_correct(
         mock_get_session, mock_get_session_maker,
-        destination_token_contract_existent, destination_hub_contract_existent,
+        destination_hub_contract_existent,
         destination_forwarder_contract_existent,
         initialized_database_session_maker, transfer,
-        other_destination_blockchain, other_recipient_address,
-        other_destination_token_contract, other_destination_hub_contract,
+        other_destination_blockchain, other_destination_hub_contract,
         other_destination_forwarder_contract):
     initialized_database_session = initialized_database_session_maker()
     mock_get_session.return_value = initialized_database_session
     mock_get_session_maker.return_value = initialized_database_session_maker
-    transfer = modify_model_instance(transfer, destination_hub_contract=None,
-                                     destination_forwarder_contract=None)
+    transfer = modify_model_instance(
+        transfer, destination_blockchain_id=other_destination_blockchain.id,
+        destination_hub_contract=None, destination_forwarder_contract=None)
     initialized_database_session.add(transfer)
-    if destination_token_contract_existent:
-        initialized_database_session.add(other_destination_token_contract)
     if destination_hub_contract_existent:
         initialized_database_session.add(other_destination_hub_contract)
     if destination_forwarder_contract_existent:
         initialized_database_session.add(other_destination_forwarder_contract)
     initialized_database_session.commit()
     update_transfer_submitted_destination_transaction(
-        transfer.id, Blockchain(other_destination_blockchain.id),
-        other_recipient_address, other_destination_token_contract.address,
-        other_destination_hub_contract.address,
+        transfer.id, other_destination_hub_contract.address,
         other_destination_forwarder_contract.address)
     initialized_database_session.refresh(transfer)
-    assert (
-        transfer.destination_blockchain_id == other_destination_blockchain.id)
-    assert (transfer.recipient_address == other_recipient_address)
-    assert (transfer.destination_token_contract.blockchain_id ==
-            transfer.destination_blockchain_id)
-    assert (transfer.destination_token_contract.address ==
-            other_destination_token_contract.address)
     assert (transfer.destination_hub_contract.blockchain_id ==
             transfer.destination_blockchain_id)
     assert (transfer.destination_hub_contract.address ==

tests/database/sqlite/conftest.py

@@ -2,6 +2,8 @@
 import sqlalchemy
 import sqlalchemy.orm
 
+from pantos.validatornode.database.models import \
+    UNIQUE_BLOCKCHAIN_NONCE_CONSTRAINT
 from pantos.validatornode.database.models import Base
 
 
@@ -15,6 +17,10 @@ def database_engine():
 
 @pytest.fixture(scope='session')
 def database_session_maker(database_engine):
+    for constraint in Base.metadata.tables['transfers'].constraints:
+        if constraint.name == UNIQUE_BLOCKCHAIN_NONCE_CONSTRAINT:
+            constraint.deferrable = None
+            break
     Base.metadata.create_all(bind=database_engine)
     yield sqlalchemy.orm.sessionmaker(bind=database_engine)
     Base.metadata.drop_all(bind=database_engine)