Skip to content

Conversation

@CoveMB
Copy link
Contributor

@CoveMB CoveMB commented Nov 11, 2025

Fixes #706

Summary

Enable UI and tooling to use explicit Soroban trait implementations instead of relying solely on #[default_impl], adding regression coverage and docs for the new option.

Motivation / Context

Contract authors asked for a way to inspect and customize every generated Soroban trait function instead of depending on #[default_impl]. This work exposes that capability in the builders, UI, and MCP tools without changing the default experience.

Changes

  • Major feature: optional explicit trait implementations throughout Stellar contract builders, plus access-control and token module plumbing updates.
  • Config / tooling: UI control section, AI descriptions, schema, and MCP tool changes so the new option can be toggled and validated end-to-end.

Description

  • packages/core/stellar/src/common-options.ts:10-36 and the builder defaults now carry an explicitImplementations flag so every contract kind can opt in to non-macro trait bodies.
  • packages/core/stellar/src/contract.ts:188-192 introduces addTraitForEachFunctions, letting trait mixins add all functions programmatically when macros are disabled.
  • Access-control helpers (packages/core/stellar/src/set-access-control.ts:18-235, packages/core/stellar/src/add-pausable.ts:4-44, packages/core/stellar/src/add-upgradeable.ts:4-45) accept the flag, generate Ownable/AccessControl function bodies, and still enforce the same role/owner checks.
  • Token builders propagate the choice through every feature (packages/core/stellar/src/fungible.ts:25-210, packages/core/stellar/src/non-fungible.ts:28-305, packages/core/stellar/src/stablecoin.ts:48-101) and update generation blueprints so option sweeps include the new mode.
  • UI/tooling surfaces the toggle (packages/ui/src/stellar/TraitImplementationSection.svelte:1-33 plus imports in each controls file) while schemas/prompts describe and validate it (packages/mcp/src/stellar/schemas.ts:24-35, packages/mcp/src/stellar/tools/fungible.ts:10-40, packages/common/src/ai/descriptions/stellar.ts:12-20).
  • New compile tests, scenario docs, and MCP tool suites cover explicit builds across fungible, non-fungible, and stablecoin variants (packages/core/stellar/src/*.compile.test.ts, packages/core/stellar/src/*.test.ts / .md, packages/mcp/src/stellar/tools/*.test.ts).

Affected Functions

  • packages/core/stellar/src/contract.ts:188 — new addTraitForEachFunctions helper to bulk-attach trait methods when macros are skipped.
  • packages/core/stellar/src/set-access-control.ts:18setAccessControl accepts the explicit flag and adds per-function trait impls for Ownable/AccessControl when needed.
  • packages/core/stellar/src/set-access-control.ts:84requireAccessControl forwards the explicit flag so pausable/upgradeable traits stay consistent.
  • packages/core/stellar/src/add-pausable.ts:4 & packages/core/stellar/src/add-upgradeable.ts:4 — feature mixins now require the explicit flag and pass it through to access control.
  • packages/core/stellar/src/fungible.ts:63 & :137 — base, burnable, and mintable helpers emit explicit trait functions when requested.
  • packages/core/stellar/src/non-fungible.ts:91 & :212 — non-fungible base, burnable/enumerable/consecutive modules support explicit implementations.
  • packages/core/stellar/src/stablecoin.ts:48 — limitation helpers forward the flag so allow/blocklist operations can be emitted explicitly.

Security Impact

  • ✅ No new security concerns; explicit implementations call the same helper libraries and still rely on requireAccessControl for gating.
  • Security-sensitive modifications: not applicable.

Testing

  • Added compile regressions for each contract family with explicitImplementations enabled (packages/core/stellar/src/fungible.compile.test.ts, non-fungible.compile.test.ts, stablecoin.compile.test.ts).
  • Added scenario/API tests, markdown snapshots, and MCP tool coverage for the new option (packages/core/stellar/src/*.test.ts, .md, .snap, and packages/mcp/src/stellar/tools/*.test.ts).

Manual verification:

  1. In the UI, toggle "Explicit trait methods" for each contract type and confirm the preview drops #[default_impl] and shows explicit bodies.
  2. Run pnpm test --filter='packages/core/stellar/src/*explicit*' and the relevant MCP tool tests to ensure the new flag compiles cleanly.

Backward Compatibility

  • ✅ No breaking changes; the wizard still uses #[default_impl] unless the new toggle is enabled.
  • ✅ No migration needed; existing saved configs remain valid and ignore the new option.

@CoveMB CoveMB requested review from a team as code owners November 11, 2025 19:15
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The PR introduces an explicitImplementations boolean configuration option enabling contracts to use explicit trait implementations instead of the #[default_impl] macro. This flag is propagated through contract builders and conditionally adjusts code generation, imports, and trait wiring across fungible, non-fungible, and stablecoin contracts, with corresponding UI and test coverage.

Changes

Cohort / File(s) Summary
Configuration & Schemas
packages/common/src/ai/descriptions/stellar.ts, packages/core/stellar/src/common-options.ts, packages/mcp/src/stellar/schemas.ts
Added explicitImplementations property to common descriptions, CommonContractOptions interface with default value false, and MCP schema.
Access Control
packages/core/stellar/src/set-access-control.ts
Extended setAccessControl and requireAccessControl signatures to accept explicitImplementations parameter; added conditional trait wiring via defineFunctions for explicit implementations mode.
Core Builder Functions
packages/core/stellar/src/add-pausable.ts, packages/core/stellar/src/add-upgradeable.ts
Updated addPausable and addUpgradeable signatures to accept and propagate explicitImplementations parameter to requireAccessControl calls.
Contract Builder
packages/core/stellar/src/contract.ts
Added new addTraitForEachFunctions method to batch-add trait functions.
Fungible Contracts
packages/core/stellar/src/fungible.ts, packages/core/stellar/src/generate/fungible.ts
Extended buildFungible, addBase, addMintable, and addBurnable signatures; added conditional trait implementation generation and function lists (fungibleTokenTraitFunctions, fungibleBurnableFunctions); added explicitImplementations field to generator blueprint.
Non-Fungible Contracts
packages/core/stellar/src/non-fungible.ts, packages/core/stellar/src/generate/non-fungible.ts
Updated defaults to include explicitImplementations; extended builder functions to conditionally generate explicit trait implementations; added explicitImplementations to generator blueprint.
Stablecoin Contracts
packages/core/stellar/src/stablecoin.ts, packages/core/stellar/src/generate/stablecoin.ts
Extended addLimitations signature and propagated explicitImplementations through access control calls; added field to generator blueprint.
Test Cases
packages/core/stellar/src/fungible.test.ts, packages/core/stellar/src/fungible.compile.test.ts, packages/core/stellar/src/non-fungible.test.ts, packages/core/stellar/src/non-fungible.compile.test.ts, packages/core/stellar/src/stablecoin.test.ts, packages/core/stellar/src/stablecoin.compile.test.ts
Added new test cases verifying explicit trait implementations for fungible, non-fungible, and stablecoin contracts.
Test Snapshots
packages/core/stellar/src/fungible.test.ts.md, packages/core/stellar/src/non-fungible.test.ts.md, packages/core/stellar/src/stablecoin.test.ts.md
Added new snapshot blocks demonstrating generated Rust code with explicit trait implementations; updated imports to include Symbol where needed.
MCP Tools
packages/mcp/src/stellar/tools/fungible.ts, packages/mcp/src/stellar/tools/non-fungible.ts (implied), packages/mcp/src/stellar/tools/stablecoin.ts (implied)
Added explicitImplementations parameter destructuring and propagation in tool registration callbacks.
MCP Tool Tests
packages/mcp/src/stellar/tools/fungible.test.ts, packages/mcp/src/stellar/tools/non-fungible.test.ts, packages/mcp/src/stellar/tools/stablecoin.test.ts
Extended all-fields test parameters to include explicitImplementations: true.
UI Components
packages/ui/src/stellar/TraitImplementationSection.svelte
New UI component providing radio selection between default implementation and explicit trait methods modes.
UI Control Updates
packages/ui/src/stellar/FungibleControls.svelte, packages/ui/src/stellar/NonFungibleControls.svelte, packages/ui/src/stellar/StablecoinControls.svelte
Integrated TraitImplementationSection component with two-way binding to opts.explicitImplementations.

Sequence Diagram(s)

sequenceDiagram
    participant User as User/Config
    participant Builder as Contract Builder
    participant AC as Access Control
    participant Gen as Code Generator
    
    User->>Builder: buildFungible({explicitImplementations: true})
    activate Builder
    
    Builder->>Builder: addBase(c, name, symbol, pausable, true)
    note over Builder: Set trait tags to ['contractimpl']<br/>conditionally import types
    
    alt explicitImplementations = true
        Builder->>Gen: addTraitForEachFunctions(trait,<br/>fungibleTokenTraitFunctions)
        note over Gen: Generate explicit<br/>function implementations
    else explicitImplementations = false
        Builder->>Gen: Add default_impl macro
        note over Gen: Use default_impl<br/>+ contractimpl
    end
    
    Builder->>Builder: addMintable(c, access, pausable, true)
    Builder->>AC: requireAccessControl(..., true)
    activate AC
    AC->>AC: setAccessControl(..., true)
    alt explicitImplementations = true
        AC->>Gen: defineFunctions(ownableFunctions)
        AC->>Builder: addTraitForEachFunctions(trait, fns)
    else
        AC->>Gen: Use default_impl blocks
    end
    deactivate AC
    
    deactivate Builder
    
    Gen-->>User: Generated Rust code<br/>(explicit or default mode)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Complexity drivers: Conditional logic branching throughout contract builders; new parameter propagation pattern repeated across multiple function signatures; changes to trait wiring and code generation paths; new UI component with reactive state management
  • Mitigating factors: Consistent pattern application across files (homogeneous parameter additions); no changes to public API contracts beyond adding optional flag; test coverage provided
  • Areas requiring attention:
    • packages/core/stellar/src/set-access-control.ts — Dynamic function definition logic and conditional trait wiring
    • packages/core/stellar/src/fungible.ts and packages/core/stellar/src/non-fungible.ts — Conditional trait function list application and import management
    • packages/ui/src/stellar/TraitImplementationSection.svelte — Reactive bindings and state synchronization patterns
    • Snapshot files — Verify generated Rust code structure and trait method signatures match expected patterns

Possibly related issues

Suggested reviewers

  • ericglau
  • ericnordelo

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.52% 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
Title check ✅ Passed The PR title accurately and specifically describes the main feature: adding a toggle to switch between default and explicit trait implementations in Stellar contracts.
Description check ✅ Passed The PR description is comprehensive and directly related to the changeset, detailing the motivation, changes across multiple modules, security impact, testing approach, and backward compatibility.

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

@CoveMB CoveMB changed the title Add toggle to switch from default to explicit trait implementation [Stellar] Add toggle to switch from default to explicit trait implementation Nov 11, 2025
Copy link
Contributor

@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: 2

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/add-pausable.ts (1)

8-36: Drop default_impl import when explicit implementations are requested.

With explicitImplementations turned on (and pausable enabled), we still add use stellar_macros::default_impl; even though no #[default_impl] tag remains. That leaves an unused import in the generated contract, which breaks builds under the -D warnings flag (our compile suite runs with warnings-as-errors) and defeats the goal of avoiding the macro in explicit mode. Please gate the import on !explicitImplementations.

-  c.addUseClause('stellar_macros', 'default_impl');
+  if (!explicitImplementations) {
+    c.addUseClause('stellar_macros', 'default_impl');
+  }
🧹 Nitpick comments (4)
packages/ui/src/stellar/TraitImplementationSection.svelte (1)

15-19: Consider simplifying the description text.

The description is quite verbose and repeats information that's already in the radio button labels and tooltips.

Consider a more concise version:

   <p>
-    Show explicit traits to override specific functions with custom behavior instead of relying on #[default_impl] macro
-    that auto-generates stub bodies for every function in the trait that you do not explicitly implement inside that
-    impl block
+    Choose how trait methods are generated: use the #[default_impl] macro for automatic stubs, or generate explicit function bodies for full control.
   </p>
packages/core/stellar/src/stablecoin.test.ts (1)

126-128: Consider testing explicit implementations with combined features.

The test case only tests explicitImplementations: true in isolation. To ensure the feature works correctly with other options (mintable, burnable, pausable, upgradeable, access control), consider adding a combined test case similar to the "stablecoin full" test variants.

Example addition:

+testStablecoin('stablecoin explicit implementations with features', {
+  explicitImplementations: true,
+  premint: '2000',
+  access: 'roles',
+  limitations: 'allowlist',
+  burnable: true,
+  mintable: true,
+  pausable: true,
+});
packages/core/stellar/src/fungible.compile.test.ts (1)

204-216: Consider testing explicit implementations with combined features.

The compilation test only verifies explicitImplementations: true in isolation. Since this is a compilation test that verifies Rust code generation, consider adding a test case that combines explicit implementations with other features (mintable, burnable, pausable, upgradeable) to ensure the generated code compiles correctly in realistic scenarios.

Example addition:

+test.serial(
+  'compilation fungible explicit implementations with features',
+  runRustCompilationTest(
+    buildFungible,
+    {
+      kind: 'Fungible',
+      name: 'MyToken',
+      symbol: 'MTK',
+      explicitImplementations: true,
+      premint: '2000',
+      burnable: true,
+      mintable: true,
+      pausable: true,
+      upgradeable: true,
+    },
+    { snapshotResult: false },
+  ),
+);
packages/core/stellar/src/non-fungible.compile.test.ts (1)

168-187: Consider testing explicit implementations with combined features.

Similar to the fungible compilation test, this test only verifies explicitImplementations: true in isolation. Consider adding a test case that combines explicit implementations with other features to ensure comprehensive compilation coverage.

Example addition:

+test.serial(
+  'compilation nonfungible explicit implementations with features',
+  runRustCompilationTest(
+    buildNonFungible,
+    {
+      kind: 'NonFungible',
+      name: 'MyNFT',
+      symbol: 'MNFT',
+      burnable: true,
+      enumerable: true,
+      consecutive: false,
+      pausable: true,
+      upgradeable: true,
+      mintable: true,
+      sequential: true,
+      explicitImplementations: true,
+    },
+    { snapshotResult: false },
+  ),
+);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce8fbba and 3d07859.

⛔ Files ignored due to path filters (3)
  • packages/core/stellar/src/fungible.test.ts.snap is excluded by !**/*.snap
  • packages/core/stellar/src/non-fungible.test.ts.snap is excluded by !**/*.snap
  • packages/core/stellar/src/stablecoin.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (30)
  • packages/common/src/ai/descriptions/stellar.ts (1 hunks)
  • packages/core/stellar/src/add-pausable.ts (2 hunks)
  • packages/core/stellar/src/add-upgradeable.ts (2 hunks)
  • packages/core/stellar/src/common-options.ts (3 hunks)
  • packages/core/stellar/src/contract.ts (1 hunks)
  • packages/core/stellar/src/fungible.compile.test.ts (1 hunks)
  • packages/core/stellar/src/fungible.test.ts (1 hunks)
  • packages/core/stellar/src/fungible.test.ts.md (2 hunks)
  • packages/core/stellar/src/fungible.ts (7 hunks)
  • packages/core/stellar/src/generate/fungible.ts (1 hunks)
  • packages/core/stellar/src/generate/non-fungible.ts (1 hunks)
  • packages/core/stellar/src/generate/stablecoin.ts (1 hunks)
  • packages/core/stellar/src/non-fungible.compile.test.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 (10 hunks)
  • packages/core/stellar/src/set-access-control.ts (6 hunks)
  • packages/core/stellar/src/stablecoin.compile.test.ts (1 hunks)
  • packages/core/stellar/src/stablecoin.test.ts (1 hunks)
  • packages/core/stellar/src/stablecoin.test.ts.md (2 hunks)
  • packages/core/stellar/src/stablecoin.ts (2 hunks)
  • packages/mcp/src/stellar/schemas.ts (1 hunks)
  • packages/mcp/src/stellar/tools/fungible.test.ts (1 hunks)
  • packages/mcp/src/stellar/tools/fungible.ts (2 hunks)
  • packages/mcp/src/stellar/tools/non-fungible.test.ts (1 hunks)
  • packages/mcp/src/stellar/tools/stablecoin.test.ts (1 hunks)
  • packages/ui/src/stellar/FungibleControls.svelte (2 hunks)
  • packages/ui/src/stellar/NonFungibleControls.svelte (2 hunks)
  • packages/ui/src/stellar/StablecoinControls.svelte (2 hunks)
  • packages/ui/src/stellar/TraitImplementationSection.svelte (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-03T12:18:33.779Z
Learnt from: typicalHuman
Repo: OpenZeppelin/contracts-wizard PR: 715
File: packages/core/solidity/src/stablecoin.ts:74-77
Timestamp: 2025-11-03T12:18:33.779Z
Learning: In packages/core/solidity/src/stablecoin.ts, the ERC20Restricted base contract from openzeppelin/community-contracts provides a default implementation of isUserAllowed that returns true for non-BLOCKED users (blocklist logic). Only allowlist mode needs to override isUserAllowed to check for Restriction.ALLOWED instead. Blocklist mode uses the default implementation without override.

Applied to files:

  • packages/core/stellar/src/stablecoin.test.ts
  • packages/core/stellar/src/stablecoin.ts
📚 Learning: 2025-08-15T23:23:13.097Z
Learnt from: ernestognw
Repo: OpenZeppelin/contracts-wizard PR: 609
File: packages/core/solidity/src/signer.ts:31-38
Timestamp: 2025-08-15T23:23:13.097Z
Learning: In OpenZeppelin contracts-wizard, the `setUpgradeableAccount` function deliberately sets `c.upgradeable = false` after upgradeable setup to exclude EIP712Upgradeable and ERC7739Upgradeable variants (avoiding per-call SLOAD overhead). This architectural pattern necessitates manual `_disableInitializers()` setup in both account.ts and signer.ts since the standard upgradeable constructor logic doesn't apply when upgradeability is disabled post-setup.

Applied to files:

  • packages/core/stellar/src/add-upgradeable.ts
📚 Learning: 2025-08-20T20:23:30.259Z
Learnt from: ericglau
Repo: OpenZeppelin/contracts-wizard PR: 609
File: packages/core/solidity/src/set-upgradeable.ts:0-0
Timestamp: 2025-08-20T20:23:30.259Z
Learning: In OpenZeppelin contracts-wizard, upgradeable contracts use two different strategies for Initializable imports: Account contracts directly import from contracts-upgradeable/proxy/utils/Initializable.sol for explicit clarity, while other upgradeable contracts (ERC20, ERC721, Governor, etc.) use helpers to automatically transpile imports to upgradeable versions. This architectural separation is intentional due to the special ERC-4337 requirements of account contracts.

Applied to files:

  • packages/core/stellar/src/add-upgradeable.ts
📚 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 (11)
packages/core/stellar/src/contract.ts (1)
packages/core/stellar/src/fungible.ts (1)
  • functions (269-359)
packages/core/stellar/src/stablecoin.compile.test.ts (2)
packages/core/stellar/src/utils/compile-test.ts (1)
  • runRustCompilationTest (40-69)
packages/core/stellar/src/stablecoin.ts (1)
  • buildStablecoin (45-55)
packages/core/stellar/src/add-pausable.ts (2)
packages/core/stellar/src/contract.ts (1)
  • ContractBuilder (74-283)
packages/core/stellar/src/set-access-control.ts (2)
  • Access (8-8)
  • requireAccessControl (78-137)
packages/core/stellar/src/non-fungible.compile.test.ts (2)
packages/core/stellar/src/utils/compile-test.ts (1)
  • runRustCompilationTest (40-69)
packages/core/stellar/src/non-fungible.ts (1)
  • buildNonFungible (65-128)
packages/core/stellar/src/add-upgradeable.ts (2)
packages/core/stellar/src/contract.ts (1)
  • ContractBuilder (74-283)
packages/core/stellar/src/set-access-control.ts (2)
  • Access (8-8)
  • requireAccessControl (78-137)
packages/mcp/src/stellar/schemas.ts (1)
packages/common/src/ai/descriptions/stellar.ts (1)
  • stellarCommonDescriptions (11-17)
packages/core/stellar/src/fungible.compile.test.ts (2)
packages/core/stellar/src/utils/compile-test.ts (1)
  • runRustCompilationTest (40-69)
packages/core/stellar/src/fungible.ts (1)
  • buildFungible (58-89)
packages/core/stellar/src/non-fungible.ts (6)
packages/core/stellar/src/add-pausable.ts (1)
  • addPausable (7-50)
packages/core/stellar/src/add-upgradeable.ts (1)
  • addUpgradeable (7-49)
packages/core/stellar/src/set-access-control.ts (3)
  • setAccessControl (18-73)
  • Access (8-8)
  • requireAccessControl (78-137)
packages/core/stellar/src/contract.ts (1)
  • ContractBuilder (74-283)
packages/core/stellar/src/utils/define-functions.ts (1)
  • defineFunctions (9-16)
packages/core/stellar/src/common-options.ts (1)
  • getSelfArg (39-41)
packages/core/stellar/src/stablecoin.ts (2)
packages/core/stellar/src/contract.ts (1)
  • ContractBuilder (74-283)
packages/core/stellar/src/set-access-control.ts (2)
  • Access (8-8)
  • requireAccessControl (78-137)
packages/core/stellar/src/fungible.ts (5)
packages/core/stellar/src/utils/convert-strings.ts (1)
  • toByteArray (28-34)
packages/core/stellar/src/add-pausable.ts (1)
  • addPausable (7-50)
packages/core/stellar/src/add-upgradeable.ts (1)
  • addUpgradeable (7-49)
packages/core/stellar/src/set-access-control.ts (3)
  • setAccessControl (18-73)
  • Access (8-8)
  • requireAccessControl (78-137)
packages/core/stellar/src/contract.ts (1)
  • ContractBuilder (74-283)
packages/core/stellar/src/set-access-control.ts (3)
packages/core/stellar/src/contract.ts (1)
  • ContractBuilder (74-283)
packages/core/stellar/src/utils/define-functions.ts (1)
  • defineFunctions (9-16)
packages/core/stellar/src/common-options.ts (1)
  • getSelfArg (39-41)
⏰ 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). (2)
  • GitHub Check: build (stellar, compile)
  • GitHub Check: build (solidity, default)
🔇 Additional comments (35)
packages/mcp/src/stellar/schemas.ts (1)

27-27: LGTM! Schema addition follows the established pattern.

The new explicitImplementations field is properly typed as an optional boolean and includes a description from the common descriptions. It will be inherited by all contract schemas through the spread operator.

packages/mcp/src/stellar/tools/fungible.test.ts (1)

50-50: LGTM! Test coverage expanded appropriately.

The addition of explicitImplementations: true to the comprehensive test case ensures the new field is validated through the MCP tool pipeline.

packages/mcp/src/stellar/tools/fungible.ts (1)

13-24: LGTM! Parameter propagation is correct.

The explicitImplementations parameter is properly destructured from the input and passed through to the FungibleOptions, following the established pattern for other optional fields.

Also applies to: 35-35

packages/mcp/src/stellar/tools/non-fungible.test.ts (1)

52-52: LGTM! Consistent test coverage.

The addition mirrors the fungible test expansion and ensures the new field is validated for non-fungible tokens.

packages/mcp/src/stellar/tools/stablecoin.test.ts (1)

52-52: LGTM! Complete test coverage.

This completes the test coverage expansion across all three token types (fungible, non-fungible, and stablecoin).

packages/core/stellar/src/contract.ts (1)

188-190: LGTM! Useful helper for bulk trait function additions.

This convenience method cleanly handles batch-adding trait functions, which aligns well with the explicit implementations feature where multiple trait methods need to be registered at once.

packages/core/stellar/src/common-options.ts (3)

10-14: LGTM!

The addition of explicitImplementations: false to contractDefaults is correct and ensures backward compatibility by making explicit trait implementations opt-in.


20-23: LGTM!

The interface extension properly adds explicitImplementations as an optional boolean property, maintaining consistency with other common contract options.


31-37: LGTM!

The propagation of explicitImplementations in withCommonContractDefaults follows the established pattern and correctly applies the nullish coalescing operator.

packages/core/stellar/src/set-access-control.ts (6)

1-3: LGTM!

The new imports defineFunctions and getSelfArg are properly used in the function definitions at the end of the file.


18-44: LGTM!

The setAccessControl function properly implements the explicit implementations feature for ownable:

  • Default parameter value maintains backward compatibility
  • Tag switching correctly omits default_impl for explicit implementations
  • Conditional use of addTraitForEachFunctions vs addTraitImplBlock is appropriate

46-73: LGTM!

The roles access control path correctly mirrors the ownable implementation pattern with proper tag switching and conditional trait wiring. The addition of the Symbol import on line 48 ensures it's available for the access control functions.


78-137: LGTM!

The requireAccessControl function properly accepts and propagates the explicitImplementations parameter to setAccessControl, maintaining backward compatibility with the default value.


139-157: LGTM!

The ownableFunctions definitions are correct and complete, properly delegating to the ownable module functions with appropriate signatures.


159-223: LGTM!

The accessControlFunctions definitions are comprehensive and correctly delegate to the access_control module with appropriate signatures and return types.

packages/core/stellar/src/generate/stablecoin.ts (1)

8-20: LGTM!

The addition of explicitImplementations: booleans to the stablecoin blueprint correctly follows the pattern of other boolean options and will generate appropriate test variants.

packages/core/stellar/src/fungible.test.ts (1)

101-103: LGTM!

The new test case properly exercises the explicit trait implementations feature and follows the established testing pattern.

packages/core/stellar/src/stablecoin.compile.test.ts (1)

161-173: LGTM!

The new compilation test for explicit trait implementations follows the established pattern and will verify that the generated code compiles successfully.

packages/ui/src/stellar/NonFungibleControls.svelte (2)

10-10: LGTM!

The import of TraitImplementationSection is appropriate for exposing the explicit implementations toggle in the UI.


56-56: LGTM!

The integration of TraitImplementationSection with two-way binding to opts.explicitImplementations follows the established pattern and properly exposes the configuration option in the UI.

packages/core/stellar/src/non-fungible.test.ts (1)

156-158: LGTM!

The new test case properly exercises the explicit trait implementations feature for non-fungible tokens and follows the established testing pattern.

packages/core/stellar/src/stablecoin.ts (2)

45-55: LGTM!

The buildStablecoin function correctly propagates allOpts.explicitImplementations to addLimitations.


57-97: LGTM!

The addLimitations function properly:

  • Accepts the new explicitImplementations parameter
  • Propagates it to both requireAccessControl calls for add and remove functions

This ensures that limitation functions respect the explicit implementations mode when access control is applied.

packages/ui/src/stellar/FungibleControls.svelte (1)

9-9: LGTM: Clean integration of trait implementation toggle.

The TraitImplementationSection is properly imported and rendered with two-way binding to opts.explicitImplementations. The placement between Settings and Features sections is consistent with the other UI controls (StablecoinControls and NonFungibleControls).

Also applies to: 48-49

packages/ui/src/stellar/StablecoinControls.svelte (1)

10-10: LGTM: Consistent UI integration.

The TraitImplementationSection integration follows the same pattern as FungibleControls.svelte, maintaining consistency across the UI components.

Also applies to: 49-50

packages/core/stellar/src/add-upgradeable.ts (2)

7-7: LGTM: Function signature properly updated.

The explicitImplementations parameter is correctly added to the function signature and aligns with the broader changes across the codebase (e.g., addPausable, setAccessControl).


37-48: LGTM: Parameter correctly threaded through.

The explicitImplementations parameter is properly passed to requireAccessControl as the final argument, matching the updated signature. The parameter defaults to false in requireAccessControl, maintaining backward compatibility.

packages/core/stellar/src/generate/fungible.ts (1)

18-18: Mathematical claim verified; actual performance impact requires testing.

The explicitImplementations: booleans addition does exactly double the permutations (from 96 to 192). The blueprint generates: 2^5 boolean fields × 3 access options × 2 info options = 192 permutations (vs. 96 before).

However, the original test path is incorrect—tests are at packages/core/stellar/src/fungible.test.ts and fungible.compile.test.ts, not in the generate/ subdirectory. Whether this doubling causes "significant" test slowdown is speculative without actual execution metrics.

To properly verify impact, run the actual tests and compare execution times before/after the change.

packages/core/stellar/src/generate/non-fungible.ts (1)

20-20: The performance concern does not apply to the test suite.

The test files use manual snapshot testing with hand-selected option combinations, not the generator functions. The generateNonFungibleOptions generator is defined but is not called anywhere in the test suite. Adding explicitImplementations: booleans increases the mathematical permutation space, but since tests don't iterate through all permutations, there is no actual test performance impact. The suggested verification command calculates the permutation count mathematically but doesn't measure real test execution time, as tests don't use the generators.

Likely an incorrect or invalid review comment.

packages/core/stellar/src/fungible.ts (6)

15-26: LGTM: explicitImplementations added to defaults.

The flag is correctly added to the defaults object, inheriting from commonDefaults.explicitImplementations. This maintains backward compatibility while enabling the new feature.


58-89: LGTM: Consistent flag propagation throughout buildFungible.

The explicitImplementations flag is correctly threaded through all helper functions (addBase, addPausable, addUpgradeable, addBurnable, addMintable, setAccessControl). This ensures the feature is applied consistently across all contract components.


91-135: LGTM: addBase correctly implements conditional trait generation.

The function properly handles both explicit and default implementations:

  • Line 104: default_impl macro imported only when needed
  • Lines 109-111: Address imported when required by explicit functions or pausable overrides
  • Line 116: Trait tags conditionally include default_impl
  • Lines 122-124: Explicit functions added when flag is true
  • Lines 126-134: Pausable overrides work correctly in both modes

The conditional import at lines 109-111 correctly covers both cases where Address is needed: when explicit implementations require it for trait function signatures, and when pausable overrides require it for transfer/transfer_from functions.


137-178: LGTM: addMintable correctly propagates explicitImplementations.

The flag is properly passed to requireAccessControl in both the ownable (line 145) and roles (lines 155-166) cases, maintaining consistency with the access control pattern used in add-pausable.ts and add-upgradeable.ts.


180-206: LGTM: addBurnable correctly handles three implementation modes.

The function properly handles the interaction between pausable and explicitImplementations:

  1. When pausable: explicit functions with pause guards (lines 191-198)
  2. When explicitImplementations && !pausable: explicit functions without guards (lines 199-200)
  3. When neither: default implementation with default_impl tag (lines 201-205)

The if-else chain correctly prioritizes pausable behavior when both flags are true, which is appropriate since pausability requires explicit function overrides.


361-373: LGTM: Function lists are complete and correctly defined.

The fungibleTokenTraitFunctions list (lines 361-371) includes all nine FungibleToken trait methods, and fungibleBurnableFunctions (line 373) includes both burnable methods. These lists are properly used in the conditional logic at lines 123 and 200 to inject explicit implementations when the flag is enabled.

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.

Suggestion: add UI toggle for default_impl attribute in contract generation

2 participants