BIP330: drop redundant booleans from the sendtxrcncl message

[vasild]

The reconciliation protocol assumes using one role consistently. Since it is irrelevant which one is which, we can imply that the initiator of the P2P connection will assume the role of reconciliation initiator.

This protocol simplification will seep into the implementation.

[vasild]

Having two booleans means we have to deal with all 4 cases:

1. initiator=false, responder=false - meaningless
2. initiator=false, responder=true - ok
3. initiator=true, responder=false - ok
4. initiator=true, responder=true - not supported (cannot be supported?)

Allowing only 2. and 3. means we can have just one boolean and derive the value of the other from it.

However, since it is completely irrelevant which one is which we can just assign the roles based on who is the P2P connection initiator. bitcoin/bitcoin#23443 does that (+ trying to handle the nonsensical cases).

I think this redundancy can be a source of confusion and/or bugs.

[naumenkogs]

The idea behind two non-implied roles was future upgradeability, which might be not based on the connection.

• What we will have to do then if we drop 2-booleans now? Have a SENDTXRCNCLV2, probably with the same two flags again, or another implied combination, or something completely new. We will have the same debate, although it could be expanded.
• How likely this is to happen? I don't know. I currently have no plans to seek for better reconciliation coordination, and I don't know anyone who has.

The fact that you caught a (comment) mistake in the code which was acked and near-merged is probably a good argument towards dropping this redundancy.

@sipa what do you think?

[sipa]

@naumenkogs I'm fine with just making the reconciliation direction fixed by the protocol. There is still a version number in sendtxrcnl, which could be used for future extensions.

[naumenkogs]

Then I hope for the following events to occur shortly:

1. Merge the Core PR
2. Change this in a follow-up PR to Core, along with this BIP update merge

(unless we come to a different opinion in the meanwhile)
(also unless it takes too long and Luke rushes us to do something :))

[sipa]

I missed this was a PR already.

ACK 8b107a0

[naumenkogs]

@vasild kinda unrelated to this PR. I'm not sure we have to always seek a synchronization between BIPs and master. I expect this BIP to be updated at least couple more times along the Erlay merging, and I think it's no big deal if e.g. the lag is one month.

[vasild]

Sounds good to use the version for future upgrades - then the sendtxrcncl can be expanded/appended with further data as needed.

Yeah, the waterfall model where design is followed by implementation rarely works. There will be iterative steps where some mods will be done to the design while the implementation is happening. We just have to keep in mind alternative implementations and not let things diverge too much for a long time, in case somebody implements stuff based on an outdated spec.

[naumenkogs]

ACK 8b107a0

[maflcko]

@luke-jr @kallewoof