Don't delete payment task when receiving invalid secret request

docs/changelog.rst

@@ -2,6 +2,8 @@
 Changelog
 =========
 
+* :bug:`3001` Don't delete payment task when receiving invalid secret request.
+
 * :release:`0.16.0 <2018-11-09>`
 * :bug'`2963` Fixes an overflow issue with the hint of the join network dialog.
 * :bug:`2973` Introduce special handling of infura endpoints so that the old getTransactionCount is used.

raiden/storage/sqlite.py

@@ -8,7 +8,7 @@
 from raiden.utils import typing
 
 # The latest DB version
-RAIDEN_DB_VERSION = 11
+RAIDEN_DB_VERSION = 12
 
 
 class EventRecord(typing.NamedTuple):

raiden/tests/unit/transfer/mediated_transfer/test_initiatorstate.py

@@ -241,6 +241,25 @@ def test_state_wait_secretrequest_valid():
 
     assert len(iteration.events) == 1
     assert isinstance(iteration.events[0], SendSecretReveal)
+    assert iteration.new_state.initiator.received_secret_request is True
+
+    state_change_2 = ReceiveSecretRequest(
+        UNIT_TRANSFER_IDENTIFIER,
+        lock.amount,
+        lock.expiration,
+        lock.secrethash,
+        UNIT_TRANSFER_TARGET,
+    )
+
+    iteration2 = initiator_manager.state_transition(
+        iteration.new_state,
+        state_change_2,
+        channel_map,
+        pseudo_random_generator,
+        block_number,
+    )
+
+    assert len(iteration2.events) == 0
 
 
 def test_state_wait_secretrequest_invalid_amount():
@@ -286,6 +305,25 @@ def test_state_wait_secretrequest_invalid_amount():
 
     assert len(iteration.events) == 1
     assert isinstance(iteration.events[0], EventPaymentSentFailed)
+    assert iteration.new_state.initiator.received_secret_request is True
+
+    state_change_2 = ReceiveSecretRequest(
+        UNIT_TRANSFER_IDENTIFIER,
+        lock.amount,
+        lock.expiration,
+        lock.secrethash,
+        UNIT_TRANSFER_TARGET,
+    )
+
+    iteration2 = initiator_manager.state_transition(
+        iteration.new_state,
+        state_change_2,
+        channel_map,
+        pseudo_random_generator,
+        block_number,
+    )
+
+    assert len(iteration2.events) == 0
 
 
 def test_state_wait_secretrequest_invalid_amount_and_sender():
@@ -330,6 +368,27 @@ def test_state_wait_secretrequest_invalid_amount_and_sender():
     )
 
     assert len(iteration.events) == 0
+    assert iteration.new_state.initiator.received_secret_request is False
+
+    # Now the proper target sends the message, this should be applied
+    state_change_2 = ReceiveSecretRequest(
+        UNIT_TRANSFER_IDENTIFIER,
+        lock.amount,
+        lock.expiration,
+        lock.secrethash,
+        UNIT_TRANSFER_TARGET,
+    )
+
+    iteration2 = initiator_manager.state_transition(
+        iteration.new_state,
+        state_change_2,
+        channel_map,
+        pseudo_random_generator,
+        block_number,
+    )
+
+    assert iteration2.new_state.initiator.received_secret_request is True
+    assert isinstance(iteration2.events[0], SendSecretReveal)
 
 
 def test_state_wait_unlock_valid():

raiden/transfer/mediated_transfer/initiator.py

@@ -244,13 +244,19 @@ def handle_secretrequest(
         initiator_state.transfer_description.secrethash,
     )
 
+    already_received_secret_request = initiator_state.received_secret_request
+
     is_valid_secretrequest = (
-        is_message_from_target and
         state_change.amount == initiator_state.transfer_description.amount and
         state_change.expiration == lock.expiration
     )
 
-    if is_valid_secretrequest:
+    if already_received_secret_request and is_message_from_target:
+        # A secret request was received earlier, all subsequent are ignored
+        # as it might be an attack
+        iteration = TransitionResult(initiator_state, list())
+
+    elif is_valid_secretrequest and is_message_from_target:
         # Reveal the secret to the target node and wait for its confirmation.
         # At this point the transfer is not cancellable anymore as either the lock
         # timeouts or a secret reveal is received.
@@ -268,6 +274,7 @@ def handle_secretrequest(
         )
 
         initiator_state.revealsecret = revealsecret
+        initiator_state.received_secret_request = True
         iteration = TransitionResult(initiator_state, [revealsecret])
 
     elif not is_valid_secretrequest and is_message_from_target:
@@ -278,7 +285,9 @@ def handle_secretrequest(
             target=initiator_state.transfer_description.target,
             reason='bad secret request message from target',
         )
-        iteration = TransitionResult(None, [cancel])
+
+        initiator_state.received_secret_request = True
+        iteration = TransitionResult(initiator_state, [cancel])
 
     else:
         iteration = TransitionResult(initiator_state, list())

raiden/transfer/mediated_transfer/state.py

@@ -91,6 +91,7 @@ class InitiatorTransferState(State):
         'channel_identifier',
         'transfer',
         'revealsecret',
+        'received_secret_request',
     )
 
     def __init__(
@@ -99,6 +100,7 @@ def __init__(
             channel_identifier: typing.ChannelID,
             transfer: 'LockedTransferUnsignedState',
             revealsecret: typing.Optional['SendSecretReveal'],
+            received_secret_request: bool = False,
     ):
 
         if not isinstance(transfer_description, TransferDescriptionWithSecretState):
@@ -114,6 +116,7 @@ def __init__(
         self.channel_identifier = channel_identifier
         self.transfer = transfer
         self.revealsecret = revealsecret
+        self.received_secret_request = received_secret_request
 
     def __repr__(self):
         return '<InitiatorTransferState transfer:{} channel:{}>'.format(
@@ -127,7 +130,8 @@ def __eq__(self, other):
             self.transfer_description == other.transfer_description and
             self.channel_identifier == other.channel_identifier and
             self.transfer == other.transfer and
-            self.revealsecret == other.revealsecret
+            self.revealsecret == other.revealsecret and
+            self.received_secret_request == other.received_secret_request
         )
 
     def __ne__(self, other):
@@ -139,6 +143,7 @@ def to_dict(self) -> typing.Dict[str, typing.Any]:
             'channel_identifier': self.channel_identifier,
             'transfer': self.transfer,
             'revealsecret': self.revealsecret,
+            'received_secret_request': self.received_secret_request,
         }
 
         return result
@@ -150,6 +155,7 @@ def from_dict(cls, data: typing.Dict[str, typing.Any]) -> 'InitiatorTransferStat
             channel_identifier=data['channel_identifier'],
             transfer=data['transfer'],
             revealsecret=data['revealsecret'],
+            received_secret_request=data['received_secret_request'],
         )
 
         return restored