Light-client: Old, valid EnvelopeMessages should be accepted

[andrevmatos]

Description

EnvelopeMessages are messages containing BalanceProof updates. They can be mainly LockedTransfer, RefundTransfer, Unlock or LockExpired, and contain state changes on top of the previous end state (balance proof). There's several mechanisms to avoid creating these state changes while partner isn't accessible/available/online/replying, but race conditions may happen, e.g: a LockedTransfer state change is generated and persisted, and at the moment it's being sent, the partner goes offline, and only come back days later.
Even if we also generate a LockExpired in the meantime, partner must accept both messages and update their side of the state, so peers should always be kept in sync. If one fails to do that, they'll get out-of-sync, and only way to resolve this conflict is on-chain, by closing the channel. We must accept old, valid state changes, while (of course) avoiding some conflicting side effects (e.g. forwarding a mediated transfer based on an expired LockedTransfer).

Notice this issue is theoric, not seen in real life due to said measures to avoid generating state changes for non-responsive peers, but still may lead to out-of-sync state, and thus should be fixed

Acceptance criteria

• State changes are always kept in sync
• We don't act on old state changes

Tasks

• [ ]

[christianbrb]

@nephix @kelsos Do you have got open questions regarding this issue? If you can label it with "Ready" Thank you :)

[kelsos]

An old valid EnvelopMessage means one with a lower nonce right?

[nephix]

I have no questions since I have no experience with that part of the SDK.. steps to reproduce such a "theoretic issue" would definitely help but I can understand that this may not be possible due to complexity

[andrevmatos]

Old valid EnvelopeMessage is like an expired LockedTransfer. Currently, we would reject it right away because it's expired. But we need to accept it (with care to not consider it an accepted received transfer) for the sake of state change syncing, and then on top of it we can accept LockExpired or Unlock (not expected, since we didn't SecretRequest nor Reveal, but valid).
It's hard to tell how to reproduce because, as I said, there's several measures to ensure these edge-cases/race conditions don't happen on real life. An e2e test could more easily reproduce it, by synthetically tweaking the conditions: simply generate a LockedTransfer, but don't 'receive' it; advance blockNumber past expiration timeout; generate the LockExpired message; try to receive both; it should be accepted as valid state changes, but currently it'll reject first (because of expiration) and will get out of sync and reject all later messages by nonce; only way to recover now would be to fix the issue or to dispute on-chain by closing channel.