-
Notifications
You must be signed in to change notification settings - Fork 183
[Stellar] add token URI setting #725
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Eric Lau <ericglau@outlook.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe PR adds a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/stellar/src/non-fungible.ts (1)
16-29: Consider adding protocol to default tokenUri.The default
tokenUrivalue'www.mytoken.com'lacks a protocol (http:// or https://), while test examples use'https://example.com/nft/'. For consistency and to avoid potential issues with URI consumers, consider using a fully-qualified URL as the default.- tokenUri: 'www.mytoken.com', + tokenUri: 'https://www.mytoken.com',
🧹 Nitpick comments (2)
packages/mcp/src/stellar/schemas.ts (1)
60-60: Consider adding URL format validation.The tokenUri field is correctly added as an optional string with the appropriate description. However, you might consider adding URL format validation using Zod's
.url()method to catch malformed URIs early:- tokenUri: z.string().optional().describe(stellarNonFungibleDescriptions.tokenUri), + tokenUri: z.string().url().optional().describe(stellarNonFungibleDescriptions.tokenUri),This would provide better input validation and clearer error messages when users provide invalid URIs.
packages/mcp/src/stellar/tools/non-fungible.test.ts (1)
42-65: Consider adding a test case for default tokenUri behavior.The comprehensive test correctly includes
tokenUri. However, consider adding a test case that omitstokenUrito verify that the default value is properly applied, ensuring backward compatibility.Example test case to add in
non-fungible.test.ts:test('default tokenUri', async t => { const params: z.infer<typeof t.context.schema> = { name: 'TestToken', symbol: 'TST', // tokenUri intentionally omitted to test default }; await assertAPIEquivalence(t, params, nonFungible.print); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/core/stellar/src/non-fungible.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
packages/common/src/ai/descriptions/stellar.ts(1 hunks)packages/core/stellar/src/generate/non-fungible.ts(1 hunks)packages/core/stellar/src/non-fungible.test.ts(1 hunks)packages/core/stellar/src/non-fungible.test.ts.md(1 hunks)packages/core/stellar/src/non-fungible.ts(5 hunks)packages/mcp/src/stellar/schemas.ts(1 hunks)packages/mcp/src/stellar/tools/non-fungible.test.ts(2 hunks)packages/mcp/src/stellar/tools/non-fungible.ts(1 hunks)packages/ui/api/ai-assistant/function-definitions/stellar.ts(1 hunks)packages/ui/src/stellar/NonFungibleControls.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T20:17:09.709Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 652
File: packages/mcp/src/confidential/schemas.ts:40-46
Timestamp: 2025-09-18T20:17:09.709Z
Learning: MCP tool schemas for confidential contracts can be more restrictive than the core API. For the votes field in confidential fungible tokens, the MCP schema only accepts 'blocknumber' or 'timestamp' strings because undefined behaves functionally equivalent to false, eliminating the need to explicitly accept boolean values in the tool definition.
Applied to files:
packages/mcp/src/stellar/schemas.ts
🧬 Code graph analysis (3)
packages/mcp/src/stellar/schemas.ts (1)
packages/common/src/ai/descriptions/stellar.ts (1)
stellarNonFungibleDescriptions(21-26)
packages/mcp/src/stellar/tools/non-fungible.ts (1)
packages/core/stellar/src/non-fungible.ts (1)
NonFungibleOptions(35-46)
packages/ui/api/ai-assistant/function-definitions/stellar.ts (1)
packages/common/src/ai/descriptions/stellar.ts (1)
stellarNonFungibleDescriptions(21-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (stellar, compile)
- GitHub Check: build (solidity, default)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (11)
packages/core/stellar/src/non-fungible.test.ts.md (1)
912-943: LGTM!The new snapshot correctly demonstrates a Non-Fungible token contract with a custom token URI. The generated code structure is consistent with other test snapshots, and the URI format includes the protocol and trailing slash as expected for NFT metadata.
packages/core/stellar/src/non-fungible.test.ts (1)
156-158: LGTM!The test case correctly validates the custom token URI functionality by passing the tokenUri option and snapshotting the generated contract. The test placement and structure align with existing test patterns in the file.
packages/mcp/src/stellar/tools/non-fungible.ts (1)
13-29: LGTM!The tokenUri parameter is correctly added to the MCP tool callback, destructured from the input, and passed through to the NonFungibleOptions object. The implementation is consistent with other optional parameters in the tool.
packages/common/src/ai/descriptions/stellar.ts (1)
25-25: LGTM!The description for tokenUri is clear and accurately explains that it's the metadata URI returned by the token contract for each NFT. The wording is consistent with other descriptions in the file.
packages/ui/src/stellar/NonFungibleControls.svelte (1)
54-60: LGTM!The Token URI input field is well-implemented with proper data binding, error handling, and user guidance. The help tooltip clearly explains the purpose, and the placeholder suggests the expected URL format.
packages/ui/api/ai-assistant/function-definitions/stellar.ts (1)
102-105: LGTM!The tokenUri parameter is correctly added to the AI function definition with the appropriate type and description. The field is properly marked as optional, which maintains backward compatibility with existing integrations.
packages/mcp/src/stellar/tools/non-fungible.test.ts (1)
33-40: LGTM! Test coverage for tokenUri added correctly.The test correctly includes the new
tokenUrifield in the basic test case, following the same pattern asnameandsymbol.packages/core/stellar/src/non-fungible.ts (4)
35-46: LGTM! Interface updated correctly.The
tokenUrifield is appropriately added as an optional string parameter, maintaining backward compatibility.
48-61: LGTM! Default handling implemented correctly.The
tokenUrifield properly falls back to the default value using the nullish coalescing operator, consistent with other fields.
67-123: LGTM! tokenUri properly propagated through build flow.The
tokenUriis correctly passed toaddBaseafter conversion viatoByteArray, following the same pattern asnameandsymbolfields.
125-159: LGTM! Function signature and usage updated correctly.The
addBasefunction signature correctly includes thetokenUriparameter, and the generated Rust code properly uses it in the metadata initialization. The implementation follows the established pattern fornameandsymbolfields.
| const blueprint = { | ||
| name: ['MyToken'], | ||
| symbol: ['MTK'], | ||
| tokenUri: ['www.mytoken.com'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent URI format in default value.
The default tokenUri value 'www.mytoken.com' lacks the protocol prefix, while the test example uses 'https://example.com/nfts/' and the UI placeholder suggests 'https://...'. This inconsistency may confuse users about the expected URI format.
Consider using a consistent format with the protocol included:
- tokenUri: ['www.mytoken.com'],
+ tokenUri: ['https://www.mytoken.com'],This aligns with the test example and UI guidance, making it clearer that full URIs (including protocol) are expected.
📝 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.
| tokenUri: ['www.mytoken.com'], | |
| tokenUri: ['https://www.mytoken.com'], |
🤖 Prompt for AI Agents
In packages/core/stellar/src/generate/non-fungible.ts around line 11, the
default tokenUri value is 'www.mytoken.com' which omits the protocol and is
inconsistent with tests and UI placeholders; update the default to a fully
qualified URI including the protocol (e.g., 'https://example.com/nfts/' or
'https://www.mytoken.com') so it matches the expected format across tests and UI
guidance.
Co-authored-by: Eric Lau <ericglau@outlook.com>
…d into stellar-token-uri
Fixes #707
Summary
Expose a configurable
tokenUrifor Stellar non-fungible contracts across the generator,tooling, and UI so deployments can mint NFTs with accurate metadata.
Description
tokenUri, it flows throughbuildNonFungibleandaddBaseso metadata is set from the provided value (
packages/core/stellar/src/non-fungible.ts:19,packages/core/stellar/src/non-fungible.ts:67,packages/core/stellar/src/non-fungible.ts:125).definition all surface the new field (
packages/core/stellar/src/generate/non-fungible.ts:11,packages/common/src/ai/descriptions/stellar.ts:25,packages/mcp/src/stellar/schemas.ts:60,packages/mcp/src/stellar/tools/non-fungible.ts:16,packages/ui/api/ai-assistant/function-definitions/stellar.ts:102).(
packages/ui/src/stellar/NonFungibleControls.svelte:56).(
packages/core/stellar/src/non-fungible.test.ts:156,packages/mcp/src/stellar/tools/non-fungible.test.ts:37).Motivation / Context
Developers need Soroban NFT contracts to advertise the correct metadata host. Allowing
tokenUrito pass through every surface removes the hard-coded placeholder and keeps wizard output accurate.
Changes
tokenUriparameter throughout the Stellar NFT builder, tooling, AI, and UI.Affected functions
packages/core/stellar/src/non-fungible.ts:67–buildNonFungiblenow forwardstokenUriinto the contract builder.
packages/core/stellar/src/non-fungible.ts:125–addBasewrites metadata from theuser-specified URI instead of a literal string.
packages/mcp/src/stellar/tools/non-fungible.ts:8–registerStellarNonFungibleaccepts the new argument so MCP requests round-trip it.
packages/mcp/src/stellar/schemas.ts:60–nonFungibleSchemavalidatestokenUri,enabling AI/MCP prompts to describe it.
packages/ui/src/stellar/NonFungibleControls.svelte:56– UI exposes a bound input sowizard users can supply the URI.
Security Impact
✅ No security-impacting changes; only metadata strings are forwarded to contract initialization.
✅ No additional security-sensitive code paths were introduced.
Testing
pnpm test --filter "packages/core/stellar -- non-fungible"(covers the newnon-fungible custom token urisnapshot) – not run in this environment.pnpm test --filter "packages/mcp -- non-fungible"– not run in this environment.contract, and confirm the emitted Soroban code sets that URI in
Base::set_metadata.Backward Compatibility
tokenUriis optional with a default identical to prior behavior.