[Feature] Warning Screen for High Tx Fees

[Chaitanya-Keyal]

Description

This PR introduces a warning screen that appears before the PSBT math screen if the transaction includes unusually high fees. The goal is to ensure the user is explicitly made aware of any excessive fees before proceeding to sign the transaction.

Fixes #721

Warning Condition

The warning is shown when:

• The fee amount exceeds 25% of the sum of all outputs excluding change, and
• There are outputs other than true change (and fee).

New Warning Screen:

Visual Warning in PSBT Flow Views

---

This pull request is categorized as a:

• New feature

Checklist

• I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

• Yes

The warning screen doesn’t currently apply to any of the existing test PSBT flows. I also noticed there aren’t tests for similar warnings (e.g. Full Spend), so figured it's not needed here either. Do let me know if a test needs to be added!

I have tested this PR on the following platforms/os:

• Raspberry Pi OS Manual Build

[kdmukai]

Ideally this PR would include a flow test for the scenario that triggers this new View. But unfortunately we are missing a flow test for other similar psbt warning screens (e.g. "full spend"), so we don't have an easy copy-paste flow test to start from.

As noted elsewhere, I DO think we should get a unit test on the total output value calcs.

So:

• refactor and add unit test: Blocker
• flow test: nice to have; not a blocker
• screenshot: Nice job already adding it!

[Chaitanya-Keyal]

Hey @kdmukai,

Thanks a lot for the review!

I've updated the PR to incorporate all your suggestions:

• Added two unit tests for checking the output value calculation:
  • One using mocked PSBT data.
  • One using a real PSBT (reused from an existing test). I think the mocked test should be sufficient, but included both for completeness — let me know if you'd prefer to drop the second one.
• Updated the terminology from "huge fee" to "high fee" — noticed Sparrow Wallet also uses "high fee", and it does sound more natural. Open to renaming if you have a better suggestion.
• Added a TODO comment for possibly making the threshold configurable via settings. The threshold value is now a named constant, so translations won't have to be redone in the future.

As for the flow test for this screen (and other PSBT warning views like "full spend"), it probably makes more sense to tackle that in a follow-up PR rather than squeeze it into this one. I can take that up once this is merged.

[Chaitanya-Keyal]

Hey @kdmukai,

Thanks again for your quick review! I've updated the PR with your suggestions:

• Fixed the function not including self-transfers as output
• Updated tests and added one more case to verify self transfer + change calculations

While I was working on these updates, I realized that it could be helpful to introduce a new variable in PSBTParser to explicitly track self-transfers. This would simplify calculations like the one in this function and also improve the screen selection logic in PSBTChangeDetailsView. By doing so, we can more easily differentiate between self-transfers and true change outputs, making the logic clearer and more reusable across the codebase.

Let me know your thoughts on this approach! If it sounds good, perhaps I'll start to work on that on another branch!

[newtonick]

tACK. I reviewed the code but would love to get some additional eyes (beyond Keith and I) on this one since its code changes around parsing the PSBT amounts and displaying them to the user.

I've also not been consistent about the PR expectation should be around including an updated messages.pot files when new text is included that needs translation. So when/if this PR gets merged it would be knowing that the messages.pot would still need to be regenerated/extracted for the dev branch.

[alvroble]

ACK tested as of 8910071 on Raspberry Pi Manual OS.

You could take it one step further. Sparrow displays a visual warning for the fee in the PSBT overview. I propose using the same approach here, perhaps by coloring the fees red when they are too high in the PSBTOverviewView and/or PSBTMathView (in addition to the warning view). This is because, when the user navigates back and forth, the PSBTHighFeeWarningView no longer appears.

The UI would look like this for the PSBTOverviewView (and something similar for PSBTMathView):

Maybe even adding some kind of warning sing like (!) or (high!) next to fee would be also useful too.

Feel free to check or cherry-pick the modifications from my repo alvroble@4f01eec.

[Chaitanya-Keyal]

Thanks @alvroble! That’s a great idea. I’ve updated the PR accordingly and added you as a co-author on the commit.

If the fee is high, both PSBTOverviewScreen and PSBTMathScreen now display "fee (!)" in the dire warning color.

I also moved the high-fee check into its own function in PSBTParser and added a corresponding unit test.

[alvroble]

Thanks @Chaitanya-Keyal!! Now it looks like this:

PSBTOverviewView

PSBTMathView

[Chaitanya-Keyal]

I've updated the tests following the addition of the new, exhaustive TestPSBTParser class. The get_total_output_value() method will now be tested for each PSBT in the list, and all other parameterized and specific tests have also been moved inside the class.

Also, I’ve reworded the warning to use "transaction" instead of "PSBT", in line with the recent changes that do the same. PR description has the updated screenshots.

[newtonick]

@easyuxd would you review the change in this PR? I'm interested in your thoughts on adding a new warning screen for a high transaction fee and the visual indications around having a high fee.

[easyuxd]

Thanks for tagging me, @newtonick.

I think this warning screen is a useful UX improvement, @Chaitanya-Keyal. I would recommend changing the message type from “Warning” to “Dire Warning.” This was a recent addition, I new message type I introduced that is more severe than a warning but not an error, and it encompasses this “high-attention / loss of funds” use case:

https://github.com/easyuxd/seedsigner-ux-guidelines?tab=readme-ov-file#dire-warning

I would also recommend changing the “fee(!)” label color to this DIRE_WARNING_COLOR to match.

[Chaitanya-Keyal]

Thanks @easyuxd! I've updated the view to use DireWarningScreen and the high fee label was already using DIRE_WARNING_COLOR. Updated screenshot is in the description, lmk what you think!

[easyuxd]

@Chaitanya-Keyal Thank you, looks great! ACK

--
Off-topic +@kdmukai:
This PR reminds me of how non-descriptive our titles are. It's not you, but how we originally designed these messaging screens. Since we focus on the screen title more than the small colored text below it, I had proposed removing the colored text and moving that content to the navbar. This would make the screens more scannable and free up valuable pixels. "Caution" doesn't mean much, especially when there's already a colored warning icon. Anyway, this type of change would be global, affecting all messaging screens and translations.

#606

Given the current experience, "Caution" seems as consistent a title as any.