Get rid of the incremental nonce check in NettingChannelLibrary

[LefterisJP]

Thanks to @stonecoldpat an issue was found with the incremental nonce check in NettingChannelLibrary.

In the case of a courtesy close a malicious Alice can provide a 0 value transfer for her latest transfer as the second argument to the close() function that has the maximum int value for the nonce.

This would render Bob unable to use updateTransfer() since any transaction would throw due to the transfer.nonce already having the maximum value.

The aforementioned check makes no sense anymore and should be removed. Some other requirements (explained below) haveto also be implemented along with the nonce check removal. This is an issue to implement:

TODO

• [x] Remove the nonce check

It currently serves no purpose other than making sure every transfer submitted is newer compared to the previous ones. When you close by providing the counterparty's transfer you are incentivized to provide the latest transfer anyway since it will contain the biggest transferred amount.

• [x] Make sure that updateTransfer() can only be called once
• [x] Get Rid of the courtesy close Even though this is just a courtesy, it makes little sense and should be done away with. Instead some sort of cooperative close (https://github.com/raiden-network/raiden/issues/217) should be implemented in its place

[stonecoldpat]

Thanks for making the ticket!

As a quick solution - the courtesy close should be removed (i.e. the extra parameter provided in close()).

In unidirectional channels (i.e. one way payments) - both Alice and Bob have the incentive to submit the transfer that pays them the most money. So an incremented counter isn't really necessary. Notably, if Alice is responsible for broadcasting the money she gets from Bob (i.e. B->A), and Bob is responsible for broadcasting the money he gets from Alice (i.e. A->B) - then technically the settlement channels requires both participants to sign and agree to it. This is going to be important when you start doing HTLC transactions.

---

Another note: I think the nonce should still be preserved to uniquely identify channels (as opposed to keeping track of messages sent). This is necessary prevent relay attacks for concurrently opened channels. I'll explain below:

Right now (from what I understand) the nonce is greater than the channel's starting block height.

When I say "channel" I mean a single unidirectional channel where Alice -> Bob. So just one-way payments.

i.e. Alice and Bob open a channel at block height 100, the nonce is n=100, and then the channel closes at block height 200.

If Alice and Bob open a new channel at block height 215, then any previous message with the nonce n=100 are not valid.

That works fine. But someday in the future - your smart contract might allow Alice and Bob to open two or more channels at the same time. *(of course; this might apply if the Raiden contract is deployed twice on Ethereum. Two A->B channels can be opened at the same time in this instance. )

For example, at block height 100 - Alice and Bob open two channels C1 and C2. The above nonce approach isn't going to work there as the counter for both channels is 100.

So what I propose (this will need discussion) is that the nonce is uniquely identifiable to the channel itself.... such as

nonce = H(block hash, raiden_contract_address, channel id, deposit_tx_id, reset_counter).

Something like the above should provide a nonce that is unique for each channel. So if the nonce for C1 is 1928d and the nonce for C2 is 17273e - the messages cannot be replayed in either channel.

I mention a 'reset_counter' in the hash as well - because from what I understand currently - when either unidirectional channel exhausts its supply of bitcoins - then you need to call close() and re-open a new pair of unidirectional channels.

Really this reset should be doable off-chain (like in Duplex Micropayment Channels by Decker) - so every time there is a reset the counter is incremented by 1. This `offchain reset' for Ethereum still needs a lot of thought, but it is worth mentioning as I suspect you will want to do something like this in the future.

[LefterisJP]

Hey @stonecoldpat. Thanks again for finding this bug.

This is going to be important when you start doing HTLC transactions.

By HTLC you mean hashed time lock transactions? We already have this functionality in the mediated transfers, where Alice sends transfers to Charlie but they are mediated through BoB since there is no direct channel between the two.

That works fine. But someday in the future - your smart contract might allow Alice and Bob to open two or more channels at the same time.

I can't see any reason why we would ever allow two parties have two different channels open for the same asset.

At the moment in order to avoid conflicts between messages from old channels between the same parties we use nonce ranges as you have probably already seen.

Can you perhaps give an example case where two channels for the same asset would be open between the same parties?

[stonecoldpat]

By HTLC you mean hashed time lock transactions? We already have this functionality in the mediated transfers, where Alice sends transfers to Charlie but they are mediated through BoB since there is no direct channel between the two.

Yeah, I seen this in the contract. I can talk to you privately about that. I wasn't sure if it was fully implemented or not yet. I noticed a few variables (https://github.com/raiden-network/raiden/blob/master/raiden/smart_contracts/NettingChannelLibrary.sol#L22) that don't seem to be doing anything.

At the moment in order to avoid conflicts between messages from old channels between the same parties we use nonce ranges as you have probably already seen.

Can you perhaps give an example case where two channels for the same asset would be open between the same parties?

I edited my comment to take that into account. Yeah, I can't see why you would have two channels in the same Raiden smart contract. But it might be possible that two or more Raiden-style smart contracts are deployed (that is something outside of Raiden's control). In that sense - if the asset is represented in two or more Raiden-style smart contracts, then you might run into a situation where nonce ranges aren't good enough. Of course, this is more hypothetical at the moment - so I thought I would just mention it as something to consider in the future - having contract/channel-specific nonces is starting to look a bit tricky.

[hackaugusto]

Thanks @stonecoldpat for bringing the issue up.

but some other things should be put in its place.

We don't necessarily need an alternative approach, it's sub optimal for a cooperative scenario to go through the dispute on-chain, but it will have the same netted value as a cooperative close (or another method) [edit: but issue #43 is a good idea ].

I think the nonce should still be preserved to uniquely identify channels

The nonces are important both for the smart contract and at the protocol level.

Protocol: We are not assuming a transport protocol with reliable delivery, in fact the current implementation is using UDP as a transport, the nonces are used at the protocol level for error detection, this means that the current raiden implementation requires a sequential nonce for each message.

Smart Contract: Nonces are required to order messages provided by third parties #293 because a third party might be lagging behind, for this we only need a monotonically increasing value, so an implementation using transport with reliable delivery can use nonces increasing at different steps (for whatever reason).

your smart contract might allow Alice and Bob to open two or more channels at the same time. *(of course; this might apply if the Raiden contract is deployed twice on Ethereum. Two A->B channels can be opened at the same time in this instance. )

Yes, indeed this can happen now, we have an issue for replay attacks mitigation #292 , there is nothing preventing someone to deploy a different registry, add the same tokens in it, and open channels at the same block height, the rationale apply for different blockchains too.

So what I propose (this will need discussion) is that the nonce is uniquely identifiable to the channel itself

Since we use the nonce to order messages provided by third parties, we are considering to use the message signature with additional data that is contract specific #43 (this scheme will change as we discuss #292 , at least we also need blockchain specific data).

As for todos, after removing the courtesy argument, we should also rename updateTransfer to something more meaningful, my first suggestion counterpartyClose.

[LefterisJP]

@hackaugusto Regarding this comment:

but some other things should be put in its place.

I was referring to the 3 bullet points this issue is made for. I will edit the first page of the issue to clarify.

[LefterisJP]

As for todos, after removing the courtesy argument, we should also rename updateTransfer to something more meaningful, my first suggestion counterpartyClose.

Yes we should do that, but perhaps use a different issue to track this. I like counterpartyClose() as well as counterpartyUpdate() since the counterparty does not close the channel, but updates his view of the channel after the initial party closed it.

[hackaugusto]

Since we use the nonce to order messages provided by third parties

Actually, we can use the transferred_amount to order the messages (it's also a monotonically increasing value), the nonce can be moved out of the envelope into a transport detail of Raiden.

NVM, transferred_amount is monotonically increasing but it does not change with every message.

[stonecoldpat]

Actually, we can use the transferred_amount to order the messages (it's also a monotonically increasing value), the nonce can be moved out of the envelope into a transport detail of Raiden.

NVM, transferred_amount is monotonically increasing but it does not change with every message.

Right now - Raiden implements two unidirectional channels (Alice -> Bob, and Bob -> Alice). So all payments are one-way. To me, this would allow the removal of the nonce as transferred_amount can only increase monotonically (i.e. largest transferred_amount represents latest payment).

I was thinking about it last night and I'm not sure why Raiden is implementing two unidirectional channels. In Bitcoin it makes sense - if we want to naively implement bidirectional payment channels then each payment needs to decrement nLockTime (i.e. the Absolute Lock Time). If we have two unidirectional channels, then we can have nearly infinite small payments until we run out of bitcoins.

However, in Ethereum, the nonce can be incremented for every new payment which isn't possible in Bitcoin. This would allow Raiden to implement bidirectional payments in a straightforward way. Everytime I send a payment - I sign the new final balance of both parties. If the counterparty isn't happy with that payment - they broadcast the previously agreed payment. This works as any payment sent to the Blockchain will require both of our signatures (so we both agree the final payment).

In the worst case: there should only be two transactions broadcast to the Blockchain (counterpartys claim of the latest payment; and if the counterparty lied, then I broadcast my latest claim of the payment).

The benefit of the above approach over unidirectional channels is that it never needs to close. We are essentially splitting the deposit in each new payment; and the balance can go back/forth.

Of course - if the above was to be implemented - it would take some time - so it might be best sticking with the unidirectional approach for now and then moving to it sometime in the future.

[konradkonrad]

The benefit of the above approach over unidirectional channels is that it never needs to close. We are essentially splitting the deposit in each new payment; and the balance can go back/forth.

Not sure if I understood correctly, but with the netting channels, we already have this property: Alice and Bob can send payments back-and-forth exceeding the initial deposit.

[stonecoldpat]

@konradkonrad

In Raiden we have two channels: A->B [10,0] and B->A [10,0].

So if Alice sends tokens to Bob then: A->B [9,1] and B->A [10,0]. A->B [8,2] and B->A [10,0]. A->B [7,3] and B->A [10,0]. A->B [6,4] and B->A [10,0].

So only one channel is updated. If A->B runs out of ether, then we either go into credit, or we close the channel.

If we include a nonce, then we can have a bidirectional channel: A<->B and [A,B,c], where A is Alice's balance, B is Bob's balance, and c is the counter. A->B and [10,10,1]. A->B and [9,11,2]. A->B and [8,12,3]. A<-B and [9,11,4]. A<-B and [10,10,5].

If Bob closes the channel with [8,12,3], then Alice has one opportunity to broadcast [10,10,5]. (Otherwise; Alice will lose out!)

It still allows for the courtesy close, while giving the counterparty one opportunity to broadcast the latest state.

Just spoke to konradkonrad. The way bidirectional payments is implemented right now is cool. Although - it should still be possible to remove the nonce from each payment in the contract (and keep it for the UDP layer). As mentioned previously, each party should only have one opportunity to broadcast the money they received from their partner.

[hackaugusto]

Although - it should still be possible to remove the nonce from each payment in the contract (and keep it for the UDP layer).

The nonce is increased for each message, not for all payments, example:

• A -> B latest message M1 was with a transferred_amount of 10 and a nonce of 3, the locksroot is empty.
• N mediates a message through A -> B, the new latest message M2 from A -> B has a transferred_amount of 10 and a nonce of 4, the locksroot contains the hash of N's hash time lock.

Now let's suppose that B goes offline and was using two third parties, the first third party T1 lagged behind and only knows M1, the second third party T2 is up-to-date and knows the message M2.

If both T1 and T2 dispute on behalf of B the smart contract needs to decide which message is newer, the transferred amount is not sufficient (because it's equal), so we use the nonce to order the messages, and this is crucial because M2 has a pending lock, if the smart contract keeps M1 and the secret is revealed while the channel is being closed, B won't be able to claim the lock and A will keep the tokens.

We need the nonces to create a total order of the messages, because not all messages change the transferred amount.

As a second note, the nonce is not shared by design, if both participants of a channel update the nonce atomically then synchronization is required, with a monotonic nonce per participant and a monotonic transferred amount we can have the same functionality without the synchronization (conflict resolution is done in a manner similar to CRDTs)

[czepluch]

closed with #417