Implement CoinJoin preauthorization and siging flow

[andrewkozlik]

This PR implements #1053 and replaces the draft PR #1094.

The implementation enables the following flow:

1. The client application sends an AuthorizeCoinJoin message. The parameters for user approval include:
   • The coordinator name, e.g. "www.trezor.io".
   • The maximum total fee.
   • BIP32 account path of the inputs and their script type.
   • Optionally the fee per anonymity set as a uint32 in units of 10^-9 percent. So 0.003 % would be given as 3000000 and the maximum fee that can be set is approximately 4.29 %. I didn't want to use floats to avoid showing something like 0.00299999 on the display, plus we don't support floats in our protobuf implementation anyway.
2. User confirms the message parameters (coordinator name, fee per anonymity set, maximum total fee, possibly a non-standard BIP32 account path).
   
3. The client application can now issue certain commands without requiring any user interaction. The commands work even in soft-locked mode. To do this the client takes the following steps:
   i. Client sends a DoPreauthorized message (no parameters).
   ii. If there is an active authorization, then the client receives a PreauthorizedRequest message from the device, otherwise ProcessError.
   iii. Client sends the desired command and communication continues as usual for that command.

At most one authorization per session is allowed to exist at any one time. If a new authorization is confirmed, it replaces any previous one. Multiple authorizations are currently possible by having them in different sessions. The DoPreauthorized flow can be performed multiple times for one authorization. It supports two commands:

• GetOwnershipProof:
  • The coin and BIP32 account path must match the authorized values.
  • The commitment data must begin with the authorized coordinator name followed by any 32 byte value, which should be a CoinJoin round ID randomly generated by the coordinator.
  • The user_confirmed bit can and should be set to True, indicating that the user confirmed participation in any CoinJoin round organized by the given coordinator.
• SignTx:
  • This command can be issued repeatedly until the authorized fees are depleted, i.e. every time a transaction is signed, the overall fee is deducted from the authorized fee amount.
  • The outputs must be ordered so that the CoinJoined outputs are grouped by amount.
  • All of the user's inputs must come from the authorized BIP32 account path and have the approved script type.
  • Each of the user's outputs must satisfy the conditions for being a change output, otherwise it is treated as an external output. This mainly means the user's outputs have to go back into the same account the inputs came from.
  • The transaction will be declined if any of the following conditions are met:
    • The transaction doesn't increase the anonymity of the user's outputs.
    • The overall mining fee exceeds the standard threshold for which we would normally show a warning.
    • The user's fees exceed his fair share of coordinator and mining fees.
    • The user's total fees exceed the authorized fee amount.
    • The transaction has non-zero nLockTime.

Further notes:

• Consider whether it's wise to decline signing if the overall mining fee exceeds the standard threshold for which we would normally show a warning.
• Having multiple outputs having the same denomination and belonging to the same user is currently supported. We could forbid this, since Wasabi doesn't allow it. However, there is a minuscule possibility of a user's change output having the same denomination as a user's CoinJoin output.
• The implementation assumes standard account paths are being used. If a different purpose value is used in the BIP32 path, then the user will need to explicitly confirm this as part of the AuthorizeCoinJoin message processing. If a non-standard chain value is used in the BIP32 path, then the SignTx request will be declined. Once we have a final decision on the paths we will be using for CoinJoin, the code should be updated to take that into account.
• It might be good to add a CancelAuthorization message (without parameters) to the management messages.
• If in the future a need arises to support multiple authorizations per session, then we can add an authorization ID parameter to the messages. But it would probably be a breaking change to the protocol, for example the AuthorizeCoinJoin message would need to return an authorization ID. Currently it returns Success, which only has a string message parameter.

[matejcik]

unrelated to preauthorization: i'm wondering if change detection and friends shouldn't also be part of the approver?

overall looks good, I have some minor style comments and some questions

[matejcik]

also let's think of some unit tests for all this?

[andrewkozlik]

unrelated to preauthorization: i'm wondering if change detection and friends shouldn't also be part of the approver?

I had the same thought, but it would mean that then we should make the MultisigFingerprintChecker and WalletPathChecker part of the approver, which also means calling the approver in sign_bip143_input() and get_legacy_tx_digest(). My intention was for the approver to function as stand-in for the checks that are normally done by the human and this would shift the line somewhere else. I am not saying that that's necessarily wrong. I will give it some more consideration.

[andrewkozlik]

also let's think of some unit tests for all this?

I think the device tests currently test the most important stuff, like that the fees get subtracted from the authorized amount and even the fair sharing of mining fees. So that seems pretty sufficient. I wouldn't want to go all out on testing everything there is to test at the moment, because I don't think the the transaction checks are set in stone yet. I wouldn't be surprised if we tweaked and revised them as the integration of the CoinJoin feature develops.

[matejcik]

also let's think of some unit tests for all this?

I think the device tests currently test the most important stuff, like that the fees get subtracted from the authorized amount and even the fair sharing of mining fees. So that seems pretty sufficient. I wouldn't want to go all out on testing everything there is to test at the moment, because I don't think the the transaction checks are set in stone yet. I wouldn't be surprised if we tweaked and revised them as the integration of the CoinJoin feature develops.

i think some very basic tests for the Authorization object in separation would be good, and for the Approvers as well; basically functioning as smoke tests plus the place to put more extensive checks and/or regression tests in the future.

I also think that with the approvers, it would be good to move out some parts of the existing apps.bitcoin.segwit.signtx testcases and convert them to stand-alone Approver testcases, but I'm not sure how big a task that is.

[matejcik]

checks that are normally done by the human

This is a reasonable basis - worth writing down somewhere, I think

[andrewkozlik]

checks that are normally done by the human

This is a reasonable basis - worth writing down somewhere, I think

Added a comment: de34cf9.

[matejcik]

LGTM on existing code now, all that remains is some basic unit tests (see #1127 (comment))

[andrewkozlik]

LGTM on existing code now, all that remains is some basic unit tests (see #1127 (comment))

@matejcik, I added basic unit tests for CoinJoinApprover and CoinJoinAuthorization: 9f76ef2

[andrewkozlik]

I also added a CancelAuthorization message, which is bound to come in handy sooner or later, see fa05f0a and 94741d9.

[MaxHillebrand]

This is fantastic - thanks and congrats!!

FYI...

Having multiple outputs having the same denomination and belonging to the same user is currently supported. We could forbid this, since Wasabi doesn't allow it. However, there is a minuscule possibility of a user's change output having the same denomination as a user's CoinJoin output.

In fact, this happens rather often, specifically when there is more than one user of the first CoinJoin registering to remix in the second CoinJoin. The input value of these users is equivalent, because it comes from the same CoinJoin, and their change output is equivalent too, as the equal denomination in the second CoinJoin is the same for them too.