-
Notifications
You must be signed in to change notification settings - Fork 13
Introduction of AttemptClass and PaymentClass (Issue #20) #24
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
Changes from all commits
519185f
cc1e862
4832253
afa6a71
4ecadee
a131eaf
0b0e868
8dffa28
8df2c57
c03fe39
7170150
16bad26
8727759
f6dad46
40967ff
5903cd2
c1c61a0
54e905c
cf25aa0
cf9c4f1
9d56f65
3e69225
bf14533
97d01cb
c8bfeb8
fd232b5
0730a91
f0bda8b
29990a2
081d756
c404105
3c850fe
111a691
c320189
de25111
40fb9be
54ccdc3
a40e61a
1a05a21
d494c0d
fc1e8d5
8541a28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| .DS_Store | ||
| __pycache__ | ||
| .idea | ||
| pickhardtpayments/pickhardtpayments.egg-info | ||
| pickhardtpayments/__pycache__ | ||
|
|
||
| # not including large file with Channel Graph | ||
| *.json | ||
| examples/.json | ||
|
|
||
| pickhardt_pay.log |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # Changelog | ||
| All notable changes to this project will be documented in this file. | ||
|
|
||
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) | ||
| and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| <!-- | ||
| TODO: Insert version codename, and username of the contributor that named the release. | ||
| --> | ||
| ## [Unreleased] | ||
|
|
||
| ## [0.1.0] - 2022-06-21 | ||
| ### Added | ||
| - introduction of an Attempt Class and a Payment Class ([#28]) | ||
| - introduction of an AttemptStatus to describe the state of the Attempt ([#28]) | ||
| - settle_payment in OracleLightingNetwork is added ([#28]) | ||
| - logging added in SyncSimulatedPaymentSession ([#28]) | ||
|
|
||
| ### Changed | ||
| - calculation of fees and probabilities is moved from SyncSimulatedPaymentSession to Attempt class ([#28]) | ||
|
|
||
| ### Deprecated | ||
|
|
||
| ### Removed | ||
|
|
||
| ### Fixed | ||
|
|
||
| ### EXPERIMENTAL |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| from enum import Enum | ||
|
|
||
| from Channel import Channel | ||
| from pickhardtpayments import UncertaintyChannel | ||
|
|
||
|
|
||
| class AttemptStatus(Enum): | ||
| PLANNED = 1 | ||
| INFLIGHT = 2 | ||
| ARRIVED = 4 | ||
| FAILED = 8 | ||
| SETTLED = 16 | ||
|
|
||
|
|
||
| class Attempt: | ||
| """ | ||
| An Attempt describes a path (a list of channels) of an amount from sender to receiver. | ||
|
|
||
| # TODO Describe life cycle of an Attempt | ||
|
|
||
| When sending an amount of sats from sender to receiver, a payment is usually split up and sent across | ||
| several paths, to increase the probability of being successfully delivered. Each of these paths is referred to as an | ||
| Attempt. | ||
| An Attempt consists of a list of Channels (class:Channel) and the amount in sats to be sent through this path. | ||
| When an Attempt is instantiated, the given amount is allocated to the inflight amount in the channels of the | ||
| path and the AttemptStatus is set to PLANNED. | ||
|
|
||
| :param path: a list of Channel objects from sender to receiver | ||
| :type path: list[Channel] | ||
| :param amount: the amount to be transferred from source to destination | ||
| :type amount: int | ||
| """ | ||
|
|
||
| def __init__(self, path: list[UncertaintyChannel], amount: int = 0): | ||
| """Constructor method | ||
| """ | ||
| if amount >= 0: | ||
| self._amount = amount | ||
| else: | ||
| raise ValueError("amount for payment attempts needs to be positive") | ||
|
|
||
| i = 1 | ||
| valid_path = True | ||
| while i < len(path): | ||
| valid_path = valid_path and (path[i - 1].dest == path[i].src) | ||
| i += 1 | ||
| if valid_path: | ||
| self._path = path | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is the difference to self._paths from line 32?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is the real one ;) |
||
|
|
||
| channel: UncertaintyChannel | ||
| self._routing_fee = 0 | ||
| self._probability = 1 | ||
| for channel in path: | ||
| self._routing_fee += channel.routing_cost_msat(amount) | ||
| self._probability *= channel.success_probability(amount) | ||
| # When Attempt is created, all amounts are set inflight. Needs to be updated with AttemptStatus change! | ||
| # This is to correctly compute conditional probabilities of non-disjoint paths in the same set of paths | ||
| # channel.in_flight(amount) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So far so good and makes sense |
||
| channel.allocate_amount(amount) | ||
|
sebulino marked this conversation as resolved.
|
||
| self._status = AttemptStatus.PLANNED | ||
|
|
||
| def __str__(self): | ||
| description = "Path with {} channels to deliver {} sats and status {}.".format(len(self._path), | ||
| self._amount, self._status.name) | ||
| if self._routing_fee and self._routing_fee > 0: | ||
| description += "\nsuccess probability of {:6.2f}% , fee of {:8.3f} sat and a ppm of {:5} ".format( | ||
| self._probability * 100, self._routing_fee/1000, int(self._routing_fee * 1000 / self._amount)) | ||
| return description | ||
|
|
||
| @property | ||
| def path(self) -> list[Channel]: | ||
| """Returns the path of the attempt. | ||
|
|
||
| :return: the list of Channels that the path consists of | ||
| :rtype: list[Channel] | ||
| """ | ||
| return self._path | ||
|
|
||
| @property | ||
| def amount(self) -> int: | ||
| """Returns the amount of the attempt. | ||
|
|
||
| :return: the amount that was tried to send in this Attempt | ||
| :rtype: int | ||
| """ | ||
| return self._amount | ||
|
|
||
| @property | ||
| def status(self) -> AttemptStatus: | ||
| """Returns the status of the attempt. | ||
|
|
||
| :return: returns the state of the attempt | ||
| :rtype: AttemptStatus | ||
| """ | ||
| return self._status | ||
|
|
||
| @status.setter | ||
| def status(self, value: AttemptStatus): | ||
| """Sets the status of the attempt. | ||
|
|
||
| A flag to describe if the path failed, succeeded or was used for settlement (see enum SettlementStatus) | ||
|
|
||
| :param value: Current state of the Attempt | ||
| :type value: AttemptStatus | ||
| """ | ||
| if not self._status == value: | ||
| # remove allocated amounts when Attempt status changes from PLANNED | ||
| if self._status == AttemptStatus.PLANNED and not value == AttemptStatus.INFLIGHT: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not inflight? I am wondering if we should have a setter API or if we can only inc status by 1. That being said I see the idea. While i think it is -given the design of the code- the only reasonable option i think it demonstrates that I messed something up. (It feels really wrong to do the amounts tweeking here) |
||
| for channel in self._path: | ||
| channel.allocate_amount(-self._amount) | ||
|
|
||
| if self._status == AttemptStatus.INFLIGHT and value == AttemptStatus.ARRIVED: | ||
| # TODO write amount from inflight to min_liquidity/max_liquidity | ||
| # for channel in self._path: | ||
| # channel.allocate_amount(-self._amount) | ||
| pass | ||
|
|
||
| self._status = value | ||
|
|
||
| @property | ||
| def routing_fee(self) -> int: | ||
| """Returns the accrued routing fee in msat requested for this path. | ||
|
|
||
| :return: accrued routing fees for this attempt in msat | ||
| :rtype: int | ||
| """ | ||
| return self._routing_fee | ||
|
|
||
| @property | ||
| def probability(self) -> float: | ||
| """Returns estimated success probability before the attempt | ||
|
|
||
| :return: estimated success probability before the attempt | ||
| :rtype: float | ||
| """ | ||
| return self._probability | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not entirely sure as the attempt class does not know the uncertainty network but we could compute this on the fly for the path (as the path contains such information)
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is btw a strong attempt against it as non disjoint paths will change probability computation.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, this is to collect statistical a priori information:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bias in probabilities in UncertaintyGraph?When collecting possible paths in a round/"payment loop", all path candidates are assumed to be sent and settled successfully.
Or am I reading it wrong? Ist this something we can live with?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This indeed the really messy book keeping that I introduced. I am happy to find a better solution to this! I need to allocate the amounts for planned onions to have probability correction of the next path correct. (Ask me for an example if you need one!) I remove all of the allocated HTLCs then before I do the actual sending / probing. It is really the most ugly part of my code :(
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just started to look at the code. I think this is btw exactly the difference between |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| from typing import List | ||
|
|
||
| from .ChannelGraph import ChannelGraph | ||
| from .OracleChannel import OracleChannel | ||
| import networkx as nx | ||
|
|
@@ -13,7 +15,7 @@ def __init__(self, channel_graph: ChannelGraph): | |
| for src, dest, short_channel_id, channel in channel_graph.network.edges(data="channel", keys=True): | ||
| oracle_channel = None | ||
|
|
||
| # If Channel in oposite direction already exists with liquidity information match the channel | ||
| # If Channel in opposite direction already exists with liquidity information match the channel | ||
| if self._network.has_edge(dest, src): | ||
| if short_channel_id in self._network[dest][src]: | ||
| capacity = channel.capacity | ||
|
|
@@ -35,14 +37,18 @@ def network(self): | |
| return self._network | ||
|
|
||
| def send_onion(self, path, amt): | ||
| """ | ||
|
|
||
| :rtype: object | ||
| """ | ||
| for channel in path: | ||
| oracle_channel = self.get_channel( | ||
| channel.src, channel.dest, channel.short_channel_id) | ||
| success_of_probe = oracle_channel.can_forward( | ||
| channel.in_flight+amt) | ||
| channel.in_flight + amt) | ||
| # print(channel,amt,success_of_probe) | ||
| channel.update_knowledge(amt, success_of_probe) | ||
| if success_of_probe == False: | ||
| if not success_of_probe: | ||
| return False, channel | ||
| return True, None | ||
|
|
||
|
|
@@ -55,7 +61,7 @@ def theoretical_maximum_payable_amount(self, source: str, destination: str, base | |
| """ | ||
| test_network = nx.DiGraph() | ||
| for src, dest, channel in self.network.edges(data="channel"): | ||
| #liqudity = 0 | ||
| # liquidity = 0 | ||
| # for channel in channels: | ||
| if channel.base_fee > base_fee: | ||
| continue | ||
|
|
@@ -71,3 +77,23 @@ def theoretical_maximum_payable_amount(self, source: str, destination: str, base | |
|
|
||
| mincut, _ = nx.minimum_cut(test_network, source, destination) | ||
| return mincut | ||
|
|
||
| def settle_payment(self, path: List[OracleChannel], payment_amount: int): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that is from the other PR #18 ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes |
||
| """ | ||
| receives a List of channels and payment amount and adjusts the balances of the channels along the path. | ||
|
|
||
| settle_payment should only be called after all send_onions for a payment terminated successfully! | ||
| # TODO testing | ||
| """ | ||
| for channel in path: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if the path is not connected? Maybe we really should have a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i changed it so that path from an Attempt is passed to settle_payment. At initialisation of the Attempt with the path, the connectedness is checked. |
||
| settlement_channel = self.get_channel(channel.src, channel.dest, channel.short_channel_id) | ||
| return_settlement_channel = self.get_channel(channel.dest, channel.src, channel.short_channel_id) | ||
| if settlement_channel.actual_liquidity > payment_amount: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you do both in one if? if for some reason the data was not consistant than we don't catch it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if understand - do you refer to the fact that an exception might leave an incomplete rebalancing of the channels and it ends up not being atomic?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am talking abou the fact that you reduce the liquidity in one channel and increase in the other one. But maybe the state of the channels was not connected anymore and now we would get liquidity beyond capacity. (negative won't work because of the if. So I was wondering if we need two sperate ifs or at least both conditions in the same if) |
||
| # decrease channel balance in sending channel by amount | ||
| settlement_channel.actual_liquidity = settlement_channel.actual_liquidity - payment_amount | ||
| # increase channel balance in the other direction by amount | ||
| return_settlement_channel.actual_liquidity = return_settlement_channel.actual_liquidity + payment_amount | ||
| else: | ||
| raise Exception("""Channel liquidity on Channel {} is lower than payment amount. | ||
| \nPayment cannot settle.""".format(channel.short_channel_id)) | ||
| return 0 | ||
Uh oh!
There was an error while loading. Please reload this page.