multisig: add post-kex verification round

[UkoeHB]

Problem: When creating a group multisig account, each step of key exchange relies on messages from all other participants. If a malicious player only sends their final message to one honest player, but not others, then the other honest players will be stuck without completed accounts. If that one honest player receives funds to the address he created, then those funds will either be unspendable or only spendable with cooperation from malicious group members.

Solution: Add a round to multisig account creation to verify that all other participants have completed their accounts.

This PR also has some semantic and control-flow cleanup of the multisig kex implementation (I am not satisfied with the readability of my earlier work - cleanup will continue in PR #8203 and alongside the Seraphis library which adds support for aggregation-style multisig signing).

TODO (done: #8582): I will add an unsafe_update_use_with_care flag to the multisig_account::initialize_kex() and multisig_account::kex_update() methods. This flag will allow users to fast-forward through the extra round added by this PR, and under some conditions even do so safely (without needing to trust other participants).

[moneromooo-monero]

I have no idea if it's OK algorithm wise, I just reviewed C code, and it's good.

[UkoeHB]

Slightly more robust in case of errors elsewhere.

[UkoeHB]

Slight cleanup

[UkoeHB]

I updated the multisig account implementation to always emit the post-kex verification msg once an account is complete. This way the msg is always available if needed. Previously, a lot of tests were using 'empty kex msg emitted' as a proxy for 'multisig account is ready'. Now you need to query if 'multisig is ready' directly (this is more intuitive and robust anyway). That change probably breaks the MMS @rbrunner7 .

Following the multisig kex protocol isn't sufficient by itself to robustly set up multisig accounts. No matter how many rounds you add, a malicious player can always selectively send their final message to only some other players. To get around that blocking vector, we need a solution in the messaging layer. There are a few options.

1. After completing a multisig account, cache the final message set (plus your own final message). If someone says they are stuck on the last round, relay that message cache to them. This has a flaw though: if player A completes an account and publishes the multisig address, player A disappears, then the address receives funds, then if other players are account-blocked then player A won't be around to unblock them, which may mean funds are locked in the account even though there are enough players available to release them.
2. Best practice: The better solution is to rebroadcast the full final message set (plus your own final message) to all other players immediately after completing your account. If automatic rebroadcast fails, then you can fall back to solution 1 (rebroadcast on request).
3. Use a 'group' communication channel for the final communication round, to ensure all players will be able to complete their accounts simultaneously. This may be infeasible or difficult to set up, but is the fastest robust solution.
4. Custom multisig implementations can decide how to handle rebroadcasts (if at all), or even use a 'force update' flag (to be implemented in a future PR) to skip the last round entirely. Customization risks opening attack vectors not present in the default best-practice solution, so implementer beware!

Note: I decided not to implement message caching in the wallet file because it is more appropriate for the message handling layer to take responsibility for handling messages (caching, transmitting, etc.). Also, this way you don't pay for what you don't use, if you don't need wallet-side caching.

[rbrunner7]

As you predicted this needed some more small changes in simplewallet.cpp in order to work correctly. It was not only a question about the MMS; also the normal "manual" multisig workflow did not work correctly anymore. You can see the necessary changes here to copy them over into your code and update it: rbrunner7@fbb6055

I ran my usual tests with your code plus my changes, using the MMS, for 2/2, 2/3 and 3/5 multisig. Everything - building wallets, receiving a tx, sending a tx - worked flawlessly. Of course with an almost fully automatic system like the MMS you hardly notice the additional data exchange round.

[rbrunner7]

To get around that blocking vector, we need a solution in the messaging layer. There are a few options.

I confess I don't fully understand everything you explain and propose here as options. But I brainstormed a little nevertheless how supporting such options could look in the MMS, and how much effort would be needed to implement.

I came up with an estimate of a few working days. It's not too hard, but not trivial either. It will probably need a handful of new commands, new logic within the mms next command and new message types.

With the low usage that the MMS sees and the corresponding low importance for the Monero ecosystem as a whole I don't see me implementing anything here in the near future, frankly.

[rbrunner7]

For this PR it's the same as with the previous multisig-related one: I can't review the code, so this is not a proper code approval, but I did carefully test the code with 2/2, 2/3 and 3/5 multisig and can confirm that everything fully worked.

I think together with @moneromooo-monero 's look at the code from a technical point of view this should give something like a full review ...

[UkoeHB]

@rbrunner7 thank you!

[UkoeHB]

@rbrunner7 for MMS documentation, you could recommend that users manually verify with each other that their accounts are complete before sending any money to the multisig address. It's not automated like the other solutions, but would still work. (edit: doing that would be the manual version of method 3)

[UkoeHB]

Update: the post-kex verification round must also check that all players have the same common pubkey (which is used as the view key by wallets).

[vtnerd]

My comments are minor.

[vtnerd]

A couple of nagging things, sorry :/ Only the one is interesting (the unused parameter in a function).

[UkoeHB]

Squashed