Skip to content

feat: integrate sns#441

Merged
aganglada merged 6 commits into
mainfrom
NWNT-395-snap-integrate-sns-on-send
Jul 2, 2025
Merged

feat: integrate sns#441
aganglada merged 6 commits into
mainfrom
NWNT-395-snap-integrate-sns-on-send

Conversation

@aganglada

Copy link
Copy Markdown
Contributor
  • Integrate SNS in onNameLookup
  • Resolve domains on the Send flow
Screenshot 2025-07-01 at 15 47 15 Screenshot 2025-07-01 at 15 47 26

Not in scope:

  • Showing .sol domain in confirmations (There is a Snap UI limitation right now)

@aganglada aganglada self-assigned this Jul 1, 2025
@socket-security

socket-security Bot commented Jul 1, 2025

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​solana-name-service/​sns-sdk-kit@​0.9.0-beta751008177100

View full report

cursor[bot]

This comment was marked as outdated.

@aganglada
aganglada force-pushed the NWNT-395-snap-integrate-sns-on-send branch from 56ca549 to 94e4449 Compare July 1, 2025 15:40
Comment thread packages/snap/src/core/handlers/onNameLookup/structs.ts Outdated
Comment thread packages/snap/src/features/send/utils/isSolanaDomain.test.ts Outdated
@aganglada
aganglada force-pushed the NWNT-395-snap-integrate-sns-on-send branch from c13b8ee to 84f7788 Compare July 2, 2025 09:23
cursor[bot]

This comment was marked as outdated.

ulissesferreira
ulissesferreira previously approved these changes Jul 2, 2025
ulissesferreira
ulissesferreira previously approved these changes Jul 2, 2025
cursor[bot]

This comment was marked as outdated.

@aganglada
aganglada force-pushed the NWNT-395-snap-integrate-sns-on-send branch from a57c3b0 to d8053a8 Compare July 2, 2025 10:27
@aganglada
aganglada merged commit 05f35a0 into main Jul 2, 2025
8 checks passed

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Domain Resolution Error Not Handled

The if (destinationValidation === null) condition checks the initial format validation result, not the final state after domain resolution. If a syntactically valid .sol domain fails to resolve, destinationValidation remains null while updatedContext.validation[SendFormNames.DestinationAccountInput] is updated with an error. This leads to buildTransactionMessageAndUpdateInterface being called with an unresolved domain. The condition should check updatedContext.validation[SendFormNames.DestinationAccountInput] === null.

packages/snap/src/features/send/views/SendForm/events.tsx#L400-L404

await updateInterface(id, <Send context={updatedContext} />, updatedContext);
if (destinationValidation === null) {
await buildTransactionMessageAndUpdateInterface(id, updatedContext);
}

Fix in Cursor


Bug: Incorrect Domain Resolution in `resolveAddress` Method

The resolveAddress method incorrectly returns domainAddress from the getPrimaryDomain result. Intended to resolve an address to its domain name, it should return domainName. Returning domainAddress is semantically incorrect and likely stems from a misunderstanding of the external library API.

packages/snap/src/core/services/name-resolution/NameResolutionService.ts#L22-L27

async resolveAddress(scope: Network, address: string): Promise<string> {
const connection = this.#connection.getRpc(scope);
return (await getPrimaryDomain(connection, asAddress(address)))
.domainAddress;
}

Fix in Cursor


Was this report helpful? Give feedback by reacting with 👍 or 👎

@aganglada
aganglada deleted the NWNT-395-snap-integrate-sns-on-send branch July 2, 2025 10:31
@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2025

Copy link
Copy Markdown

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.

3 participants