Nodes shouldn't publish their commitment when receiving outdated channel_reestablish

[t-bast]

This was discussed during the spec meeting and is weakly linked to #932

Let's imagine we have two nodes Alice and Bob.
Alice takes her node offline, does weird stuff with her DB (e.g. a migration from sqlite to postgres) then comes back online.
She sends a channel_reestablish to Bob with outdated values (because she messed up her migration).

Expected behavior

Bob detects that Alice is late, so Bob will likely need to publish his latest commitment to help Alice get her funds back.
Bob waits for Alice to send an error before publishing his commitment.

When Alice receives Bob's channel_reestablish, she realizes she's late.
She stops her node (without sending an error), figures out where she messed up in her migration, fixes her DB, restarts.
Now she sends a channel_reestablish with the up-to-date value, so the channel can resume operating.

Non optimal behavior

Bob detects that Alice is late, so Bob publishes his latest commitment to help Alice.
But now Alice lost her chance to fix her DB.
If Alice is a big node with a ton of channels, she just lost a ton of money on on-chain fees...

Conclusion

Implementations should really wait to receive an error before publishing their commitment!
@Roasbeef is that clearer than during the spec meeting?

EDIT: the spec was clarified in #942 to highlight that this is the desired behavior.

[rustyrussell]

OK, I looked at this for the latest c-lightning release. Changing behavior to not close when sending error was a big change this close to release, but there is an alternative: send a warning instead on an error here:

BOLT 2:
	 *  - otherwise:
	 *    - if `next_revocation_number` is not equal to 1 greater
	 *      than the commitment number of the last `revoke_and_ack` the
	 *      receiving node has sent:
	 *      - SHOULD fail the channel.

This works for this particular case, at least.

[vincenzopalazzo]

There is another thing that is unclear to me! what does the recipient need to do in the case when he receives the warning message?

Right now, the spec (with warning message proposed by @rustyrussell ) gives the possibility to know where there is some error, but what about the following case:

• Sender -> outdated channel_reestablish -> receiver
• Sender <- warning <- receiver
• Sender -> another outdated channel_reestablish -> receiver

In this case, the sender should have only one choice, or send the right channel_reestablish or he should be able to force close the channel, right?

because in che case of warning message we can fall in a ping pong case, where the sender continue to send wrong data, and the receive continue to response with warnings!

[TheBlueMatt]

That isn't something the spec should really concern itself with - the node that realized its out of date has to do something, but what that is is up to the implementation - it can suggest the user search for a newer state on drive somewhere, it can ask the peer to force-close (using an error message) or it can throw up its hands and light the cat on fire.

[vincenzopalazzo]

Ok! this is clear, but in the spec should be something that should avoid the ping point event? In other words, the sender can put another wrong channel_reestablish? and in this case, the receiver should force close the channel?

Or this is something that will never happen?

[vincenzopalazzo]

After some reasoning over a discussion that @TheBlueMatt points out on LDK (lightningdevkit/rust-lightning#1430 (comment))

I think that I expressed my idea wrong here, and I agree that all this behavior should not specify in the spec, but in my opinion, here we should have a line that specifies "the sender should not send multiple times the wrong channel_restablish" because we know that if a node receives a wrong channel_reestablish it should send a channel_reestablish with the data updated.

What are the benefits of this addition? that we know when a node is not spec compliant, and we can detect this with a test in lnprototest. I mean what is the benefit to keep receiving them wrong channel_restablish?

On the other hand, this is possible from a software point of view, basically because the sender doesn't take into count this case and ignores the update data received with the sender channel_reestablish and due to a possible DB data lost for the send, the data that it sends are perfectly sane.