Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handle transactions after NAJ update to v4.0.x for WalletConnect #1205

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

kujtimprenku
Copy link
Contributor

@kujtimprenku kujtimprenku commented Sep 15, 2024

Description

After the update of wallet selector to use NAJ v4.0.x the wallet-connect signAndSendTransaction(s) has stopped working because the type of the value of tx.encode() has changed from Buffer to Uint8Array since NAJ v3 the [email protected] update.

WalletConnect uses formats and encodes the payload and this handles the Buffer and Uint8Array differently.

This PR adds support to handle the result returned from a wallet that uses latest version of NAJ (Uint8Array) and still keeps support for the old versions (Buffer).

I have also updated the web-examples of WalletConnect to add support for the latest version(s) of NAJ:
reown-com/web-examples#702

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Type of change.

  • FIX - a PR of this type patches a bug.
  • FEATURE - a PR of this type introduces a new feature.
  • BUILD - a PR of this type introduces build changes.
  • CI - a PR of this type introduces CI changes.
  • DOCS - a PR of this type introduces DOCS improvement.
  • STYLE - a PR of this type introduces style changes.
  • REFACTOR - a PR of this type introduces refactoring.
  • PERFORMANCE - a PR of this type introduces performance changes.
  • TEST - a PR of this type adds more tests.
  • CHORE - a PR introduces other changes than the specified above.

@github-actions github-actions bot changed the title Handle transactions after NAJ update to v4.0.x for WalletConnect fix: Handle transactions after NAJ update to v4.0.x for WalletConnect Sep 15, 2024
Copy link
Contributor

@pivanov pivanov left a comment

Choose a reason for hiding this comment

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

What do you think about this approach?

@@ -354,7 +354,10 @@ const WalletConnect: WalletBehaviourFactory<
},
});

return nearAPI.transactions.SignedTransaction.decode(Buffer.from(result));
// @ts-ignore
Copy link
Contributor

@pivanov pivanov Sep 17, 2024

Choose a reason for hiding this comment

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

let signatureData: Uint8Array;

if (result instanceof Uint8Array) {
  // This will handle both Buffer and Uint8Array since Buffer is a subclass of Uint8Array
  signatureData = result;
} else if (Array.isArray(result)) {
  signatureData = new Uint8Array(result);
} else if (typeof result === "object" && result !== null) {
  signatureData = new Uint8Array(Object.values(result));
} else {
  throw new Error("Unexpected result type from near_signTransaction");
}

return nearAPI.transactions.SignedTransaction.decode(
  Buffer.from(signatureData)
);

We can make the helper method to handle the transaction and use it in the results.map below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pivanov Thanks for the review. I added a new helper function using the suggested code above.

Copy link
Contributor

@pivanov pivanov left a comment

Choose a reason for hiding this comment

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

r+

@pivanov pivanov merged commit aa2db1f into near:dev Oct 4, 2024
4 checks passed
@gtsonevv gtsonevv mentioned this pull request Oct 23, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants