feat(legacy): Allow decreasing output amount in RBF transactions.

legacy/firmware/CHANGELOG.md

@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Public key to ECDHSessionKey.  [#1518]
 
 ### Changed
+- Allow decreasing the output value in RBF transactions.  [#1491]
 
 ### Deprecated
 
@@ -380,3 +381,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 [#1402]: https://github.com/trezor/trezor-firmware/pull/1402
 [#1415]: https://github.com/trezor/trezor-firmware/pull/1415
 [#1518]: https://github.com/trezor/trezor-firmware/pull/1518
+[#1491]: https://github.com/trezor/trezor-firmware/issues/1491

legacy/firmware/layout2.c

@@ -503,6 +503,37 @@ void layoutConfirmReplacement(const char *description, uint8_t txid[32]) {
                     description, str[0], str[1], str[2], str[3], NULL);
 }
 
+void layoutConfirmModifyOutput(const CoinInfo *coin, AmountUnit amount_unit,
+                               TxOutputType *out, TxOutputType *orig_out,
+                               int page) {
+  if (page == 0) {
+    render_address_dialog(coin, out->address, _("Modify amount for"),
+                          _("address:"), NULL);
+  } else {
+    char *question = NULL;
+    uint64_t amount_change = 0;
+    if (orig_out->amount < out->amount) {
+      question = _("Increase amount by:");
+      amount_change = out->amount - orig_out->amount;
+    } else {
+      question = _("Decrease amount by:");
+      amount_change = orig_out->amount - out->amount;
+    }
+
+    char str_amount_change[32] = {0};
+    format_coin_amount(amount_change, NULL, coin, amount_unit,
+                       str_amount_change, sizeof(str_amount_change));
+
+    char str_amount_new[32] = {0};
+    format_coin_amount(out->amount, NULL, coin, amount_unit, str_amount_new,
+                       sizeof(str_amount_new));
+
+    layoutDialogSwipe(&bmp_icon_question, _("Cancel"), _("Confirm"), NULL,
+                      question, str_amount_change, NULL, _("New amount:"),
+                      str_amount_new, NULL);
+  }
+}
+
 void layoutConfirmModifyFee(const CoinInfo *coin, AmountUnit amount_unit,
                             uint64_t fee_old, uint64_t fee_new) {
   char str_fee_change[32] = {0};

legacy/firmware/layout2.h

@@ -54,6 +54,9 @@ void layoutConfirmTx(const CoinInfo *coin, AmountUnit amount_unit,
                      uint64_t total_in, uint64_t total_out,
                      uint64_t change_out);
 void layoutConfirmReplacement(const char *description, uint8_t txid[32]);
+void layoutConfirmModifyOutput(const CoinInfo *coin, AmountUnit amount_unit,
+                               TxOutputType *out, TxOutputType *orig_out,
+                               int page);
 void layoutConfirmModifyFee(const CoinInfo *coin, AmountUnit amount_unit,
                             uint64_t fee_old, uint64_t fee_new);
 void layoutFeeOverThreshold(const CoinInfo *coin, AmountUnit amount_unit,

legacy/firmware/signing.c

@@ -296,6 +296,10 @@ static bool add_amount(uint64_t *dest, uint64_t amount) {
   return true;
 }
 
+static bool is_rbf_enabled(TxInfo *tx_info) {
+  return tx_info->min_sequence <= MAX_BIP125_RBF_SEQUENCE;
+}
+
 void send_req_1_input(void) {
   signing_stage = STAGE_REQUEST_1_INPUT;
   resp.has_request_type = true;
@@ -527,6 +531,21 @@ void phase1_request_next_input(void) {
         return;
       }
 
+      char *description = NULL;
+      if (!is_rbf_enabled(&info) && is_rbf_enabled(&orig_info)) {
+        description = _("Finalize TXID:");
+      } else {
+        description = _("Update TXID:");
+      }
+
+      // Confirm original TXID.
+      layoutConfirmReplacement(description, orig_hash);
+      if (!protectButton(ButtonRequestType_ButtonRequest_SignTx, false)) {
+        fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
+        signing_abort();
+        return;
+      }
+
       idx2 = 0;
     }
 
@@ -1449,15 +1468,28 @@ static bool signing_check_orig_output(TxOutputType *orig_output) {
       return false;
     }
 
-    // Replacement transactions must not decrease the value of any external
-    // outputs. Furthermore, the only way to increase the amount would be by
-    // supplying an external input, which is currently not supported, so the
-    // external output amounts must remain unchanged.
     if (!is_change) {
-      if (output.amount != orig_output->amount) {
+      if (output.amount < orig_output->amount) {
+        // Replacement transactions may need to decrease the value of external
+        // outputs to bump the fee. This is needed if the original transaction
+        // transfers the entire account balance ("Send Max").
+        for (int page = 0; page < 2; ++page) {
+          layoutConfirmModifyOutput(coin, amount_unit, &output, orig_output,
+                                    page);
+          if (!protectButton(ButtonRequestType_ButtonRequest_ConfirmOutput,
+                             false)) {
+            fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
+            signing_abort();
+            return false;
+          }
+        }
+      } else if (output.amount > orig_output->amount) {
+        // Only PayJoin transactions may increase the value of external outputs
+        // by supplying an external input. However, external inputs are
+        // currently not supported.
         fsm_sendFailure(
             FailureType_Failure_ProcessError,
-            _("Changing original output amounts is not supported."));
+            _("Increasing original output amounts is not supported."));
         signing_abort();
         return false;
       }
@@ -1544,29 +1576,6 @@ static bool signing_confirm_tx(void) {
       return false;
     }
 
-    bool rbf_disabled = info.min_sequence > MAX_BIP125_RBF_SEQUENCE;
-    bool orig_rbf_disabled = orig_info.min_sequence > MAX_BIP125_RBF_SEQUENCE;
-    char *description = NULL;
-    if (rbf_disabled && !orig_rbf_disabled) {
-      description = _("Finalize TXID:");
-    } else if (fee != orig_fee) {
-      description = _("Modify fee for TXID:");
-    } else {
-      // The host might want to modify nSequence on some inputs (e.g. to
-      // re-enable RBF on a transaction that was dropped from the mempool), add
-      // more inputs and consolidate them in a change-output or use a different
-      // change-output address.
-      description = _("Update TXID:");
-    }
-
-    // Confirm original TXID.
-    layoutConfirmReplacement(description, orig_hash);
-    if (!protectButton(ButtonRequestType_ButtonRequest_SignTx, false)) {
-      fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL);
-      signing_abort();
-      return false;
-    }
-
     // Fee modification.
     if (fee != orig_fee) {
       layoutConfirmModifyFee(coin, amount_unit, orig_fee, fee);