tapchannel: don't break backward compatibility

[guggero]

This partially reverts commit 3ed142f to restore backward compatibility with nodes running on v0.5.x.

The intention with removing this explicit split root assignment was to simplify the code. But it actually would lead to force closes between nodes on v0.6.0 and v0.5.x, because they wouldn't be able to agree on what output should be chosen for the split root (since the behavior was changed).

[guggero]

I'm going to attempt to add a backward compatibility integration test in litd for this. But that might take me a while, so we should still be able to merge this before (assuming CI is green).

[coveralls]

Pull Request Test Coverage Report for Build 15023058416

Details

• 0 of 17 (0.0%) changed or added relevant lines in 2 files are covered.
• 23 unchanged lines in 6 files lost coverage.
• Overall coverage decreased (-0.02%) to 36.875%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
tapchannel/aux_closer.go	0	2	0.0%
tapchannel/commitment.go	0	15	0.0%

Files with Coverage Reduction	New Missed Lines	%
address/address.go	2	67.47%
tappsbt/create.go	2	26.74%
tapchannel/aux_leaf_signer.go	3	43.43%
asset/asset.go	4	48.98%
asset/mock.go	5	65.01%
tapgarden/caretaker.go	7	68.21%

Totals
Change from base Build 15014433046:	-0.02%
Covered Lines:	26510
Relevant Lines:	71891

---

💛 - Coveralls

[Roasbeef]

LGRT

[Roasbeef]

So this output is the split because it has a lower output index, even though we have a settled to-self output present in the commitment?

[guggero]

This shows why the reverted commit was added initially: If there are multiple asset pieces in a channel, you can't guarantee that every on-chain output gets assets from every piece (depending on the amount distribution). So even if you say the to_remote output should house split root, there might be virtual packets (in this example asset ID 4) will not have any split output in the to_remote output.
In that case we just fall back to having the split at index 0.

But, since this new fallback case can only happen for new, grouped asset channels, it's okay since both nodes need to be on the newer version to understand.
And for the cases where there's only one asset in the channel, we know that each output will get a piece of that asset and we can guarantee that the split output is at the location it was in previous versions, hence restoring backward compatibility.

[guggero]

I've added a backward compatibility test here: lightninglabs/lightning-terminal#1074
As expected, that test broke and channels force closed before this fix. After updating to this commit, the test passes.

[Roasbeef]

LGTM 💎