feat: add on-chain .skr resolution in sample wallet#282
Open
lopeselio wants to merge 6 commits into
Open
Conversation
Support .skr name resolution for transfer and receive flows without requiring a backend service. Resolve wrapped domains to the actual NFT holder and query mainnet for ANS lookups to improve Seeker mobile UX. Refs magicblock-labs#281
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds native on-chain Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Contributor
Author
|
@Kuldotha requesting your review! |
Contributor
Author
|
In this feature, i have tried to resolve it directly on-chain in Unity (C#), with no backend.
So compared to alternatives:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Samples`~/Solana Wallet/Scripts/example/screens/ReceiveScreen.cs:
- Around line 48-57: ResolveWalletDisplayName currently does a fire-and-forget
reverse lookup and directly sets publicKey_txt.text, risking exceptions or stale
UI updates; change it to capture the walletAddress at invocation (e.g., local
var requestedAddress = walletAddress), wrap the await
SkrAddressResolutionClient.ResolveAddressToDomain(...) in try/catch to
swallow/log errors, and only assign publicKey_txt.text = domain if domain is
non-empty AND requestedAddress still matches the currently displayed wallet (the
same value the UI is showing) so the pubkey fallback remains intact and stale
results are not applied.
In `@Samples`~/Solana Wallet/Scripts/example/screens/TransferScreen.cs:
- Around line 43-58: ResolveRecipientAddress() should be validated for emptiness
once before branching and amount/funds validation (CheckInput) should be applied
to every transfer path; update the control flow around ResolveRecipientAddress,
TransferNft, TransferSol, and TransferToken so that you return early if
recipientAddress is null/empty, then call CheckInput(recipientAddress) (or the
appropriate amount/funds validator) for every branch (including the NFT branch)
before invoking TransferNft/TransferSol/TransferToken.
In `@Samples`~/Solana
Wallet/Scripts/example/services/SkrAddressResolutionClient.cs:
- Around line 44-55: Both public methods ResolveDomainToAddress and
ResolveAddressToDomain should swallow RPC/network failures and return null so
callers can preserve fallback UX: keep the existing early null-for-empty-input
checks, but wrap the calls to ResolveDomainToAddressOnChain(domain) and
ResolveAddressToDomainOnChain(address) in try/catch, log or ignore the
exception, and return null on any caught exception instead of letting it
propagate.
- Around line 296-307: The TrySplitDomain method currently accepts multi-label
domains (e.g., "foo.bar.skr") and returns label="bar" which can incorrectly
resolve "bar.skr"; change TrySplitDomain to only accept exactly two-dot parts
(parts.Length must equal 2) and require the tld to be "skr" (case-insensitive)
before setting label and tld; return false for any other length or non-".skr"
TLD to prevent resolving unintended multi-label domains and ensure label/tld
remain non-empty.
- Around line 133-165: The reverse lookup loop currently returns any label it
finds without verifying ownership or expiry; update the loop (after
ExtractUtf8Label and before returning) to mirror the forward-resolution checks:
call ResolveWrappedDomainOwner (or the equivalent forward-resolution helper) for
the candidate nameAccount.Key to obtain the resolved owner and expiresAt, verify
the resolved owner matches the requested owner (handling wrapped/NFT owner PDAs)
and that expiresAt is in the future, and only then return the label; if
verification fails, continue the loop. If scanning all children is too
expensive, use the canonical ANS reverse index but still perform the same
ResolveWrappedDomainOwner and expiry checks on each candidate before returning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fec77a6b-0efa-445d-a18d-2382123e4a56
📒 Files selected for processing (5)
Samples~/Solana Wallet/Scripts/example/screens/ReceiveScreen.csSamples~/Solana Wallet/Scripts/example/screens/TransferScreen.csSamples~/Solana Wallet/Scripts/example/services.metaSamples~/Solana Wallet/Scripts/example/services/SkrAddressResolutionClient.csSamples~/Solana Wallet/Scripts/example/services/SkrAddressResolutionClient.cs.meta
Address CodeRabbit findings by guarding async receive UI updates, validating recipient/amount flow across transfer paths, and tightening resolver behavior for failures, strict .skr parsing, and reverse lookup ownership/expiry checks.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Samples~/Solana Wallet/Scripts/example/services/SkrAddressResolutionClient.cs (1)
149-153:⚠️ Potential issue | 🟠 MajorAvoid a Mainnet-wide
.skrscan for receive-screen reverse lookup.This filter only bounds by the
.skrparent, soResolveAddressToDomainOnChain()can fetch every.skrchild account and then perform per-account RPC reads until it finds a match. On mobile/public Mainnet RPC, wallets without an early match can hit timeouts, rate limits, or very slow receive-screen loads. Prefer a bounded on-chain reverse index/candidate lookup, then keep the ownership/expiry verification already added below.Please verify reverse lookup latency and RPC request volume against a Mainnet wallet with no
.skrand one with a late-match.skr; expected behavior should stay bounded and not depend on the total number of.skrdomains.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Samples`~/Solana Wallet/Scripts/example/services/SkrAddressResolutionClient.cs around lines 149 - 153, ResolveAddressToDomainOnChain currently calls NameResolutionRpcClient.GetProgramAccountsAsync with only a parent filter (AnsProgramId and tldParentAccount) which can return all .skr children and cause unbounded RPC work; change the query to a bounded reverse-indexed candidate lookup by adding an additional MemCmp filter keyed to the recipient address (or its hashed/indexed prefix) so the RPC returns only candidates for that specific wallet before doing per-account reads, then keep the existing ownership and expiry checks afterwards; update the GetProgramAccountsAsync call and the memCmpList construction used by ResolveAddressToDomainOnChain (and any helper that builds filters for AnsProgramId/tldParentAccount) to include the address-specific filter so mainnet reverse-lookups are bounded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Samples`~/Solana Wallet/Scripts/example/screens/TransferScreen.cs:
- Around line 52-60: CheckInput currently uses direct parses that throw and
doesn't reject zero/negative values; update CheckInput to use a safe parse
(e.g., decimal.TryParse or double.TryParse with appropriate
NumberStyles/CultureInfo) for the amount field, return false and set errorTxt
when parsing fails, and explicitly verify parsedAmount > 0 (reject
zero/negative) before allowing TransferSol or TransferToken to run; keep the
conditional flow around _transferTokenAccount/TransferSol/TransferToken but rely
on the new CheckInput behavior so invalid or non-numeric amounts never reach
TransferSol or TransferToken.
In `@Samples`~/Solana
Wallet/Scripts/example/services/SkrAddressResolutionClient.cs:
- Around line 194-198: The code returns a reverse-extracted label without
ensuring that label + SuffixSkr forward-resolves to the same name account that
was verified; update the logic in SkrAddressResolutionClient (around
ExtractUtf8Label and where nameAccount is defined) to construct the candidate
name (e.g., $"{label}.{SuffixSkr}"), derive or lookup its nameAccount/public
key, perform the same owner/account verification used earlier, and only return
the label if the derived/forward-resolved nameAccount matches the original
verified nameAccount; otherwise continue/ignore the reverse record.
---
Duplicate comments:
In `@Samples`~/Solana
Wallet/Scripts/example/services/SkrAddressResolutionClient.cs:
- Around line 149-153: ResolveAddressToDomainOnChain currently calls
NameResolutionRpcClient.GetProgramAccountsAsync with only a parent filter
(AnsProgramId and tldParentAccount) which can return all .skr children and cause
unbounded RPC work; change the query to a bounded reverse-indexed candidate
lookup by adding an additional MemCmp filter keyed to the recipient address (or
its hashed/indexed prefix) so the RPC returns only candidates for that specific
wallet before doing per-account reads, then keep the existing ownership and
expiry checks afterwards; update the GetProgramAccountsAsync call and the
memCmpList construction used by ResolveAddressToDomainOnChain (and any helper
that builds filters for AnsProgramId/tldParentAccount) to include the
address-specific filter so mainnet reverse-lookups are bounded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f80a7ed7-d4ee-47e0-b997-5830232754d2
📒 Files selected for processing (3)
Samples~/Solana Wallet/Scripts/example/screens/ReceiveScreen.csSamples~/Solana Wallet/Scripts/example/screens/TransferScreen.csSamples~/Solana Wallet/Scripts/example/services/SkrAddressResolutionClient.cs
Address remaining CodeRabbit feedback by preventing invalid transfer amounts from reaching send paths and hardening reverse .skr lookup with owner-bounded filtering plus forward-name consistency validation.
Contributor
Author
|
@Kuldotha hey, i tightened many tests after working on the coderabbit checks. requesting your review |
Kuldotha
reviewed
Apr 28, 2026
| @@ -0,0 +1,351 @@ | |||
| using System; | |||
Contributor
There was a problem hiding this comment.
This file shouldn't be under samples, otherwise people can only use it when having the samples in their project. Or is this entire file meant to be a sample setup?
Contributor
Author
There was a problem hiding this comment.
@Kuldotha great callout. agreed this should be reusable outside samples. i’ll move the resolver into runtime so apps can use it without importing sample assets, and keep the sample side as a thin consumer. i’ll push an update in this pr.
Expose SkrAddressResolutionClient under Solana.Unity.SDK so apps can resolve .skr without importing sample assets. Sample wallet screens now reference the runtime type; sample-only services folder removed per review feedback.
Contributor
Author
|
@Kuldotha i've done the necessary changes. waiting for your feedback |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Samples~/Solana Wallet/Scripts/example/screens/ReceiveScreen.cs (1)
107-111: 🧹 Nitpick | 🔵 TrivialClipboard copies raw public key, not displayed domain.
After reverse lookup,
publicKey_txt.textshows the.skrdomain, butCopyPublicKeyToClipboardalways copies the raw public key. This is arguably correct (public keys are more universally accepted), but could confuse users who expect to copy what they see.Consider either:
- Keeping current behavior with a toast like "Public Key copied" (already done)
- Adding a separate "Copy domain" option if needed
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Samples`~/Solana Wallet/Scripts/example/screens/ReceiveScreen.cs around lines 107 - 111, The CopyPublicKeyToClipboard method currently always copies the raw key via Clipboard.Copy(Web3.Instance.WalletBase.Account.PublicKey.ToString()), which differs from the displayed value in publicKey_txt.text when a .skr domain is shown; update CopyPublicKeyToClipboard so it copies publicKey_txt.text if it contains a displayed domain (e.g., endsWith ".skr" or is non-empty and not the raw key), otherwise fall back to the raw public key, and update the toast text to a neutral "Copied to clipboard" or include which form was copied; reference CopyPublicKeyToClipboard, publicKey_txt, Clipboard.Copy, and Web3.Instance.WalletBase.Account.PublicKey.ToString() when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Runtime/codebase/SkrAddressResolutionClient.cs`:
- Around line 41-42: The NameResolutionRpcClient field is hardcoded to
"https://api.mainnet-beta.solana.com", limiting configurability; change it from
a private static readonly initialization to a configurable static property or an
initialization method so callers can set a custom RPC endpoint before use.
Replace the current private static readonly IRpcClient NameResolutionRpcClient =
ClientFactory.GetClient(...) with either a private static IRpcClient
_nameResolutionRpcClient plus a public static property NameResolutionRpcClient {
get; set; } (or a public static InitializeNameResolutionClient(string url) that
calls ClientFactory.GetClient(url)), ensure existing usages read the
property/field, and add null/validation checks so attempting to use it without
initialization throws a clear exception.
- Around line 110-111: The code currently extracts slices like "var ownerBytes =
rawData.Skip(OwnerOffset).Take(32).ToArray();" which allocates intermediate
enumerables and arrays; replace these with a lower-allocation approach (e.g.,
use Array.Copy(rawData, OwnerOffset, ownerBytes, 0, 32) with a pre-sized
ownerBytes = new byte[32] or use Span<byte> ownerSpan =
rawData.AsSpan(OwnerOffset, 32)) to avoid the Skip/Take/ToArray chain; make the
same change for the other slices referenced in the file (the extractions at the
locations using offsets on rawData on lines analogous to 182, 213, 276, and
296), ensuring you preserve the All(b => b == 0) check by operating on the Span
or the copied fixed-size array.
- Around line 49-56: The catch block in the address resolution method swallows
all exceptions and returns null, which hampers debugging; update the method(s)
that call ResolveDomainToAddressOnChain(domain) to log the caught Exception
before returning null—preferably by injecting an
ILogger<SkrAddressResolutionClient> (e.g. a private readonly _logger) into the
SkrAddressResolutionClient class and replacing the empty catch with catch
(Exception ex) { _logger?.LogDebug(ex, "ResolveDomainToAddressOnChain failed for
domain '{Domain}'", domain); return null; } so you preserve the null-return
behavior while recording exception details for troubleshooting.
- Around line 222-229: The code assumes Data[0] is base64 when calling
Convert.FromBase64String on encodedData from
NameResolutionRpcClient.GetAccountInfoAsync; instead, check the RPC response's
Data array for an encoding indicator (typically Data[1]) and only call
Convert.FromBase64String when that encoding equals "base64" (or handle other
encodings appropriately), otherwise decode using the correct scheme or return an
empty byte[]/throw a clear exception; update the logic around
response?.Result?.Value?.Data, encodedData, and the Convert.FromBase64String
call to perform this defensive encoding check.
- Around line 149-156: The Owner memcmp filter (using OwnerOffset = 40) passed
into NameResolutionRpcClient.GetProgramAccountsAsync is excluding wrapped
domains because wrapped name records store the NFT PDA as the owner; remove the
owner memcmp from the filters list so queries use only the tldParentAccount
filter (AnsProgramId, memCmpList without the owner filter), then after fetching
accounts validate ownership per-account by calling
ResolveWrappedDomainOwner/your existing ownership-check logic to determine the
true wallet owner. Ensure references to OwnerOffset and the owner memcmp are
removed so wrapped domain records are returned and processed.
In `@Samples`~/Solana Wallet/Scripts/example/screens/TransferScreen.cs:
- Around line 188-192: The CheckInput code is fragile because it reparses
ownedAmountTxt.text; instead store the raw token balance when you populate the
UI (introduce a field like _ownedTokenAmount alongside the existing
_ownedSolAmount) and set it at the same time you set ownedAmountTxt.text, then
update CheckInput to use _ownedTokenAmount rather than parsing
ownedAmountTxt.text; update any places that relied on parsing to use the new
_ownedTokenAmount and remove the ulong.TryParse call around ownedAmountTxt.text.
---
Outside diff comments:
In `@Samples`~/Solana Wallet/Scripts/example/screens/ReceiveScreen.cs:
- Around line 107-111: The CopyPublicKeyToClipboard method currently always
copies the raw key via
Clipboard.Copy(Web3.Instance.WalletBase.Account.PublicKey.ToString()), which
differs from the displayed value in publicKey_txt.text when a .skr domain is
shown; update CopyPublicKeyToClipboard so it copies publicKey_txt.text if it
contains a displayed domain (e.g., endsWith ".skr" or is non-empty and not the
raw key), otherwise fall back to the raw public key, and update the toast text
to a neutral "Copied to clipboard" or include which form was copied; reference
CopyPublicKeyToClipboard, publicKey_txt, Clipboard.Copy, and
Web3.Instance.WalletBase.Account.PublicKey.ToString() when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98d428ef-3976-477c-bb68-f42c33f2ebf1
📒 Files selected for processing (4)
Runtime/codebase/SkrAddressResolutionClient.csRuntime/codebase/SkrAddressResolutionClient.cs.metaSamples~/Solana Wallet/Scripts/example/screens/ReceiveScreen.csSamples~/Solana Wallet/Scripts/example/screens/TransferScreen.cs
Comment on lines
+41
to
+42
| private static readonly IRpcClient NameResolutionRpcClient = | ||
| ClientFactory.GetClient("https://api.mainnet-beta.solana.com"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded mainnet RPC endpoint limits flexibility.
The static NameResolutionRpcClient uses a hardcoded mainnet URL. This prevents users from configuring custom RPC endpoints (e.g., for rate limits, private nodes, or testing). Consider making this configurable via a static property or initialization method.
♻️ Suggested approach
- private static readonly IRpcClient NameResolutionRpcClient =
- ClientFactory.GetClient("https://api.mainnet-beta.solana.com");
+ private static IRpcClient _nameResolutionRpcClient;
+ private static IRpcClient NameResolutionRpcClient =>
+ _nameResolutionRpcClient ??= ClientFactory.GetClient(DefaultMainnetRpcUrl);
+
+ private const string DefaultMainnetRpcUrl = "https://api.mainnet-beta.solana.com";
+
+ /// <summary>
+ /// Configure a custom RPC client for name resolution queries.
+ /// </summary>
+ public static void SetRpcClient(IRpcClient client)
+ {
+ _nameResolutionRpcClient = client;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private static readonly IRpcClient NameResolutionRpcClient = | |
| ClientFactory.GetClient("https://api.mainnet-beta.solana.com"); | |
| private static IRpcClient _nameResolutionRpcClient; | |
| private static IRpcClient NameResolutionRpcClient => | |
| _nameResolutionRpcClient ??= ClientFactory.GetClient(DefaultMainnetRpcUrl); | |
| private const string DefaultMainnetRpcUrl = "https://api.mainnet-beta.solana.com"; | |
| /// <summary> | |
| /// Configure a custom RPC client for name resolution queries. | |
| /// </summary> | |
| public static void SetRpcClient(IRpcClient client) | |
| { | |
| _nameResolutionRpcClient = client; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Runtime/codebase/SkrAddressResolutionClient.cs` around lines 41 - 42, The
NameResolutionRpcClient field is hardcoded to
"https://api.mainnet-beta.solana.com", limiting configurability; change it from
a private static readonly initialization to a configurable static property or an
initialization method so callers can set a custom RPC endpoint before use.
Replace the current private static readonly IRpcClient NameResolutionRpcClient =
ClientFactory.GetClient(...) with either a private static IRpcClient
_nameResolutionRpcClient plus a public static property NameResolutionRpcClient {
get; set; } (or a public static InitializeNameResolutionClient(string url) that
calls ClientFactory.GetClient(url)), ensure existing usages read the
property/field, and add null/validation checks so attempting to use it without
initialization throws a clear exception.
Comment on lines
+49
to
+56
| try | ||
| { | ||
| return await ResolveDomainToAddressOnChain(domain); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent exception swallowing hinders debugging.
Both public methods catch all exceptions and return null without any logging. While graceful failure is appropriate for resolution APIs, consider adding optional debug logging to aid troubleshooting connectivity or parsing issues.
♻️ Optional: Add conditional debug logging
try
{
return await ResolveDomainToAddressOnChain(domain);
}
catch (Exception)
{
+#if UNITY_EDITOR
+ UnityEngine.Debug.LogWarning($"[SKR] Forward resolution failed for '{domain}': {ex.Message}");
+#endif
return null;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Runtime/codebase/SkrAddressResolutionClient.cs` around lines 49 - 56, The
catch block in the address resolution method swallows all exceptions and returns
null, which hampers debugging; update the method(s) that call
ResolveDomainToAddressOnChain(domain) to log the caught Exception before
returning null—preferably by injecting an ILogger<SkrAddressResolutionClient>
(e.g. a private readonly _logger) into the SkrAddressResolutionClient class and
replacing the empty catch with catch (Exception ex) { _logger?.LogDebug(ex,
"ResolveDomainToAddressOnChain failed for domain '{Domain}'", domain); return
null; } so you preserve the null-return behavior while recording exception
details for troubleshooting.
Comment on lines
+110
to
+111
| var ownerBytes = rawData.Skip(OwnerOffset).Take(32).ToArray(); | ||
| if (ownerBytes.All(b => b == 0)) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reduce allocations in byte extraction.
Skip().Take().ToArray() creates intermediate enumerators and allocates a new array. For a method that may be called frequently, consider using Array.Copy or span-based slicing.
♻️ Lower-allocation alternative
- var ownerBytes = rawData.Skip(OwnerOffset).Take(32).ToArray();
+ var ownerBytes = new byte[PublicKeyLength];
+ Array.Copy(rawData, OwnerOffset, ownerBytes, 0, PublicKeyLength);Apply similar changes to lines 182, 213, 276, and 296.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Runtime/codebase/SkrAddressResolutionClient.cs` around lines 110 - 111, The
code currently extracts slices like "var ownerBytes =
rawData.Skip(OwnerOffset).Take(32).ToArray();" which allocates intermediate
enumerables and arrays; replace these with a lower-allocation approach (e.g.,
use Array.Copy(rawData, OwnerOffset, ownerBytes, 0, 32) with a pre-sized
ownerBytes = new byte[32] or use Span<byte> ownerSpan =
rawData.AsSpan(OwnerOffset, 32)) to avoid the Skip/Take/ToArray chain; make the
same change for the other slices referenced in the file (the extractions at the
locations using offsets on rawData on lines analogous to 182, 213, 276, and
296), ensuring you preserve the All(b => b == 0) check by operating on the Span
or the copied fixed-size array.
Address follow-up review feedback by allowing wrapped-domain candidates in reverse lookups, adding defensive account-data encoding checks, and using stored raw token balance values instead of parsing UI text.
Contributor
Author
before:
now:
so it’s mainly a correctness/safety fix for token transfers and addresses the CodeRabbit concern.
|
Contributor
|
@lopeselio I'm testing the system but it looks like the callback isn't called relyably. Also, does it make sense to cache reverse lookups so we don't have to do another reverse lookup? Better to limit the amount of getprogramaccounts calls. Edit, it looks like the callback is returned properly but the result isn't consistently filled, looks like a race condition (result is good when debugging but empty at runtime) |
Improve receive-flow reliability by deduplicating in-flight reverse lookups, adding short-lived reverse lookup caching, and guarding UI updates against stale async completions.
Contributor
Author
thanks for testing and for the detailed feedback, i pushed updates to address both points.
this should reduce getprogramaccounts pressure and avoid the race behavior where result appears empty at runtime. cc: @Kuldotha |
Contributor
Author
|
@Kuldotha please let me know if we can add helpful information about the feature in the project readme as well. |
Contributor
Author
|
Hey @Kuldotha , just checking back here to see if we have any updates, thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With recent work in this repo focused on Solana Mobile Wallet Adapter support for Seeker, it makes sense to improve identity UX for the same audience by adding native
.skrresolution in the SDK.A large and growing number of Seeker users interact with apps built on Solana Unity SDK, including games like Cryptara Conquest. In these experiences, users should be able to transfer assets, complete purchases, and trade with other players using human-readable
.skrANS names (AllDomains), instead of manually copying and pasting wallet addresses.This would significantly reduce friction, improve usability on mobile, and better align the SDK with how Seeker users actually interact on-chain.
Support .skr name resolution for transfer and receive flows without requiring a backend service. Resolve wrapped domains to the actual NFT holder and query mainnet for ANS lookups to improve Seeker mobile UX.
check video below ⬇️
Refs #281
Closes #281
Problem
The sample wallet currently expects raw base58 addresses for transfers and does not natively resolve
.skrnames.This creates friction for Seeker/mobile users and fails common UX expectations for human-readable identity flows.
Additionally:
.skrout of the box.skridentitySolution
Implemented native on-chain ANS resolution in sample wallet flows:
SkrAddressResolutionClientin sample scripts for on-chain.skrlookups.skrin recipient field.skr -> walletbefore transaction submissionaddress -> .skr.skrwhen available, otherwise falls back to pubkey.skrlives)Before & After Screenshots
BEFORE:
.skrfails / unresolved behaviorAFTER:
.skrresolves and transfer proceeds.skrshowsUnable to resolve <domain>and aborts submission.skrwhen availablePR_SKR.-.Large.540p.mov
0.001 SOL transfer from lopeselio.skr -> magellan.skr tx on devnet
Other changes (e.g. bug fixes, small refactors)
Deploy Notes
No backend/service deployment required for
.skrresolution in this feature.New scripts:
Samples~/Solana Wallet/Scripts/example/services/SkrAddressResolutionClient.cs: native on-chain.skrresolution service for transfer and receive flowsNew dependencies:
Summary by CodeRabbit
New Features
Bug Fixes / UX