Skip to content

Fix explanation for settlement overflow#119

Merged
loredanacirstea merged 4 commits into
raiden-network:masterfrom
loredanacirstea:settlement-spec-fix
Aug 31, 2018
Merged

Fix explanation for settlement overflow#119
loredanacirstea merged 4 commits into
raiden-network:masterfrom
loredanacirstea:settlement-spec-fix

Conversation

@loredanacirstea
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread smart_contracts.rst Outdated
(7 R) T2 + L2 - T1 - L1 <= D2

``T2 + L2 - T1 - L1`` is the netted total transferred amount from ``P2`` to ``P1``. This amount cannot be bigger than ``P2``'s deposit -> a participant cannot transfer more tokens than what he has in the channel, during the lifecycle of a channel.
This ``MUST`` be ensured by the Raiden client.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So, I think (7 R) T2 + L2 - T1 - L1 <= D2 is actually what the Raiden client MUST enforce.
However, the Raiden client only enforces (5 R) AB1 = D1 - W1 + T2 - T1 - L1; AB1 >= 0, AB1 <= TAD as far as I know.
So, we might want to prove that (5 R) ensures (7 R)

Comment thread smart_contracts.rst Outdated

(7 R) T2 + L2 - T1 - L1 <= D2

``T2 + L2 - T1 - L1`` is the netted total transferred amount from ``P2`` to ``P1``. This amount cannot be bigger than ``P2``'s deposit -> a participant cannot transfer more tokens than what he has in the channel, during the lifecycle of a channel.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about mentioning the lower bound? This value can be negative but not smaller than D1. (This is a corollary that must be enforce by P2)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct, added.

Comment thread smart_contracts.rst Outdated
::

(7 R) T2 + L2 - T1 - L1 <= D2
(7 R) -D1 <= T2 + L2 - T1 - L1 <= D2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

btw, this is missing the withdraws:

-(D1 - W1) <= T2 + L2 - T1 - L1 <= D2 - W2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? T + L are the total transferred amounts. The netted total transferred amounts should be bounded by the channel deposit.
The withdraws are then taken from the available balance, but we do not subtract them from the total transferred amounts.

Copy link
Copy Markdown
Contributor

@hackaugusto hackaugusto Aug 30, 2018

Choose a reason for hiding this comment

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

The netted total transferred amounts should be bounded by the channel deposit.

Nope. You cannot have a net value that is larger than your available deposit on chain, consider:

P2 Deposit 10
P2 Withdraws 10
If P1 enforce just T2 + L2 - T1 - L1 <= D2 it means P2 can make transfer up to 10 tokens, which is wrong.

Additionally, since you're talking about the client, D2 - W2 is not sufficient, since W2 is defined as the current on-chain withdraw. The client must enforce this against the off-chain withdraw, which is different from the on-chain value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think you did not consider P1's transferred amounts. P2 can transfer up to 10 tokens if P1 owes him 10 tokens.

E.g.:

D1 = 10
W1 = 0
T1 = 4
L1 = 0


D2 = 10
W2 = 5
T2 = 10
L2 = 0
T2 + L2 - T1 - L1 = 10 + 0 - 4 - 0 = 6
# The following is true:
# - D1 <= 6 <= D2
# - 10 <= 6 <= 10

# The following is not true:
# -(D1 - W1) <= 6 <= D2 - W2
# - (10 - 0) <= 6 <= 10 - 5
# -10 <= 6 <= 5
# 6 <= 5 not true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, I think you are right.
Calculating the available balances from above and I see that they are not valid balance proofs:

AB1 = D1 - W1 + T2 - T1 - L1 = 10 - 0 + 10 - 4 - 0 = 16
AB2 =  D2 - W2 + T1 - T2 - L2 = 10 - 5 + 4 - 10 - 0 = -1  # must always be > 0

So, I will use -(D1 - W1) <= T2 + L2 - T1 - L1 <= D2 - W2

Comment thread smart_contracts.rst
``T2 + L2 - T1 - L1`` is the netted total transferred amount from ``P2`` to ``P1``. This amount cannot be bigger than ``P2``'s **available** deposit. We enforce that a participant cannot transfer more tokens than what he has in the channel, during the lifecycle of a channel.
This amount cannot be smaller than the negative value of ``P1``'s **available** deposit ``- (D1 - W1)``. This can also be deducted from the corresponding ``T1 + L1 - T2 - L2 <= D1 - W1``
The Raiden client ``MUST`` ensure this. However, it must use up-to-date values for ``D2`` and ``W2`` (e.g. Raiden node might have sent an on-chain transaction to withdraw tokens; this is not mined yet, therefore it does not reflect in the contract yet. The Raiden client will use the off-chain ``W2`` value.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@hackaugusto , I added the off-chain values mention.

@loredanacirstea loredanacirstea merged commit 48418f0 into raiden-network:master Aug 31, 2018
@loredanacirstea loredanacirstea deleted the settlement-spec-fix branch August 31, 2018 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants