hashgraph / hedera-wallet-connect

This package is a messaging relay between decentralized applications and wallets in Hedera network based on Wallet Connect relays.
Apache License 2.0
13 stars 22 forks source link

fix: improves receipt handling for transactions with multiple signatures #338

Closed kantorcodes closed 2 weeks ago

kantorcodes commented 2 weeks ago

Description:

We discovered that certain transactions e.g. AccountCreateTransactions with a threshold key would fail to return a receipt when executeWithSigner was called on the transaction.

Under the hood, call was being executed in the DAppSigner and attempting to run the query through the wallet. Because these queries are free, we should utilize getReceipt instead to ensure consistency and reliability for these complex transactions.

Additionally, we should avoid guessing and running both a query and transaction when call is used by the Hedera SDK. This would return the following incorrect error

Error: (BUG) body.data was not set in the protobuf
    at Transaction.fromBytes (/hedera-sdk-js/src/transaction/Transaction.js:361:27)
    at /hedera-sdk-js/src/checkUpdate.mjs:55:40
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Related issue(s):

Fixes https://github.com/hashgraph/hedera-wallet-connect/issues/337

Notes for reviewer:

  1. You can test this new functionality by running dev:react-demo and running the newest example for creating a multi signature account at the bottom of the demo. The receipt will return instantly instead of hanging.
  2. We'll want https://github.com/hashgraph/hedera-wallet-connect/pull/327 to be merged in first before this one.

Checklist

github-actions[bot] commented 2 weeks ago

Coverage report

St.:grey_question:
Category Percentage Covered / Total
🟑 Statements
74.76% (+36.16% πŸ”Ό)
462/618
πŸ”΄ Branches
58.59% (+30.29% πŸ”Ό)
75/128
🟑 Functions
75.51% (+49.95% πŸ”Ό)
111/147
🟑 Lines
75.74% (+36.05% πŸ”Ό)
434/573
Show new covered files 🐣
|
St.:grey_question:
| File | Statements | Branches | Functions | Lines | | :-: | :- | :- | :- | :- | :- | | 🟒 | index.ts | 100% | 100% | 100% | 100% | | 🟒 | lib/index.ts | 100% | 100% | 100% | 100% | | 🟒 | lib/shared/index.ts | 100% | 100% | 100% | 100% | | 🟒 |
`...` / chainIds.ts
| 100% | 100% | 100% | 100% | | πŸ”΄ |
`...` / errors.ts
| 20% | 0% | 0% | 25% | | 🟒 |
`...` / events.ts
| 100% | 100% | 100% | 100% | | 🟒 |
`...` / methods.ts
| 100% | 100% | 100% | 100% | | 🟒 | lib/shared/utils.ts | 86.96% | 66.67% | 86.21% | 88.46% | | πŸ”΄ |
`...` / extensionController.ts
| 30.77% | 11.11% | 16.67% | 28.57% | | πŸ”΄ | lib/wallet/index.ts | 38.68% | 28.57% | 55.56% | 40% | | πŸ”΄ |
`...` / provider.ts
| 38.46% | 100% | 27.27% | 38.46% | | 🟒 | lib/dapp/index.ts | 86.83% | 76.19% | 83.33% | 87.83% | | 🟒 |
`...` / logger.ts
| 100% | 100% | 100% | 100% | | 🟒 |
`...` / DAppSigner.ts
| 84.17% | 61.76% | 92.59% | 84.35% |

Test suite run success

108 tests passing in 11 suites.

Report generated by πŸ§ͺjest coverage report action from 9534961b694728a22a07e0335ca74a3e77c89f84

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 15.66265% with 70 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@5ae78a5). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/lib/dapp/index.ts 7.84% 43 Missing and 4 partials :warning:
src/lib/dapp/DAppSigner.ts 31.03% 19 Missing and 1 partial :warning:
src/lib/shared/utils.ts 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #338 +/- ## ======================================= Coverage ? 36.05% ======================================= Files ? 14 Lines ? 613 Branches ? 80 ======================================= Hits ? 221 Misses ? 372 Partials ? 20 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.