Skip to content

feat(tangle-cloud): runtime permitted callers management#3132

Merged
drewstone merged 2 commits intov2from
linh/feat/permitted-callers
Feb 19, 2026
Merged

feat(tangle-cloud): runtime permitted callers management#3132
drewstone merged 2 commits intov2from
linh/feat/permitted-callers

Conversation

@vutuanlinh2k2
Copy link
Contributor

@vutuanlinh2k2 vutuanlinh2k2 commented Feb 19, 2026

Summary

  • Add permitted callers management UI to the service detail page, allowing service owners to add/remove addresses that can submit jobs at runtime
  • Fetch permittedCallers from the GraphQL indexer with backward-compatible fallback for older indexer schemas
  • Rename "View" button to "Blueprint" in running instance table for clarity

Video

CleanShot.2026-02-19.at.16.10.09.mp4

Test plan

  • Verify service detail page loads permitted callers list correctly
  • Verify owner can add a new permitted caller address
  • Verify owner can remove a permitted caller
  • Verify non-owners see a read-only view with an info message
  • Verify input validation (zero address, duplicate, owner address, invalid format)
  • Verify backward compatibility when indexer doesn't expose permittedCallers field

🤖 Generated with Claude Code

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

claude bot commented Feb 19, 2026

Claude finished @vutuanlinh2k2's task —— View job


PR Review: Runtime Permitted Callers Management

Todo List

  • Read and analyze changed files
  • Review code quality and best practices adherence
  • Check for potential bugs and security issues
  • Assess performance considerations
  • Provide comprehensive feedback

Summary

This PR adds a comprehensive permitted callers management system to the service detail page. Overall, the implementation is solid with good attention to security and backward compatibility. Here are my detailed findings:

✅ Strengths

Security & Validation:

  • Excellent input validation in validatePermittedCallerInput function (apps/tangle-cloud/src/pages/services/[id]/page.tsx:51-94)
  • Proper prevention of zero address, owner duplication, and existing caller checks
  • Owner-only restrictions properly enforced for add/remove operations
  • Address normalization using getAddress() for consistent comparison

Backward Compatibility:

  • Smart fallback handling in GraphQL query (libs/tangle-shared-ui/src/data/graphql/useServices.ts:169-198)
  • Graceful degradation when permittedCallers field isn't available in older indexer schemas

State Management:

  • Comprehensive local state synchronization with on-chain data
  • Transaction history tracking and validation for UI consistency
  • Proper error handling and loading states

🔧 Areas for Improvement

1. Performance Concerns

  • Transaction validation on every render (apps/tangle-cloud/src/pages/services/[id]/page.tsx:219-241): The aclHistoryTxChecks queries run for every transaction in history, which could become expensive
    // Consider adding pagination or limiting to recent transactions
    .slice(-100) // Good limit, but could be configurable

2. Code Quality Issues

Unused Props Parameter (apps/tangle-cloud/src/pages/instances/Instances/RunningInstanceTable.tsx:52):

export const RunningInstanceTable: FC<RunningInstanceTableProps> = () => {

The component receives refreshTrigger and setRefreshTrigger props but doesn't use them. Either use them or remove from the interface.

Magic Numbers (apps/tangle-cloud/src/pages/services/[id]/page.tsx:239):

staleTime: 30_000, // Consider extracting to constants

3. Type Safety
The Hash type casting (apps/tangle-cloud/src/pages/services/[id]/page.tsx:228) could be more robust:

hash: tx.hash as Hash, // Consider validating hash format first

4. UX/UI Improvements

Loading State Clarity: The permitted callers section doesn't show loading state while isLoadingOnChainDetails is true, which could confuse users about whether data is still loading.

Error Message Positioning: Error messages for ACL operations appear at the bottom of the card, which might not be visible if the user has scrolled.

🚨 Minor Security Considerations

1. Transaction Hash Validation: While the error handling is good, consider validating transaction hash format before attempting to fetch receipts.

2. Rate Limiting: The transaction validation queries could potentially hit rate limits if there are many historical transactions. Consider implementing exponential backoff.

📝 Code Style Alignment

The code generally follows the project conventions well:

  • ✅ Uses const ... => {} over function
  • ✅ Proper React FC typing
  • ✅ Descriptive variable names
  • ✅ Proper use of useMemo/useCallback
  • ✅ Consistent error handling patterns

🎯 Recommendations

  1. Add loading states for the permitted callers section
  2. Extract magic numbers to named constants
  3. Fix unused props in RunningInstanceTable
  4. Consider pagination for transaction history validation
  5. Add transaction hash validation before API calls
  6. Position error messages closer to relevant inputs

📋 Test Coverage Recommendations

The test plan looks comprehensive, but consider adding:

  • Edge cases for malformed transaction history
  • Network failure scenarios during ACL operations
  • Concurrent add/remove operations

Overall Assessment: ✅ Approve with Minor Improvements

This is a well-implemented feature that adds valuable functionality while maintaining security and backward compatibility. The issues identified are minor and don't block the core functionality.

@vutuanlinh2k2 vutuanlinh2k2 marked this pull request as draft February 19, 2026 09:53
…oading state

Remove tx history merging from permitted callers (indexer + refetch on
success is sufficient), simplify error display, add skeleton loader for
the permitted callers card, and clean up unused RunningInstanceTable props.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vutuanlinh2k2 vutuanlinh2k2 marked this pull request as ready for review February 19, 2026 10:15
@claude
Copy link

claude bot commented Feb 19, 2026

Claude finished @vutuanlinh2k2's task —— View job


PR Review: Runtime Permitted Callers Management

Todo List

  • Read and analyze changed files
  • Review code quality and best practices adherence
  • Check for potential bugs and security issues
  • Assess performance considerations
  • Provide comprehensive feedback

Summary

This PR adds a comprehensive permitted callers management system to the service detail page. Overall, the implementation is solid with good attention to security and backward compatibility. Here are my detailed findings:

✅ Strengths

Security & Validation:

  • Excellent input validation in validatePermittedCallerInput function (apps/tangle-cloud/src/pages/services/[id]/page.tsx:49-92)
  • Proper prevention of zero address, owner duplication, and existing caller checks
  • Owner-only restrictions properly enforced for add/remove operations
  • Address normalization using getAddress() for consistent comparison

Backward Compatibility:

  • Smart fallback handling in GraphQL query (libs/tangle-shared-ui/src/data/graphql/useServices.ts:169-198)
  • Graceful degradation when permittedCallers field isn't available in older indexer schemas

State Management:

  • Comprehensive local state synchronization with on-chain data
  • Proper error handling and loading states
  • Clean separation of concerns between UI and business logic

🔧 Areas for Improvement

1. Code Quality Issues

Unused Props Parameter (apps/tangle-cloud/src/pages/instances/Instances/RunningInstanceTable.tsx:40):

export const RunningInstanceTable: FC = () => {

The component interface RunningInstanceTableProps defines refreshTrigger and setRefreshTrigger props (apps/tangle-cloud/src/pages/instances/Instances/index.tsx:38-41), but the component doesn't accept or use them. Either use them or remove from the interface.

Magic Numbers (apps/tangle-cloud/src/pages/services/[id]/page.tsx:322, libs/tangle-shared-ui/src/data/graphql/useServices.ts:322):

staleTime: 30_000, // Consider extracting to constants

2. Performance Considerations

Potential N+1 Queries: The permitted callers validation runs on every input change, which could be optimized with debouncing for better UX during typing.

Memory Usage: The permittedCallers computation (apps/tangle-cloud/src/pages/services/[id]/page.tsx:189-216) recreates arrays on every render. Consider memoization optimizations.

3. UX/UI Improvements

Loading State Clarity: The permitted callers section doesn't show loading state while isLoadingOnChainDetails is true, which could confuse users about whether data is still loading.

Error Message Positioning: Error messages for ACL operations appear at the bottom of the card, which might not be visible if the user has scrolled.

🚨 Minor Security Considerations

1. Input Sanitization: While address validation is thorough, consider additional sanitization for any user input that might be logged or stored.

2. Race Conditions: The concurrent refetchPermission() and refetchService() calls could potentially create race conditions in edge cases.

📝 Code Style Alignment

The code generally follows the project conventions well:

  • ✅ Uses const ... => {} over function
  • ✅ Proper React FC typing
  • ✅ Descriptive variable names
  • ✅ Proper use of useMemo/useCallback
  • ✅ Consistent error handling patterns

🎯 Recommendations

  1. Fix unused props in RunningInstanceTable component
  2. Add loading states for the permitted callers section
  3. Extract magic numbers to named constants
  4. Consider debouncing input validation for better UX
  5. Position error messages closer to relevant inputs
  6. Add JSDoc comments for complex validation logic

📋 Test Coverage Recommendations

The test plan looks comprehensive, but consider adding:

  • Edge cases for concurrent add/remove operations
  • Network failure scenarios during ACL operations
  • Input validation edge cases (malformed addresses, etc.)

Overall Assessment: ✅ Approve with Minor Improvements

This is a well-implemented feature that adds valuable functionality while maintaining security and backward compatibility. The issues identified are minor and don't block the core functionality. The security validation is particularly well done, and the backward compatibility handling shows good foresight.

@vutuanlinh2k2
Copy link
Contributor Author

Addressing Claude's Review Comments

The latest commit (288a578) addresses the actionable items from the review:

Fixed:

  • Unused props in RunningInstanceTable — Removed RunningInstanceTableProps interface and the unused refreshTrigger/setRefreshTrigger props from the component and its call site.
  • Loading state for permitted callers — Added SkeletonLoader to the permitted callers card while isLoadingOnChainDetails or isLoadingServices is true.
  • aclHistoryTxChecks removed entirely — The tx history merging (useQueries + useTxHistoryStore + usePublicClient) was unnecessary since onSuccess callbacks in both useAddPermittedCallerTx and useRemovePermittedCallerTx already refetch indexer data. This also eliminates the staleTime: 30_000 magic number and the N RPC calls per render that the review flagged.

Review points that don't need action:

  • "Memory usage — permittedCallers recreates arrays on every render" — Incorrect; the computation is already wrapped in useMemo.
  • "Race conditions from concurrent refetchPermission/refetchService" — These are independent queries with no shared state; parallel refetch is correct.
  • "Debouncing input validation" — The validation is purely local (in-memory string comparisons via viem), no network calls involved.
  • "Add JSDoc comments" — Project guidelines say to avoid comments unless logic is complex.
  • "staleTime: 30_000 in useServices.ts" — Outside the scope of this PR.

@drewstone drewstone merged commit 5bf64cc into v2 Feb 19, 2026
2 checks passed
@drewstone drewstone deleted the linh/feat/permitted-callers branch February 19, 2026 12:58
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.

2 participants