Skip to content

fix: TransactionBaseModel.computeSignatureKey() must propagate exceptions - #2607

Merged
svienot merged 5 commits into
mainfrom
2552_compute_signature_key
Apr 7, 2026
Merged

fix: TransactionBaseModel.computeSignatureKey() must propagate exceptions#2607
svienot merged 5 commits into
mainfrom
2552_compute_signature_key

Conversation

@ericleponner

@ericleponner ericleponner commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description:

Changes below update TransactionBaseModel.computeSignatureKey() method.
Current method code swallows exceptions which prevents to use them for user error reporting.
Changes below make sure that:

  • TransactionBaseModel.computeSignatureKey() propagates errors
  • callers correctly handle bubbling errors

For archive purpose, here is the calling tree of TransactionBaseModel.computeSignatureKey():

TransactionBaseModel.computeSignatureKey()                  renderer/utils/transactionSignatureModels/transaction.model.ts
    computeSignatureKey()                                      renderer/utils/transactionSignatureModels/index.ts

        usersPublicRequiredToSign() OK                              renderer/utils/transactionSignatureModels/index.ts
            isSignableTransaction() OK                                  renderer/utils/transactionSignatureModels/index.ts
            collectRequiredKeys() OK                                    renderer/utils/index.ts
            signTransactions() OK                                       renderer/utils/index.ts
            TransactionDetailsHeader.watch(tx) OK                       renderer/pages/TransactionDetails/components/TransactionDetailsHeader.vue


        BaseTransaction.updateTransactionKey FIXED                  renderer/components/Transaction/Create/BaseTransaction/BaseTransaction.vue

        useTransactionAudit.signatureKey() FIXED                    renderer/composables/useTransactionAudit.ts

        ExportTransactionsModal.handleExport() FIXED                renderer/components/ExternalSigning/ExportTransactionsModal.vue

        TransactionBrowserEntry.make() OK                           renderer/components/ExternalSigning/TransactionBrowser/TransactionBrowserEntry.ts

        filterTransactionFileItemsToBeSigned() OK                   shared/utils/transactionFile.ts

        TransactionBrowserKeySection.updateSignatureKeyObject() OK  renderer/components/ExternalSigning/TransactionBrowser/TransactionBrowserKeySection.vue

        TransactionDetails.fetchTransaction() FIXED                 renderer/pages/TransactionDetails/TransactionDetails.vue

        collectMissingSignerKeys() OK                               shared/utils/transactionFile.ts

OK means code is already protected from bubbling errors.
FIXED means code is updated by this PR.

Related issue(s):

Prerequisite for #2552

Notes for reviewer:

M   front-end/src/renderer/utils/transactionSignatureModels/transaction.model.ts
    # TransactionBaseModel.computeSignatureKey() now propagates errors

M   front-end/src/renderer/components/ExternalSigning/ExportTransactionsModal.vue
M   front-end/src/renderer/components/Transaction/Create/BaseTransaction/BaseTransaction.vue
M   front-end/src/renderer/composables/useTransactionAudit.ts
M   front-end/src/renderer/pages/TransactionDetails/TransactionDetails.vue
    # Now processes errors bubbling from computeSignatureKey()

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@codecov

codecov Bot commented Apr 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (d2c3e78) to head (3f4317d).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2607   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files         183      183           
  Lines        7410     7410           
  Branches     1478     1478           
=======================================
  Hits         7407     7407           
  Misses          3        3           

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…to callers.

Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>
@ericleponner
ericleponner force-pushed the 2552_compute_signature_key branch from 76897a5 to 1da8a31 Compare April 7, 2026 10:39
@ericleponner
ericleponner marked this pull request as ready for review April 7, 2026 15:14
@ericleponner
ericleponner requested a review from a team as a code owner April 7, 2026 15:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the renderer transaction signature audit flow so that TransactionBaseModel.computeSignatureKey() no longer swallows mirror-node / cache lookup failures, enabling upstream code to detect and react to signature-key computation errors.

Changes:

  • Make TransactionBaseModel.computeSignatureKey() rethrow lookup errors after logging.
  • Update multiple callers to wrap computeSignatureKey() in try/catch and apply fallbacks when it fails.
  • Adjust export filtering logic for “external signer only” flows to tolerate signature-key computation failures.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
front-end/src/renderer/utils/transactionSignatureModels/transaction.model.ts Stops swallowing lookup exceptions and rethrows them so callers can handle failures.
front-end/src/renderer/pages/TransactionDetails/TransactionDetails.vue Wraps signature-audit computation in try/catch and clears the audit on failure.
front-end/src/renderer/composables/useTransactionAudit.ts Wraps signature-audit computation in try/catch and returns null on failure.
front-end/src/renderer/components/Transaction/Create/BaseTransaction/BaseTransaction.vue Wraps transaction key computation in try/catch and falls back when it fails.
front-end/src/renderer/components/ExternalSigning/ExportTransactionsModal.vue Adds try/catch around external-transaction filtering based on signature audit.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread front-end/src/renderer/components/ExternalSigning/ExportTransactionsModal.vue Outdated
Comment thread front-end/src/renderer/pages/TransactionDetails/TransactionDetails.vue Outdated
Comment thread front-end/src/renderer/composables/useTransactionAudit.ts Outdated
ericleponner and others added 4 commits April 7, 2026 17:34
…ctionsModal.vue

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Eric Le Ponner <91124272+ericleponner@users.noreply.github.com>
Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>
Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>
Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>

@svienot svienot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@steven-sheehy steven-sheehy added Bug A error that causes the feature to behave differently than what was expected based on design docs Frontend Frontend development work labels Apr 7, 2026
@steven-sheehy steven-sheehy added this to the v0.28.0 milestone Apr 7, 2026
@svienot
svienot merged commit fd95dd6 into main Apr 7, 2026
32 of 33 checks passed
@svienot
svienot deleted the 2552_compute_signature_key branch April 7, 2026 17:21
Kolpic pushed a commit that referenced this pull request Apr 15, 2026
…ions (#2607)

* TransactionBaseModel.computeSignatureKey() now propagates exceptions to callers.

Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>

* Update front-end/src/renderer/components/ExternalSigning/ExportTransactionsModal.vue

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Eric Le Ponner <91124272+ericleponner@users.noreply.github.com>

* Fixed ExportTransactionsModal.handleExport().

Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>

* TransactionDetails.fetchTransaction() now post a toast in case of error.

Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>

* useTransactionAudit.signatureKey now send logger message.

Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>

---------

Signed-off-by: Eric Le Ponner <eric.leponner@icloud.com>
Signed-off-by: Eric Le Ponner <91124272+ericleponner@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug A error that causes the feature to behave differently than what was expected based on design docs Frontend Frontend development work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants