Skip to content

Add mediation fees for internal routing and maximum fee limit #4760

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.

Sign up for GitHub

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

Merged
LefterisJP merged 17 commits into from
Sep 10, 2019
Merged

Add mediation fees for internal routing and maximum fee limit #4760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d32eb2b
Add a fee for internal routing that is a percentage to the amount
LefterisJP Sep 5, 2019
2430e3c
Add a check for the maximum amount limit of transfer + fees
LefterisJP Sep 5, 2019
fc2a01a
Use UNIT_TRANSFER_AMOUNT in tests instead of magic constants
LefterisJP Sep 5, 2019
446018f
Set amount/fees in unit tests that agree with our defaults
LefterisJP Sep 5, 2019
67d1022
Add a test for the maximum mediation fee limit
LefterisJP Sep 5, 2019
c9c6bb0
Adjust test_mediated_transfer for the fee changes
LefterisJP Sep 6, 2019
43f4bbf
Adjust test_restapi for the fee changes
LefterisJP Sep 6, 2019
07b392e
Adjust test_refund_transfer for fee changes
LefterisJP Sep 6, 2019
bd5899f
Adjust test_regression.py for the fee changes
LefterisJP Sep 6, 2019
eec83d3
Adjust test_pending_transfers_endpoint for fees
LefterisJP Sep 6, 2019
cd837e9
Adjust test_settlement after fee changes
LefterisJP Sep 6, 2019
8a7cec9
Disable max mediation fee checks for test_mediated_transfer_with_fee
LefterisJP Sep 6, 2019
1d0a279
/s/ininitial/initial
LefterisJP Sep 6, 2019
1c299c2
Fixes after rebase
LefterisJP Sep 9, 2019
aff2ab7
PR review fixes for readability
LefterisJP Sep 9, 2019
5ddf099
Add exception with msg at timeout of wait for transfer in tests
LefterisJP Sep 9, 2019
33df00b
Add proper checks for amount/fee ratio in the mediation fee test
LefterisJP Sep 9, 2019
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
8 changes: 6 additions & 2 deletions raiden/routing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from raiden.exceptions import ServiceRequestFailed
from raiden.messages.metadata import RouteMetadata
from raiden.network.pathfinding import PFSConfig, query_paths
from raiden.settings import INTERNAL_ROUTING_DEFAULT_FEE_PERC
from raiden.transfer import channel, views
from raiden.transfer.state import ChainState, ChannelState, RouteState
from raiden.utils.typing import (
Expand Down Expand Up @@ -209,12 +210,15 @@ def get_best_routes_internal(
# The complete route includes the initiator, add it to the beginning
complete_route = [Address(from_address)] + neighbour.route

# https://github.com/raiden-network/raiden/issues/4751
# Internal routing doesn't know about fee so it should set a percentage per hop
estimated_fee = FeeAmount(round(INTERNAL_ROUTING_DEFAULT_FEE_PERC * amount))
Copy link
Copy Markdown
Contributor

@karlb karlb Sep 10, 2019

Choose a reason for hiding this comment

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

We're adding fees here even in case of a direct transfer.

Copy link
Copy Markdown
Contributor

@karlb karlb Sep 10, 2019

Choose a reason for hiding this comment

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

Sorry for not being thorough, yesterday. It also looks like we only apply INTERNAL_ROUTING_DEFAULT_FEE_PERC once. Wasn't the intention of the ticket to do that once per hop?

Copy link
Copy Markdown
Contributor

@karlb karlb Sep 10, 2019

Choose a reason for hiding this comment

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

I'll write a PR that changes these two things. If any of these statements is wrong, let me know!

Copy link
Copy Markdown
Contributor Author

@LefterisJP LefterisJP Sep 10, 2019

Choose a reason for hiding this comment

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

@karlb

We're adding fees here even in case of a direct transfer.

It's correct. I did not notice that.

It also looks like we only apply INTERNAL_ROUTING_DEFAULT_FEE_PERC once. Wasn't the intention of the ticket to do that once per hop?

No, since we are not sure of the hops anyway in internal routing the intention (and it's also written in the issue) was to add a big percentage.


available_routes.append(
RouteState(
route=complete_route,
forward_channel_id=neighbour.channelid,
# Internal routing doesn't know about fees, so set them to 0
estimated_fee=FeeAmount(0),
estimated_fee=estimated_fee,
)
)

Expand Down
2 changes: 2 additions & 0 deletions raiden/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
DEFAULT_MEDIATION_PROPORTIONAL_FEE = ProportionalFeeAmount(0)
DEFAULT_MEDIATION_PROPORTIONAL_IMBALANCE_FEE = ProportionalFeeAmount(0)
DEFAULT_MEDIATION_FEE_MARGIN: float = 0.05
INTERNAL_ROUTING_DEFAULT_FEE_PERC: float = 0.02
MAX_MEDIATION_FEE_PERC: float = 0.2

ORACLE_BLOCKNUMBER_DRIFT_TOLERANCE = 3
ETHERSCAN_API = "https://{network}.etherscan.io/api?module=proxy&action={action}"
Expand Down
20 changes: 11 additions & 9 deletions raiden/tests/integration/api/test_restapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from raiden.tests.utils.network import CHAIN
from raiden.tests.utils.protocol import WaitForMessage
from raiden.tests.utils.smartcontracts import deploy_contract_web3
from raiden.tests.utils.transfer import calculate_fee_for_amount
from raiden.transfer import views
from raiden.transfer.state import ChannelState
from raiden.waiting import (
Expand Down Expand Up @@ -840,7 +841,7 @@ def test_api_payments_target_error(api_server_test_instance, raiden_network, tok
@pytest.mark.parametrize("number_of_nodes", [2])
def test_api_payments(api_server_test_instance, raiden_network, token_addresses):
_, app1 = raiden_network
amount = 200
amount = 100
identifier = 42
token_address = token_addresses[0]
target_address = app1.raiden.address
Expand Down Expand Up @@ -985,7 +986,7 @@ def test_api_payments_with_secret_no_hash(
api_server_test_instance, raiden_network, token_addresses
):
_, app1 = raiden_network
amount = 200
amount = 100
identifier = 42
token_address = token_addresses[0]
target_address = app1.raiden.address
Expand Down Expand Up @@ -1058,7 +1059,7 @@ def test_api_payments_with_secret_and_hash(
api_server_test_instance, raiden_network, token_addresses
):
_, app1 = raiden_network
amount = 200
amount = 100
identifier = 42
token_address = token_addresses[0]
target_address = app1.raiden.address
Expand Down Expand Up @@ -1556,7 +1557,7 @@ def test_api_deposit_limit(api_server_test_instance, token_addresses, reveal_tim
@pytest.mark.parametrize("number_of_nodes", [3])
def test_payment_events_endpoints(api_server_test_instance, raiden_network, token_addresses):
app0, app1, app2 = raiden_network
amount1 = 200
amount1 = 10
identifier1 = 42
secret1, secrethash1 = factories.make_secret_with_hash()
token_address = token_addresses[0]
Expand All @@ -1582,7 +1583,7 @@ def test_payment_events_endpoints(api_server_test_instance, raiden_network, toke

# app0 is sending some tokens to target 2
identifier2 = 43
amount2 = 123
amount2 = 10
secret2, secrethash2 = factories.make_secret_with_hash()
request = grequests.post(
api_url_for(
Expand Down Expand Up @@ -1884,7 +1885,7 @@ def test_payment_events_endpoints(api_server_test_instance, raiden_network, toke
@pytest.mark.parametrize("number_of_nodes", [2])
def test_channel_events_raiden(api_server_test_instance, raiden_network, token_addresses):
_, app1 = raiden_network
amount = 200
amount = 100
identifier = 42
token_address = token_addresses[0]
target_address = app1.raiden.address
Expand All @@ -1906,7 +1907,8 @@ def test_channel_events_raiden(api_server_test_instance, raiden_network, token_a
@pytest.mark.parametrize("channels_per_node", [CHAIN])
def test_pending_transfers_endpoint(raiden_network, token_addresses):
initiator, mediator, target = raiden_network
amount = 200
amount = 150
amount_with_fee = amount + calculate_fee_for_amount(amount)
identifier = 42

token_address = token_addresses[0]
Expand Down Expand Up @@ -1944,7 +1946,7 @@ def test_pending_transfers_endpoint(raiden_network, token_addresses):
)

transfer_arrived = target_wait.wait_for_message(LockedTransfer, {"payment_identifier": 42})
transfer_arrived.wait()
transfer_arrived.wait(timeout=30.0)

for server in (initiator_server, mediator_server, target_server):
request = grequests.get(api_url_for(server, "pending_transfers_resource"))
Expand All @@ -1953,7 +1955,7 @@ def test_pending_transfers_endpoint(raiden_network, token_addresses):
content = json.loads(response.content)
assert len(content) == 1
assert content[0]["payment_identifier"] == str(identifier)
assert content[0]["locked_amount"] == str(amount)
assert content[0]["locked_amount"] == str(amount_with_fee)
assert content[0]["token_address"] == to_checksum_address(token_address)
assert content[0]["token_network_address"] == to_checksum_address(token_network_address)

Expand Down
12 changes: 9 additions & 3 deletions raiden/tests/integration/long_running/test_settlement.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@
from raiden.tests.utils.events import raiden_state_changes_search_for_item, search_for_item
from raiden.tests.utils.network import CHAIN
from raiden.tests.utils.protocol import WaitForMessage
from raiden.tests.utils.transfer import assert_synced_channel_state, get_channelstate, transfer
from raiden.tests.utils.transfer import (
assert_synced_channel_state,
calculate_fee_for_amount,
get_channelstate,
transfer,
)
from raiden.transfer import channel, views
from raiden.transfer.events import SendWithdrawConfirmation
from raiden.transfer.identifiers import CanonicalIdentifier
Expand Down Expand Up @@ -655,6 +660,7 @@ def run_test_settled_lock(token_addresses, raiden_network, deposit):
registry_address = app0.raiden.default_registry.address
token_address = token_addresses[0]
amount = PaymentAmount(30)
amount_with_fee = amount + calculate_fee_for_amount(amount)
token_network_address = views.get_token_network_address_by_token_address(
views.state_from_app(app0), app0.raiden.default_registry.address, token_address
)
Expand Down Expand Up @@ -727,8 +733,8 @@ def run_test_settled_lock(token_addresses, raiden_network, deposit):
given_block_identifier=current_block,
)

expected_balance0 = initial_balance0 + deposit0 - amount * 2
expected_balance1 = initial_balance1 + deposit1 + amount * 2
expected_balance0 = initial_balance0 + deposit0 - amount_with_fee * 2
expected_balance1 = initial_balance1 + deposit1 + amount_with_fee * 2

assert token_proxy.balance_of(address0) == expected_balance0
assert token_proxy.balance_of(address1) == expected_balance1
Expand Down
12 changes: 9 additions & 3 deletions raiden/tests/integration/test_regression.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@
)
from raiden.tests.utils.factories import UNIT_CHAIN_ID
from raiden.tests.utils.network import payment_channel_open_and_deposit
from raiden.tests.utils.transfer import get_channelstate, transfer, watch_for_unlock_failures
from raiden.tests.utils.transfer import (
calculate_amount_to_drain_channel,
get_channelstate,
transfer,
watch_for_unlock_failures,
)
from raiden.transfer import views
from raiden.transfer.mediated_transfer.events import EventRouteFailed, SendSecretReveal
from raiden.transfer.mediated_transfer.state_change import ReceiveTransferCancelRoute
Expand Down Expand Up @@ -285,12 +290,13 @@ def run_regression_payment_complete_after_refund_to_the_initiator(
app_channels = [(app0, app1), (app1, app2), (app0, app3), (app3, app4), (app4, app2)]
open_and_wait_for_channels(app_channels, registry_address, token, deposit, settle_timeout)

exhaust_amount = calculate_amount_to_drain_channel(deposit)
# Use all deposit from app1->app2 to force a refund
transfer(
initiator_app=app1,
target_app=app2,
token_address=token,
amount=deposit,
amount=exhaust_amount,
identifier=PaymentID(1),
)

Expand All @@ -299,7 +305,7 @@ def run_regression_payment_complete_after_refund_to_the_initiator(
initiator_app=app0,
target_app=app2,
token_address=token,
amount=deposit,
amount=exhaust_amount,
identifier=PaymentID(2),
timeout=20,
)
Expand Down
25 changes: 20 additions & 5 deletions raiden/tests/integration/transfer/test_mediatedtransfer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
assert_succeeding_transfer_invariants,
assert_synced_channel_state,
block_timeout_for_transfer_by_secrethash,
calculate_amount_to_drain_channel,
transfer,
transfer_and_assert_path,
wait_assert,
Expand Down Expand Up @@ -195,10 +196,12 @@ def run_test_mediated_transfer_with_entire_deposit(
chain_state, token_network_registry_address, token_address
)

amount = calculate_amount_to_drain_channel(deposit)

secrethash = transfer_and_assert_path(
path=raiden_network,
token_address=token_address,
amount=deposit,
amount=amount,
identifier=1,
timeout=network_wait * number_of_nodes,
)
Expand All @@ -207,7 +210,7 @@ def run_test_mediated_transfer_with_entire_deposit(
transfer_and_assert_path(
path=reverse_path,
token_address=token_address,
amount=deposit * 2,
amount=amount * 2,
identifier=2,
timeout=network_wait * number_of_nodes,
)
Expand Down Expand Up @@ -442,7 +445,7 @@ def run_test_mediated_transfer_with_node_consuming_more_than_allocated_fee(
chain_state, token_network_registry_address, token_address
)
fee = FeeAmount(5)
amount = PaymentAmount(10)
amount = PaymentAmount(100)

app1_app2_channel_state = views.get_channelstate_by_token_network_and_partner(
chain_state=views.state_from_raiden(app1.raiden),
Expand Down Expand Up @@ -570,7 +573,13 @@ def assert_balances(expected_transferred_amounts=List[int]):

fee_without_margin = FeeAmount(20)
fee = round(fee_without_margin * (1 + DEFAULT_MEDIATION_FEE_MARGIN))
amount = PaymentAmount(10)
amount = PaymentAmount(35)
msg = (
"The chosen values will result in less than the amount reaching "
"the target after the 3rd hop"
)
amount_at_end = amount + fee - (amount + fee) // 5 - (amount + fee - (amount + fee) // 5) // 5
assert amount_at_end >= amount, msg
cases = [
# The fee is added by the initiator, but no mediator deducts fees. As a
# result, the target receives the fee.
Expand Down Expand Up @@ -646,7 +655,13 @@ def assert_balances(expected_transferred_amounts=List[int]):
for i, fee_schedule in enumerate(case.get("incoming_fee_schedules", [])):
if fee_schedule:
set_fee_schedule(apps[i + 1], apps[i], fee_schedule)
with patch("raiden.routing.get_best_routes_internal", get_best_routes_with_fees):

route_patch = patch("raiden.routing.get_best_routes_internal", get_best_routes_with_fees)
disable_max_mediation_fee_patch = patch(
Comment thread
karlb marked this conversation as resolved.
"raiden.transfer.mediated_transfer.initiator.MAX_MEDIATION_FEE_PERC", new=10000
)

with route_patch, disable_max_mediation_fee_patch:
transfer_and_assert_path(
path=raiden_network, token_address=token_address, amount=amount, identifier=2
)
Expand Down
Loading