Allow decreasing the output value in RBF transactions

[prusnak]

Currently we don't allow user to decrease output values during the RBF process.

We could allow this providing the following is met:

• all output values are either staying the same or they are decreased, i.e. no output value increase allowed
• for each changed output we'll show a confirmation dialog with output (address), delta amount and final amount

[andrewkozlik]

Here are some screenshots of the current WIP. In this example we are taking 0.0001 from an external output and 0.0009428 from a change output to bump the fee from 0.0000378 to 0.0010806.

It's hard to fit the output modification on a single screen, because the address can take up more than two lines. Here is an earlier attempt, where the new amount can easily overflow the screen:

The same flow on TT:

[prusnak]

I like these screens 👍

[andrewkozlik]

The use-case that we are trying to solve by this is fee bumping when the original transaction is transferring the entire account balance ("Send Max"), in which case the only option is to take the fee bump from the external output. I am wondering, do we want to enforce that the decrease in the output amount is always used towards the fee, i.e. that it doesn't go back to the user's change address? I don't see any strong arguments either way, but I am curious if anyone has any opinion on this.

[prusnak]

I am wondering, do we want to enforce that the decrease in the output amount is always used towards the fee

Yes. We want this. That's why I wrote that none of the outputs' values are being increased in the process. Otherwise we'd allow users to create double-spends very easily.

[andrewkozlik]

I am wondering, do we want to enforce that the decrease in the output amount is always used towards the fee

Yes. We want this. That's why I wrote that none of the outputs' values are being increased in the process. Otherwise we'd allow users to create double-spends very easily.

Summary of follow-up discussion IRL:

• A malicious user can always use trezorctl or a third-party tool to create double-spends simply by sending two SignTx requests to Trezor for two transactions that have the same inputs. Preventing easy creation of double-spend transactions needs to be handled by Suite. It is infeasible for the firmware to prevent this.
• In general there are cases where it makes sense to increase the output values in the replacement transaction:
  • Increase/create a change-output if a new input was added to cover the fees.
  • Increase/create an external output in case of a PayJoin. The funds for the increase have to come from a new external input.

So far there doesn't seem to be any problem with allowing increasing of change-outputs in any scenario. As for external outputs, I think it makes sense to split it into two scenarios:

• Non-PayJoin: Allow decreasing of external output amounts and disallow increasing of external output amounts.
• PayJoin: Allow increasing of external output amounts and disallow decreasing of external output amounts, because allowing both would add some extra complexity, which we don't seem to need right now. (The Send Max + PayJoin scenario seems like an edge case not worth solving at the moment. By "extra complexity" I mean that we would need to ensure that the decrease in external output amounts goes entirely towards the fee, i.e. add another variable like orig_external_decrease to the Approver and check and present it correctly in the UI. We can always do that in the future if the need arises.)

[andrewkozlik]

One more comment about the UI on T1. I can make it possible to go back and forth between the "Modify amount for address" screen and "Decrease amount by" screen, since logically they are the same screen and on the TT the user can swipe back and forth between them. @matejzak let me know if you'd like that, otherwise let's leave it for the UI redesign.