Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process encrypted secret by the target #7252

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions raiden/message_handler.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
from math import inf

import structlog
from eth_utils import to_hex
from gevent import joinall
from gevent.pool import Pool

from raiden.constants import ABSENT_SECRET, BLOCK_ID_LATEST
from raiden.exceptions import ServiceRequestFailed
from raiden.exceptions import InvalidSecret, ServiceRequestFailed
from raiden.messages.abstract import Message
from raiden.messages.decode import balanceproof_from_envelope, lockedtransfersigned_from_message
from raiden.messages.synchronization import Delivered, Processed
Expand Down Expand Up @@ -40,7 +42,9 @@
ReceiveWithdrawExpired,
ReceiveWithdrawRequest,
)
from raiden.transfer.utils import decrypt_secret
from raiden.transfer.views import TransferRole
from raiden.utils.formatting import to_checksum_address
from raiden.utils.transfers import random_secret
from raiden.utils.typing import (
TYPE_CHECKING,
Expand All @@ -50,6 +54,7 @@
Set,
TargetAddress,
Tuple,
Union,
)

if TYPE_CHECKING:
Expand Down Expand Up @@ -103,18 +108,21 @@ def on_messages(self, raiden: "RaidenService", messages: List[Message]) -> None:
# (an asynchronous network is assumed) This reduces latency when a
# balance proof is considered invalid because of a race with the
# blockchain view of each node.
def by_canonical_identifier(state_change: StateChange) -> Tuple[int, int]:
def by_canonical_identifier(
state_change: StateChange,
) -> Union[Tuple[int, int], Tuple[float, float]]:
if isinstance(state_change, BalanceProofStateChange):
balance_proof = state_change.balance_proof
return (
balance_proof.canonical_identifier.channel_identifier,
balance_proof.nonce,
)

elif isinstance(state_change, ReceiveSecretReveal):
# ReceiveSecretReveal depends on other state changes happening first.
return inf, inf
return 0, 0

all_state_changes.sort(key=by_canonical_identifier)

raiden.handle_and_track_state_changes(all_state_changes)

@staticmethod
Expand Down Expand Up @@ -338,6 +346,24 @@ def handle_message_lockedtransfer(
sender = from_transfer.balance_proof.sender

if message.target == TargetAddress(raiden.address):
encrypted_secret = message.metadata.encrypted_secret
if encrypted_secret is not None:
try:
secret = decrypt_secret(encrypted_secret, raiden.rpc_client.privkey)
log.info("Using encrypted secret", sender=to_checksum_address(sender))
return [
ActionInitTarget(
from_hop=from_hop,
transfer=from_transfer,
balance_proof=balance_proof,
sender=sender,
received_valid_secret=True,
),
ReceiveSecretReveal(secret=secret, sender=message.sender),
]
except InvalidSecret:
sender_addr = to_checksum_address(sender)
log.error("Ignoring invalid encrypted secret", sender=sender_addr)
return [
ActionInitTarget(
from_hop=from_hop,
Expand Down
7 changes: 6 additions & 1 deletion raiden/tests/integration/api/rest/test_rest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import datetime
import json
from http import HTTPStatus
from unittest.mock import Mock, patch

import gevent
import grequests
Expand All @@ -11,6 +12,7 @@
from raiden.api.python import RaidenAPI
from raiden.api.rest import APIServer
from raiden.constants import BLOCK_ID_LATEST, Environment
from raiden.exceptions import InvalidSecret
from raiden.messages.transfers import LockedTransfer, Unlock
from raiden.raiden_service import RaidenService
from raiden.settings import (
Expand Down Expand Up @@ -937,7 +939,10 @@ def test_channel_events_raiden(
@pytest.mark.parametrize("number_of_nodes", [3])
@pytest.mark.parametrize("channels_per_node", [CHAIN])
@pytest.mark.parametrize("enable_rest_api", [True])
def test_pending_transfers_endpoint(raiden_network: List[RaidenService], token_addresses):
@patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually use pytest's monkeypatch fixture instead of patch in the test suite (although not always...).
The advantage is that the monkeypatch fixture is integrated with pytest and also defaults to raising an exception if the patch target doesn't exist (e.g. after a refactoring) which is not the case with patch.

I think it would be nicer to put this into a fixture and / or test helper (e.g. disable_encrypted_secret) so as not having to repeat this patch boilerplate on many tests (which would need to be changed if ever the decrypt_secret function was moved or otherwise refactored).

Also this only tests the branch where the included secret is invalid, not the one where none is provided in the first place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #7287 for this

def test_pending_transfers_endpoint(
decrypt_patch: Mock, raiden_network: List[RaidenService], token_addresses
):
initiator, mediator, target = raiden_network
token_address = token_addresses[0]
token_network_address = views.get_token_network_address_by_token_address(
Expand Down
24 changes: 15 additions & 9 deletions raiden/tests/integration/api/test_pythonapi.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest.mock import patch

import gevent
import pytest
from eth_utils import to_canonical_address
Expand All @@ -12,6 +14,7 @@
InsufficientEth,
InsufficientGasReserve,
InvalidBinaryAddress,
InvalidSecret,
InvalidSettleTimeout,
RaidenRecoverableError,
SamePeerAddress,
Expand Down Expand Up @@ -353,15 +356,18 @@ def test_payment_timing_out_if_partner_does_not_respond( # pylint: disable=unus
assert isinstance(app1.raiden_event_handler, HoldRaidenEventHandler), msg
app1.raiden_event_handler.hold(SendSecretRequest, {})

greenlet = gevent.spawn(
RaidenAPI(app0).transfer_and_wait,
app0.default_registry.address,
token_address,
1,
target=app1.address,
)
waiting.wait_for_block(app0, app1.get_block_number() + 2 * reveal_timeout + 1, retry_timeout)
greenlet.join(timeout=5)
with patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret):
greenlet = gevent.spawn(
RaidenAPI(app0).transfer_and_wait,
app0.default_registry.address,
token_address,
1,
target=app1.address,
)
waiting.wait_for_block(
app0, app1.get_block_number() + 2 * reveal_timeout + 1, retry_timeout
)
greenlet.join(timeout=5)
assert not greenlet.value


Expand Down
133 changes: 70 additions & 63 deletions raiden/tests/integration/long_running/test_settlement.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import random
from unittest.mock import patch

import gevent
import pytest
Expand All @@ -8,7 +9,7 @@
from raiden import waiting
from raiden.api.python import RaidenAPI
from raiden.constants import BLOCK_ID_LATEST, EMPTY_SIGNATURE, UINT64_MAX
from raiden.exceptions import RaidenUnrecoverableError
from raiden.exceptions import InvalidSecret, RaidenUnrecoverableError
from raiden.messages.transfers import LockedTransfer, LockExpired, RevealSecret, Unlock
from raiden.messages.withdraw import WithdrawExpired
from raiden.raiden_service import RaidenService
Expand Down Expand Up @@ -241,15 +242,16 @@ def test_lock_expiry(
LockExpired, {"secrethash": transfer_1_secrethash}
)

alice_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=alice_to_bob_amount,
target=target,
identifier=identifier,
secret=transfer_1_secret,
route_states=[create_route_state_for_route([alice_app, bob_app], token_address)],
)
transfer1_received.wait()
with patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret):
alice_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=alice_to_bob_amount,
target=target,
identifier=identifier,
secret=transfer_1_secret,
route_states=[create_route_state_for_route([alice_app, bob_app], token_address)],
)
transfer1_received.wait()

alice_bob_channel_state = get_channelstate(alice_app, bob_app, token_network_address)
lock = channel.get_lock(alice_bob_channel_state.our_state, transfer_1_secrethash)
Expand Down Expand Up @@ -292,15 +294,16 @@ def test_lock_expiry(

hold_event_handler.hold_secretrequest_for(secrethash=transfer_2_secrethash)

alice_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=alice_to_bob_amount,
target=target,
identifier=identifier,
secret=transfer_2_secret,
route_states=[create_route_state_for_route([alice_app, bob_app], token_address)],
)
transfer2_received.wait()
with patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret):
alice_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=alice_to_bob_amount,
target=target,
identifier=identifier,
secret=transfer_2_secret,
route_states=[create_route_state_for_route([alice_app, bob_app], token_address)],
)
transfer2_received.wait()

# Make sure the other transfer still exists
alice_chain_state = views.state_from_raiden(alice_app)
Expand Down Expand Up @@ -356,16 +359,17 @@ def test_batch_unlock(

secret_request_event = hold_event_handler.hold_secretrequest_for(secrethash=secrethash)

alice_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=PaymentAmount(alice_to_bob_amount),
target=TargetAddress(bob_address),
identifier=PaymentID(identifier),
secret=secret,
route_states=[create_route_state_for_route([alice_app, bob_app], token_address)],
)
with patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret):
alice_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=PaymentAmount(alice_to_bob_amount),
target=TargetAddress(bob_address),
identifier=PaymentID(identifier),
secret=secret,
route_states=[create_route_state_for_route([alice_app, bob_app], token_address)],
)

secret_request_event.get() # wait for the messages to be exchanged
secret_request_event.get() # wait for the messages to be exchanged

alice_bob_channel_state = get_channelstate(alice_app, bob_app, token_network_address)
lock = channel.get_lock(alice_bob_channel_state.our_state, secrethash)
Expand Down Expand Up @@ -699,31 +703,33 @@ def test_settled_lock(

secret_available = hold_event_handler.hold_secretrequest_for(secrethash=secrethash)

app0.mediated_transfer_async(
token_network_address=token_network_address,
amount=amount,
target=target,
identifier=identifier,
secret=secret,
route_states=[create_route_state_for_route([app0, app1], token_address)],
)
with patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret):
app0.mediated_transfer_async(
token_network_address=token_network_address,
amount=amount,
target=target,
identifier=identifier,
secret=secret,
route_states=[create_route_state_for_route([app0, app1], token_address)],
)

secret_available.wait() # wait for the messages to be exchanged
secret_available.wait() # wait for the messages to be exchanged

# Save the pending locks from the pending transfer, used to test the unlock
channelstate_0_1 = get_channelstate(app0, app1, token_network_address)
pending_locks = channelstate_0_1.our_state.pending_locks

hold_event_handler.release_secretrequest_for(app1, secrethash)

transfer(
initiator_app=app0,
target_app=app1,
token_address=token_address,
amount=amount,
identifier=PaymentID(2),
routes=[[app0, app1]],
)
with patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret):
transfer(
initiator_app=app0,
target_app=app1,
token_address=token_address,
amount=amount,
identifier=PaymentID(2),
routes=[[app0, app1]],
)

# The channel state has to be recovered before the settlement, otherwise
# the object is cleared from the node's state.
Expand Down Expand Up @@ -1077,26 +1083,27 @@ def test_batch_unlock_after_restart(
secrethash=bob_transfer_secrethash
)

alice_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=alice_to_bob_amount,
target=TargetAddress(bob_app.address),
identifier=identifier,
secret=alice_transfer_secret,
route_states=[create_route_state_for_route([alice_app, bob_app], token_address)],
)
with patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret):
alice_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=alice_to_bob_amount,
target=TargetAddress(bob_app.address),
identifier=identifier,
secret=alice_transfer_secret,
route_states=[create_route_state_for_route([alice_app, bob_app], token_address)],
)

bob_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=alice_to_bob_amount,
target=TargetAddress(alice_app.address),
identifier=PaymentID(identifier + 1),
secret=bob_transfer_secret,
route_states=[create_route_state_for_route([bob_app, alice_app], token_address)],
)
bob_app.mediated_transfer_async(
token_network_address=token_network_address,
amount=alice_to_bob_amount,
target=TargetAddress(alice_app.address),
identifier=PaymentID(identifier + 1),
secret=bob_transfer_secret,
route_states=[create_route_state_for_route([bob_app, alice_app], token_address)],
)

alice_transfer_hold.wait(timeout=timeout)
bob_transfer_hold.wait(timeout=timeout)
alice_transfer_hold.wait(timeout=timeout)
bob_transfer_hold.wait(timeout=timeout)

alice_bob_channel_state = get_channelstate(alice_app, bob_app, token_network_address)
alice_lock = channel.get_lock(alice_bob_channel_state.our_state, alice_transfer_secrethash)
Expand Down
6 changes: 5 additions & 1 deletion raiden/tests/integration/test_regression_parity.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import math
from typing import cast
from unittest.mock import Mock, patch

import pytest

from raiden import waiting
from raiden.constants import BLOCK_ID_LATEST
from raiden.exceptions import InvalidSecret
from raiden.network.rpc.client import JSONRPCClient
from raiden.raiden_service import RaidenService
from raiden.settings import DEFAULT_NUMBER_OF_BLOCK_CONFIRMATIONS
Expand Down Expand Up @@ -41,7 +43,9 @@
@raise_on_failure
@pytest.mark.parametrize("number_of_nodes", [2])
@pytest.mark.parametrize("blockchain_extra_config", [STATE_PRUNING])
def test_locksroot_loading_during_channel_settle_handling(
@patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret)
def test_locksroot_loading_during_channel_settle_handling( # pylint: disable=unused-argument
decrypt_patch: Mock,
raiden_chain: List[RaidenService],
restart_node: RestartNode,
deploy_client: JSONRPCClient,
Expand Down
7 changes: 6 additions & 1 deletion raiden/tests/integration/test_send_queued_messages.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from unittest.mock import Mock, patch

import gevent
import pytest

from raiden import waiting
from raiden.constants import RoutingMode
from raiden.exceptions import InvalidSecret
from raiden.message_handler import MessageHandler
from raiden.network.transport import MatrixTransport
from raiden.raiden_event_handler import RaidenEventHandler
Expand Down Expand Up @@ -164,7 +167,9 @@ def test_send_queued_messages_after_restart( # pylint: disable=unused-argument
@pytest.mark.parametrize("number_of_nodes", [2])
@pytest.mark.parametrize("channels_per_node", [1])
@pytest.mark.parametrize("number_of_tokens", [1])
def test_payment_statuses_are_restored(
@patch("raiden.message_handler.decrypt_secret", side_effect=InvalidSecret)
def test_payment_statuses_are_restored( # pylint: disable=unused-argument
decrypt_patch: Mock,
raiden_network: List[RaidenService],
restart_node: RestartNode,
token_addresses: List[TokenAddress],
Expand Down
Loading