Skip to content

Comments

fix(tangle-cloud): fix service request args and improve UX#3134

Open
vutuanlinh2k2 wants to merge 21 commits intov2from
linh/fix/service-req-args
Open

fix(tangle-cloud): fix service request args and improve UX#3134
vutuanlinh2k2 wants to merge 21 commits intov2from
linh/fix/service-req-args

Conversation

@vutuanlinh2k2
Copy link
Contributor

@vutuanlinh2k2 vutuanlinh2k2 commented Feb 23, 2026

Summary

  • Fix possibly undefined requirements type error in service request args
  • Improve service request variant tooltip descriptions
  • Improve service request detail modal UX
  • Consolidate request variant display into CommitmentSection
  • Support tntExposureBps in standard approval flow
  • Add standard approval flow and improve service request UX
  • Replace globalExposurePercent with per-operator exposure in deploy form
  • Show per-operator exposure labels in ServiceRequestSummary
  • Expose requestedOperators from useServiceRequestDetails
  • Improve duration validation and fix membership label import
  • Display request variant and exposure details in ServiceRequestSummary
  • Fetch defaultTntMinExposureBps from contract instead of hardcoding
  • Detect service request variant and expose custom security requirements
  • Wire up multi-mode deploy UI and update deploy schema
  • Support basic/exposure/security service request modes
  • Add useBlueprintRequestSchema hook and TLV request args encoder

Videos

CleanShot.2026-02-23.at.22.16.27.mp4
CleanShot.2026-02-23.at.22.12.45.mp4

Test plan

  • Verify service request modal displays correctly for all variants (basic, exposure, security)
  • Verify per-operator exposure labels show correctly in deploy form
  • Verify standard approval flow works end-to-end
  • Verify tntExposureBps is handled correctly in approval

🤖 Generated with Claude Code

vutuanlinh2k2 and others added 21 commits February 22, 2026 14:58
Adds encodeRequestArgsFromJson which validates and encodes JSON args
against a parsed blueprint schema into TLV binary format. Introduces
RequestArgsEncodingError for precise per-path error reporting.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fetches and parses the on-chain request schema from getBlueprintDefinition.
Returns parsed SchemaField[], hasRequestSchema flag, and any parse errors
so consumers can gate encoding and surface friendly error messages.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…st modes

Adds exposureBps to ServiceRequestParams and a selectRequestFunction helper
to pick between requestService, requestServiceWithExposure, and
requestServiceWithSecurity based on the params. Adds validateServiceRequestParams
for pre-flight checks. Updates encodeServiceConfig to use the schema-aware
TLV encoder instead of throwing for non-empty args.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds requestMode (basic/exposure/security) and globalExposurePercent fields.
Makes assets and securityCommitments optional by default with mode-gated
validation. Removes approvalModel/minApproval fields and the now-unused
formatServiceRequestData function. Passes request schema metadata through
BaseDeployStepProps.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces AdvancedOptionsStep with RequestModeStep (basic/exposure/security
selector with optional global exposure input). AssetConfigurationStep now
hides itself in non-security modes. OperatorSelectionStep syncs security
commitments only when in security mode and removes the approval model logic.
RequestArgsConfigurationStep shows schema-driven field counts and parse
error banners. Deploy page integrates useBlueprintRequestSchema, passes
schema metadata to steps, and routes the submit to the correct contract
variant with structured error handling.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switches to a cleaner border/background treatment: rounded-xl with a
thinner border and solid mono background, keeping the gradient only in
dark mode.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tom security requirements

Adds ServiceRequestVariant type (basic/exposure/security/unknown) and
detects the variant by decoding the originating transaction's calldata.
Separates the default TNT security requirement from custom ones so
consumers can distinguish between the two.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ad of hardcoding

Replaces the hardcoded DEFAULT_TNT_MIN_EXPOSURE_BPS constant (1000) with
a live contract read of defaultTntMinExposureBps so the check stays
accurate if the contract value changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…erviceRequestSummary

Shows the request mode (Basic / With Exposure / With Security), the
per-operator requested exposure percentages for exposure-mode requests,
and the default TNT collateral range when present.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…el import

BasicInformationStep: wires up setError/clearErrors for real-time
duration range feedback, adds isInvalid and max props to the Input, and
properly clears the field when empty.

OperatorSelectionStep: replaces the removed getMembershipModelLabel
import with getMembershipLabel from the local serviceRequest types.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…stDetails

Decode operator addresses from service request calldata and return them
alongside requestedExposureBps. Swap TangleABI-based getLogs calls for
typed parseAbi/parseAbiItem signatures to avoid ambiguous overload
resolution, and separate variant detection (via selector) from argument
decoding so a decode failure does not discard the already-identified
variant.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…stSummary

Replace the flat comma-separated exposure list with a per-operator table
that maps each exposure bps value to its operator address (shortened).
Also add a human-readable "Meaning" row that explains what each request
variant (basic / exposure / security / unknown) implies.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…xposure in deploy form

- Remove AssetConfigurationStep from Deployment; asset security
  configuration is now consolidated inside RequestModeStep
- Replace the single globalExposurePercent slider with a per-operator
  Slider row (keyed by normalised operator address) in exposure mode
- Add security mode UI: asset selector + AssetRequirementFormItem rows
  with min/max exposure sliders and remove buttons
- Update deployBlueprint Zod schema from globalExposurePercent to
  operatorExposurePercents record; validate that every selected operator
  has an entry when requestMode === 'exposure'
- Fix ZodError path serialisation in page.tsx to use err.path.join('.')
  so nested field errors (e.g. operatorExposurePercents.<addr>) are set
  correctly; switch watch() to getValues() in onDeployBlueprint to read
  latest committed values

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…quest UX

- Add StandardApprovalInfo component for basic/exposure approval (read-only view)
- Add standard approval path in ServiceRequestDetailModal for non-custom requirements
- Move asset requirements selector inline with Mode selector in RequestModeStep
- Replace verbose "Meaning" section with ModalSafeTooltip on variant label in ServiceRequestSummary
- Add disabled state (opacity + cursor) to Slider component
- Remove redundant hasRequirements variable (subsumed by hasCustomRequirements)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace static StandardApprovalInfo display with an interactive
ExposureCommitmentInput for the TNT requirement, pass tntExposureBps
through the form and approval tx, and add the 3-arg approveService /
approveServiceWithBls ABI overloads that accept a tntExposureBps param.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…itmentSection

Move ModalSafeTooltip and request variant/exposure logic from ServiceRequestSummary
into CommitmentSection, removing the separate "Request Variant" section. Also update
ExposureCommitmentInput to always show the operator exposure row with a "protocol
default" label when exposure is 100%.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace BlueprintInfoCard with an inline blueprint link in CommitmentSection
- Highlight current operator with a "(You)" label in exposure list
- Move CommitmentSection above PaymentTermsSection for better info hierarchy
- Fix modal layout to support scrollable body with sticky footer

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Feb 23, 2026

Claude finished @vutuanlinh2k2's task —— View job


PR Code Review

This is a substantial PR that implements multi-mode service requests with impressive scope and good architectural decisions. Here's my comprehensive analysis:

Todo List

  • Analyze overall architecture and approach
  • Review new TLV encoding implementation
  • Check service request variant detection logic
  • Examine deployment UI changes and validation
  • Review security requirements handling
  • Check for potential bugs and edge cases
  • Assess performance implications
  • Evaluate code quality and best practices
  • Provide specific feedback and recommendations

🟢 Strengths

Architecture & Design

  • Clean separation of concerns: Well-structured separation between basic/exposure/security modes
  • Reusable components: Good abstraction of shared logic into hooks and utilities
  • Type safety: Comprehensive TypeScript usage with proper error handling

Code Quality

  • Error handling: Excellent error handling in TLV encoder with custom error types
  • Validation: Robust form validation with clear user feedback
  • Documentation: Good JSDoc comments and inline documentation

🟡 Issues & Recommendations

Critical Issues

1. TLV Encoder Validation - libs/tangle-shared-ui/src/codec/requestArgsEncoder.ts:48

// Current regex allows empty hex strings
if (!/^0x[0-9a-fA-F]*$/.test(hex)) {

Issue: The regex allows 0x which may not be intended for all contexts.
Fix: Add explicit length validation or handle empty case separately.

2. Service Request Variant Detection Logic - libs/tangle-shared-ui/src/data/services/useServiceRequestDetails.ts:248-285

The transaction lookup approach is clever but has reliability concerns:

  • Race conditions: If multiple transactions occur in the same block
  • Performance: Full blockchain log scanning could be slow
  • Fallback handling: Limited error recovery if transaction data is unavailable

Recommendation: Consider caching variant information or storing it on-chain for more reliable detection.

Security Concerns

3. Exposure Validation - apps/tangle-cloud/src/pages/blueprints/[id]/deploy/DeploySteps/RequestModeStep.tsx:119-136

if (minExposurePercent < 1) {
  errorMsg = 'Minimum exposure percent must be at least 1%';
} else if (maxExposurePercent < 1) {
  errorMsg = 'Maximum exposure percent must be at least 1%';
}

Issue: Hardcoded 1% minimum may not align with contract requirements.
Fix: Fetch minimum from contract or validate against contract constants.

4. Address Normalization - Multiple files

Inconsistent address normalization could lead to edge cases:

  • normalizeOperatorKey uses toLowerCase()
  • parseAddressLowercase handles parsing + lowercasing
  • Some direct toLowerCase() calls

Recommendation: Standardize on a single address normalization utility.

Performance & UX

5. Redundant Contract Calls - apps/tangle-cloud/src/data/services/useServiceApproveTx.ts:121-133

if (simpleParams.tntExposureBps && simpleParams.tntExposureBps > 0) {
  return { functionName: 'approveService', args: [id, percent, exposureBps] }
}
return { functionName: 'approveService', args: [id, percent] }

Good: Clean overload selection logic handles different contract method signatures properly.

6. Slider Component Enhancement - libs/ui-components/src/components/Slider/Slider.tsx:52-55

if (!value || !value.length || value.length > 2) {
  console.trace('[Slider.tsx] You must provide...')
}

Issue: console.trace in production code.
Fix: Use proper logging utility or remove for production builds.


🔧 Specific Recommendations

1. Input Sanitization

// In requestArgsEncoder.ts, add explicit empty hex handling:
const normalizeHex = (value: unknown, path: string): string => {
  // ... existing validation
  if (hex === '0x') {
    fail(path, 'Hex string cannot be empty (0x only)');
  }
  // ... rest of validation
};

2. Contract Integration

Fetch validation constants from contracts instead of hardcoding:

// Instead of hardcoded 1%, fetch from contract
const minExposureBps = await publicClient.readContract({
  address: tangleAddress,
  functionName: 'minExposureBps'
});

3. Error Recovery

Add fallback variant detection:

// In useServiceRequestDetails.ts, add fallback logic
if (requestVariant === 'unknown') {
  // Fallback: check if custom security requirements exist
  requestVariant = customSecurityRequirements.length > 0 ? 'security' : 'basic';
}

4. Performance Optimization

Consider memoizing expensive operations:

const availableAssets = useMemo(() => {
  // Expensive filtering operation
}, [allAssetsMap, assets]); // ✅ Already implemented correctly

🟢 What's Working Well

  1. Form State Management: Excellent use of React Hook Form with proper validation
  2. UI/UX Design: Clean, intuitive interface for complex multi-mode workflows
  3. Type Safety: Comprehensive TypeScript usage prevents many runtime errors
  4. Error Boundaries: Good error handling and user feedback throughout
  5. Code Organization: Clear file structure and component responsibilities

📋 Final Assessment

This is a high-quality implementation that demonstrates:

  • ✅ Strong architectural decisions
  • ✅ Comprehensive feature coverage
  • ✅ Good code organization and maintainability
  • ⚠️ A few areas needing attention (validation, logging, edge cases)

The issues identified are mostly minor and don't block the core functionality. This PR successfully implements a complex feature with good attention to user experience and code quality.

Recommendation: Approve after addressing the critical validation and logging issues.


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.

1 participant