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

Feat: enable signAllTransactions for Ingame & External walllets #236

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

owais-star
Copy link
Contributor

@owais-star owais-star commented Oct 15, 2024

⚠️ NOTE: This PR introduces significant improvements to transaction signing, particularly SignAllTransactions, across wallets. Additionally, deprecated and unmaintained Phantom-specific integration has been removed as part of a refactor. Ensure compatibility with your project's transaction flow and update your code if it relied on Phantom WebGL integration.

Status Type ⚠️ Core Change Issue
Ready Feature/Refactor Yes Link

Problem

The current implementation of SignAllTransactions in the Solana.Unity-SDK is incomplete and inefficient, causing issues when signing multiple transactions simultaneously. Additionally, the SDK contains redundant code paths, including outdated Phantom wallet integration that is no longer maintained.

Solution

This PR enhances the SignAllTransactions method by:

  • Allowing multiple transactions to be signed more efficiently across wallets.
  • Adding deduplication of transaction signatures to prioritize non-empty signatures.
  • Improving transaction deserialization for both VersionedTransaction and Transaction types, making it more robust for different transaction versions.

Additionally, as part of a refactor:

  • Phantom-specific integration (WebGL and mobile) has been removed due to its deprecated status and lack of maintenance, consolidating wallet logic across platforms.

Before & After Screenshots

BEFORE:
Inefficient handling of multiple transactions in SignAllTransactions

AFTER:
Improved SignAllTransactions method with efficient handling of multiple transactions and signature validation

Other changes (e.g. bug fixes, small refactors)

  • Deprecated Phantom wallet-specific code paths were removed to clean up and simplify wallet integration logic.
  • Introduced the GetSignatureForAddress method to retrieve signatures for specific public keys.
  • Refined error handling for wallet connection failures.

Deploy Notes

This PR introduces breaking changes by removing the Phantom-specific wallet integrations. Projects relying on Phantom in WebGL will need to migrate to the updated wallet adapter system, which now offers enhanced support for multiple transactions.

New dependencies: None

Copy link
Collaborator

@GabrielePicco GabrielePicco left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @owais-star! I left some questions and comments in the review

Runtime/codebase/DeepLinkWallets/PhantomDeepLink.cs Outdated Show resolved Hide resolved
Runtime/codebase/WalletBase.cs Outdated Show resolved Hide resolved
Runtime/codebase/WalletBase.cs Outdated Show resolved Hide resolved
Runtime/codebase/WalletBase.cs Outdated Show resolved Hide resolved
@owais-star owais-star force-pushed the main branch 3 times, most recently from 539d3b6 to ccae350 Compare October 21, 2024 11:26
Copy link
Collaborator

@GabrielePicco GabrielePicco left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@GabrielePicco GabrielePicco merged commit 92491f6 into magicblock-labs:main Oct 29, 2024
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