Skip to content

Conversation

@0xFirekeeper
Copy link
Member

@0xFirekeeper 0xFirekeeper commented Oct 16, 2025

Introduces support for 'featuredWalletIds' and 'singleWalletId' in ReownOptions, allowing more granular control over wallet selection in the Reown wallet integration. Updates the PlaygroundManager example to showcase these new options and adds an OAuthProvider enum for improved social login handling. Also updates Reown package dependencies to version 1.5.1.


PR-Codex overview

This PR focuses on updating the version of several com.reown packages in the manifest.json and modifying the ReownOptions class and related methods to include new properties. It also introduces a new OAuthProvider enum and updates wallet connection logic.

Detailed summary

  • Updated com.reown package versions from 1.5.0 to 1.5.1 in Packages/manifest.json.
  • Added properties: FeaturedWalletIds, SingleWalletId, TryResumeSession to ReownOptions.
  • Modified constructor of ReownOptions to validate singleWalletId usage.
  • Updated wallet connection logic to incorporate new options.
  • Introduced OAuthProvider enum in PlaygroundManager.
  • Updated Wallet_Social method to parse Social property into AuthProvider.
  • Added new Wallet_External_Direct method for direct wallet connections.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • New Features

    • Social wallet sign-in via multiple OAuth providers.
    • Configurable featured wallet list for richer wallet discovery.
    • Single-wallet direct connection mode for streamlined onboarding.
    • Optional session resume behavior for Reown-based connections.
  • Chores

    • Bumped Reown-related package versions to the latest patch release.

Introduces support for 'featuredWalletIds' and 'singleWalletId' in ReownOptions, allowing more granular control over wallet selection in the Reown wallet integration. Updates the PlaygroundManager example to showcase these new options and adds an OAuthProvider enum for improved social login handling. Also updates Reown package dependencies to version 1.5.1.
@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2025

Walkthrough

Adds OAuth provider selection and social wallet support; extends Reown options with FeaturedWalletIds, SingleWalletId, and TryResumeSession plus validation; updates Reown interactive and direct connection flows to honor new options; bumps multiple com.reown.* packages from 1.5.0 to 1.5.1.

Changes

Cohort / File(s) Summary
Playground / OAuth
Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs
Adds OAuthProvider enum and public OAuthProvider Social; maps selected OAuthProvider to an AuthProvider (fallback to Google) for InApp wallet flows; updates social wallet creation path.
Reown Options & Manager
Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs
Adds FeaturedWalletIds (string[]), SingleWalletId (string), and TryResumeSession (bool) to ReownOptions; extends constructor/signature to accept them; assigns fields and validates SingleWalletId is not combined with other ID filters; passes new options into Reown wallet creation.
Reown Wallet Flow
Assets/Thirdweb/Runtime/Unity/Wallets/Reown/ReownWallet.cs
Extends ReownWallet.Create(...) to accept featuredWalletIds, singleWalletId, and tryResumeSession; when singleWalletId is set clears included/excluded/featured lists in AppKitConfig; gates resume logic with tryResumeSession; adds WaitForInteractiveConnectionAsync(TimeSpan timeout, string singleWalletId = null) and uses ConnectAsync(singleWalletId) when provided.
Package Version Bumps
Packages/manifest.json, Packages/packages-lock.json
Bumps multiple com.reown.* package versions from 1.5.0 to 1.5.1 and updates corresponding lockfile entries; no package additions or removals.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Playground as PlaygroundManager
    participant Manager as ThirdwebManagerBase
    participant Reown as ReownWallet
    participant AppKit

    rect rgb(245,245,245)
    Note over User,AppKit: Direct single-wallet flow (SingleWalletId set)
    User->>Playground: Select Reown wallet
    Playground->>Manager: ConnectWallet(reownOptions with SingleWalletId)
    Manager->>Reown: Create(..., singleWalletId)
    Reown->>Reown: Null included/excluded/featured in AppKitConfig
    Reown->>AppKit: ConnectAsync(singleWalletId)
    AppKit->>User: Direct connect (no modal)
    end

    rect rgb(240,250,240)
    Note over User,AppKit: Featured-wallets / modal flow
    User->>Playground: Select Reown wallet
    Playground->>Manager: ConnectWallet(reownOptions with FeaturedWalletIds)
    Manager->>Reown: Create(..., featuredWalletIds)
    Reown->>Reown: Populate AppKitConfig.featuredWalletIds
    Reown->>AppKit: OpenModal()
    AppKit->>User: Show modal with featured wallets
    end

    rect rgb(250,240,245)
    Note over User,Playground: OAuth social flow
    User->>Playground: Choose Social provider
    Playground->>Playground: Map OAuthProvider -> AuthProvider (fallback Google)
    Playground->>Playground: Build InAppWalletOptions(authProvider)
    Playground->>User: Trigger OAuth sign-in via InApp wallet
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Add featured and single wallet options to Reown integration" directly and clearly describes the main feature introduced in this changeset. The core changes involve adding FeaturedWalletIds and SingleWalletId properties to ReownOptions across multiple files (ThirdwebManagerBase.cs, ReownWallet.cs), implementing validation logic, and updating the wallet connection flow to support these new options. The title is concise, avoids vague terminology, and accurately captures the primary objective without unnecessary detail like file names or implementation specifics. A teammate reviewing the commit history would immediately understand that this PR adds new wallet selection capabilities to the Reown integration.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch firekeeper/reown-flags

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs (1)

149-151: Guard against enum drift at runtime.

Enum.Parse will throw if OAuthProvider ever gains a value that AuthProvider doesn’t recognize. Using Enum.TryParse with a safe fallback (e.g., AuthProvider.Default) would keep the playground from hard-crashing during demos while still supporting future additions.

-            var parsedOAuthProvider = (AuthProvider)System.Enum.Parse(typeof(AuthProvider), this.Social.ToString());
+            if (!System.Enum.TryParse<AuthProvider>(this.Social.ToString(), out var parsedOAuthProvider))
+            {
+                parsedOAuthProvider = AuthProvider.Default;
+            }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 156a566 and 4f30b81.

📒 Files selected for processing (5)
  • Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs (4 hunks)
  • Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs (2 hunks)
  • Assets/Thirdweb/Runtime/Unity/Wallets/Reown/ReownWallet.cs (5 hunks)
  • Packages/manifest.json (1 hunks)
  • Packages/packages-lock.json (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs (2)
Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs (4)
  • WalletOptions (233-248)
  • InAppWalletOptions (22-43)
  • ReownOptions (184-209)
  • SmartWalletOptions (134-151)
Assets/Thirdweb/Runtime/Unity/Wallets/Reown/ReownWallet.cs (2)
  • ReownWallet (14-355)
  • ReownWallet (24-24)
🔇 Additional comments (1)
Assets/Thirdweb/Runtime/Unity/Wallets/Reown/ReownWallet.cs (1)

301-308: Graceful direct-connect fallback looks solid.

Nice touch using singleWalletId to try a direct ConnectAsync before falling back to the modal—captures the single-wallet UX without breaking existing multi-wallet behavior.

Replaces Enum.Parse with Enum.TryParse for social auth provider selection. Defaults to Google provider if parsing fails, preventing potential runtime errors.
Introduces a tryResumeSession boolean to ReownOptions and propagates it through the wallet initialization flow. This allows control over whether to attempt resuming an existing session when initializing the Reown wallet.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (6)
Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs (3)

178-186: Serialize cleanly; ignore nulls for new fields.

To reduce noise in persisted JSON (PlayerPrefs) and API payloads, ignore nulls for optional fields.

-        [JsonProperty("featuredWalletIds")]
+        [JsonProperty("featuredWalletIds", NullValueHandling = NullValueHandling.Ignore)]
         public string[] FeaturedWalletIds;

-        [JsonProperty("singleWalletId")]
+        [JsonProperty("singleWalletId", NullValueHandling = NullValueHandling.Ignore)]
         public string SingleWalletId;

200-203: Tighten validation; treat whitespace as null and allow empty arrays.

Current check blocks even empty arrays and allows whitespace-only SingleWalletId. Prefer non-empty arrays and non-blank single ID.

-            if (singleWalletId != null && (includedWalletIds != null || excludedWalletIds != null || featuredWalletIds != null))
+            singleWalletId = string.IsNullOrWhiteSpace(singleWalletId) ? null : singleWalletId;
+            var hasIncluded = (includedWalletIds?.Length ?? 0) > 0;
+            var hasExcluded = (excludedWalletIds?.Length ?? 0) > 0;
+            var hasFeatured = (featuredWalletIds?.Length ?? 0) > 0;
+            if (singleWalletId != null && (hasIncluded || hasExcluded || hasFeatured))
             {
                 throw new ArgumentException("singleWalletId cannot be used with includedWalletIds, excludedWalletIds, or featuredWalletIds.");
             }

211-213: Defensive copies for arrays.

Clone arrays to avoid external mutation after construction.

-            this.FeaturedWalletIds = featuredWalletIds;
-            this.SingleWalletId = singleWalletId;
+            this.FeaturedWalletIds = featuredWalletIds?.ToArray();
+            this.SingleWalletId = singleWalletId;
             this.TryResumeSession = tryResumeSession;

Add once at file top:

using System.Linq;
Assets/Thirdweb/Runtime/Unity/Wallets/Reown/ReownWallet.cs (3)

60-63: Handle blank SingleWalletId and keep arrays when blank.

Use IsNullOrWhiteSpace so whitespace doesn’t null out arrays.

-                includedWalletIds = singleWalletId == null ? includedWalletIds : null,
-                excludedWalletIds = singleWalletId == null ? excludedWalletIds : null,
-                featuredWalletIds = singleWalletId == null ? featuredWalletIds : null,
+                includedWalletIds = string.IsNullOrWhiteSpace(singleWalletId) ? includedWalletIds : null,
+                excludedWalletIds = string.IsNullOrWhiteSpace(singleWalletId) ? excludedWalletIds : null,
+                featuredWalletIds = string.IsNullOrWhiteSpace(singleWalletId) ? featuredWalletIds : null,

275-275: Optional: support cancellation in wait helper.

Consider accepting a CancellationToken to allow callers to abort before timeout (e.g., scene unload).

-        private static async Task<bool> WaitForInteractiveConnectionAsync(TimeSpan timeout, string singleWalletId = null)
+        private static async Task<bool> WaitForInteractiveConnectionAsync(TimeSpan timeout, string singleWalletId = null, System.Threading.CancellationToken cancellationToken = default)

Then compose a var cancelTask = ThirdwebTask.Run(cancellationToken); (or equivalent) and include it in Task.WhenAny(...). If no helper exists, you can create a TaskCompletionSource<bool> that completes on token cancellation.


302-309: Fallback to modal if direct connect fails.

AppKit.ConnectAsync(singleWalletId) exceptions will currently bubble and abort the flow. Prefer graceful degradation to the modal when the single wallet ID is invalid/unavailable.

-                    if (singleWalletId != null)
-                    {
-                        await AppKit.ConnectAsync(singleWalletId);
-                    }
-                    else
-                    {
-                        AppKit.OpenModal();
-                    }
+                    if (!string.IsNullOrWhiteSpace(singleWalletId))
+                    {
+                        try
+                        {
+                            await AppKit.ConnectAsync(singleWalletId);
+                        }
+                        catch (Exception e)
+                        {
+                            ThirdwebDebug.LogWarning($"Reown direct connect failed ({singleWalletId}): {e.Message}. Falling back to modal.");
+                            AppKit.OpenModal();
+                        }
+                    }
+                    else
+                    {
+                        AppKit.OpenModal();
+                    }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 03e5b67 and 28ce01b.

📒 Files selected for processing (2)
  • Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs (2 hunks)
  • Assets/Thirdweb/Runtime/Unity/Wallets/Reown/ReownWallet.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Assets/Thirdweb/Runtime/Unity/Wallets/Reown/ReownWallet.cs (1)
Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs (5)
  • Task (376-384)
  • Task (390-585)
  • Task (587-601)
  • Task (603-643)
  • Task (645-658)
🔇 Additional comments (5)
Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs (2)

451-455: LGTM: Reown.Create passthrough extended parameters.

The new arguments are correctly forwarded under THIRDWEB_REOWN.


194-198: No positional constructor calls found—concern does not apply.

The verification found all ReownOptions call sites use either the default constructor or named arguments. Zero positional argument invocations exist, so parameter reordering poses no risk of subtle misbinding.

Assets/Thirdweb/Runtime/Unity/Wallets/Reown/ReownWallet.cs (3)

79-79: LGTM: Resume only when allowed.

Gating session resume on TryResumeSession is correct and non-breaking.


88-88: LGTM: Direct-connect when SingleWalletId provided.

Passing singleWalletId into the interactive wait aligns with the new flow.


35-39: Create(...) API change is safe: internal call uses named arguments.

Verification found only one call site in the repo (ThirdwebManagerBase.cs:442), which correctly uses named arguments:

wallet = await ReownWallet.Create(
    client: this.Client,
    activeChainId: walletOptions.ChainId,
    projectId: walletOptions.ReownOptions.ProjectId,
    ...

All new parameters lack defaults, so any external positional callers would fail at compile time—standard breaking change behavior requiring recompilation.

@0xFirekeeper 0xFirekeeper merged commit 30fef7a into v6 Oct 24, 2025
3 checks passed
@0xFirekeeper 0xFirekeeper deleted the firekeeper/reown-flags branch October 24, 2025 17:19
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