config+itest: allow unsafe disconnect by default

[cfromknecht]

This commit beings the process of deprecating unsafe-disconnect. Many
moons ago this was disallowed to prevent concurency bugs surrounding
reconnect. Despite the name, it has been safe to enable this feature for
well over a year, as several PRs have been merged that addressed the
possible issues that existed when the feature was added.

[joostjager]

I find it hard to form an opinion about this without references to the original concurrency issues and PRs that fixed those. Perhaps getting a review from someone who reviewed the original fix PRs is more efficient.

[cfromknecht]

I find it hard to form an opinion about this without references to the original concurrency issues and PRs that fixed those. Perhaps getting a review from someone who reviewed the original fix PRs is more efficient.

good point, i hadn't thought of that 😂

[halseth]

Why do we want to allow this by default? IIRC it was only added to allow disconnecting nodes during testing, I don't think it had to do with any concurrency issues.

[cfromknecht]

Why do we want to allow this by default? IIRC it was only added to allow disconnecting nodes during testing, I don't think it had to do with any concurrency issues.

@halseth It was both. there were concurrency issues in the peer lifecycle and possibility of duplicate spawning duplicate mailboxes at the time we needed to test with this flag, which have since been resolved via #1658 #1166 #1551 #982 #821 and probably others i'm missing. prior to those changes the link's mailbox could have desynced and caused a channel closure, and preventing disconnects made it less likely to happen as a result of user behavior. that was the rationale behind naming it unsafe-disconnect, to dissuade people from using the feature while those issues existed.

Why do we want to allow this by default?

There are subtle issues that can be resolved by simply disconnecting and reconnecting to your peer. For most who don't know about this option, they need to restart just to be able to disconnect from a channel partner. Our own node operator guide even suggests using this option.

[halseth]

Why do we want to allow this by default?

There are subtle issues that can be resolved by simply disconnecting and reconnecting to your peer. For most who don't know about this option, they need to restart just to be able to disconnect from a channel partner. Our own node operator guide even suggests using this option.

Ok, cool. I think that should be the argument for enabling this by default now, so maybe add that to the PR description? :)

[Roasbeef]

LGTM ☄️

Only some minor commit nits.