Tweak CoinJoin transaction checks

[andrewkozlik]

As the integration of the CoinJoin feature develops we should revise and tweak the rules for CoinJoin transaction validation in Trezor which were implemented in #1127:

• The implementation assumes standard account paths are being used (purposes 44, 45, 48, 49, 84). 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.

• Decide what checks should Trezor perform on the transaction. Currently we have the following:

  • All of the user's inputs must come from the approved account path and have the approved script type.
  • All of the user's outputs should go back into the same account the inputs came from, otherwise they are treated as external outputs, i.e. fees.
  • The sum of all fees paid by the user doesn't exceed the authorized limit.
  • The overall mining fee must not exceed the standard threshold for which we would normally show a warning. It might be better to drop this requirement, especially once we have Introduce hard limit on transaction fee (core) #1087.
  • The user must not pay more than the sum of their fair share of the total mining fees and of the coordinator fee at the approved rate per anonymity set. This might need some tweaking to conform with the way Wasabi computes the mining fees, because it apparently counts 67 bytes per input (seems slightly lower than expected), 33 bytes per output (seems slightly higher than expected) and then there are some rules around exempting users who don't have enough change from paying mining fees etc. Also, I am not sure who pays the mining fee share for the coordinator's output. The current implementation effectively assumes it gets deducted from the coordinator's output.
  • The transaction must improve the anonymity of at least one of the user's outputs, i.e. it must have the same denomination as another output which is not owned by the user. Just a basic sanity check.
  • nLockTime must be 0. I don't see a use-case for allowing a non-zero value, but neither do I see any danger in it.

[tsusanka]

Tokens/credits will alter most of these points so we do not want to deal with this at this moment.

[andrewkozlik]

We will also need to tweak the format of the commitment data in the ownership proofs. Currently we use coordinator URL + round ID, but the WabiSabi backend uses len(URL) + URL + round ID, which is better.

Update: Done in #2122.