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
7 stars 15 forks source link

Update DAppSigner.ts #170

Open teacoat opened 1 month ago

teacoat commented 1 month ago

When working with HSuite they were encountering issues with signing transactions, this is the change we applied to hashconnect in order to fix the issue. When signing a transaction multiple times (multisig), you want to preserve the node id.

May resolve this: https://github.com/hashgraph/hedera-wallet-connect/issues/124

github-actions[bot] commented 1 month ago

Coverage report

St.:grey_question:
Category Percentage Covered / Total
πŸ”΄ Statements
45.26% (-0.24% πŸ”»)
258/570
πŸ”΄ Branches
25.14% (-0.59% πŸ”»)
44/175
πŸ”΄ Functions 27.34% 38/139
πŸ”΄ Lines
45.4% (-0.26% πŸ”»)
242/533
Show files with reduced coverage πŸ”»
|
St.:grey_question:
| File | Statements | Branches | Functions | Lines | | :-: | :- | :- | :- | :- | :- | | πŸ”΄ |
`...` / DAppSigner.ts
|
47.5% (-1.85% πŸ”»)
|
13.64% (-1.36% πŸ”»)
| 37.5% |
47.37% (-1.95% πŸ”»)
|

Test suite run success

25 tests passing in 2 suites.

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

teacoat commented 1 month ago

It is possible the entire signing function here is wrong, I will do some more testing on this as its quite different to what we are doing in hashconnect

franfernandez20 commented 1 month ago

Maybe we can use an approach similar to this function: keep setting the nodes if the transaction does not have any set, and use the node from the transaction if it does.

/**
 * Sets default consensus nodes that a transaction will be submitted to. Node Account ID(s)
 * must be set before a transaction can be frozen. If they have already been set, this
 * function will not modify the transaction.
 * @param transaction - any instance of a class that extends `Transaction`
 *
 * @see {@link https://docs.hedera.com/hedera/networks/testnet/testnet-nodes | Full list of Testnet-nodes}
 * @see {@link https://docs.hedera.com/hedera/networks/mainnet/mainnet-nodes | Full list of Mainnet-nodes}
 */
export function setDefaultNodeAccountIds<T extends Transaction>(transaction: T): void {
  const isNodeAccountIdNotSet =
    !transaction.nodeAccountIds || transaction.nodeAccountIds.length === 0

  if (!transaction.isFrozen() && isNodeAccountIdNotSet)
    transaction.setNodeAccountIds([new AccountId(3), new AccountId(4), new AccountId(5)])
}
tomachianura commented 1 month ago

thanks guys,trule appreciated!