Skip to content

Conversation

@devpavan04
Copy link
Member

Summary of changes

  • Modified useVaultParticipants to accept sorted vaults and adjusted participant mapping accordingly.
  • Enhanced VaultsTable to handle cases where depositCap is zero.

@drewstone I'm doing a QA of all the pages in tangle-dapp and tangle-cloud, few more fixes will be addressed here.

  • Fix restake vault icon size
  • Restake page tx notification links redirects to polkadot apps UI dashboard page and not to the block page where tx can be viewed
  • Error when connecting to custom network in bridge with Metamask
CleanShot 2025-11-07 at 07 51 29@2x

QA

  • Restake deposit
  • Restake delegate
  • Restake undelegate
  • Restake cancel undelegate
  • Restake withdraw
  • Restake cancel withdraw
CleanShot.2025-11-07.at.07.44.44.mp4

  • AI Credits

  • Nomination page stats
CleanShot 2025-11-07 at 07 53 46@2x

Proposed area of change

  • apps/tangle-dapp
  • apps/tangle-cloud
  • apps/leaderboard
  • libs/tangle-shared-ui
  • libs/ui-components

…ticipant handling and capacity checks

- Modified useVaultParticipants to accept sorted vaults and adjusted participant mapping accordingly.
- Enhanced VaultsTable to handle cases where depositCap is zero, ensuring accurate capacity percentage calculations.
@netlify
Copy link

netlify bot commented Nov 7, 2025

Deploy Preview for tangle-leaderboard ready!

Name Link
🔨 Latest commit b26a692
🔍 Latest deploy log https://app.netlify.com/projects/tangle-leaderboard/deploys/690e16d050d12200081213ee
😎 Deploy Preview https://deploy-preview-3070--tangle-leaderboard.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link

netlify bot commented Nov 7, 2025

Deploy Preview for tangle-dapp failed. Why did it fail? →

Name Link
🔨 Latest commit b26a692
🔍 Latest deploy log https://app.netlify.com/projects/tangle-dapp/deploys/690e16d04b77fa00082c5ea3

@netlify
Copy link

netlify bot commented Nov 7, 2025

Deploy Preview for tangle-cloud ready!

Name Link
🔨 Latest commit b26a692
🔍 Latest deploy log https://app.netlify.com/projects/tangle-cloud/deploys/690e16d093e3aa00081b30da
😎 Deploy Preview https://deploy-preview-3070--tangle-cloud.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@claude
Copy link

claude bot commented Nov 7, 2025

Claude finished @devpavan04's task —— View job


Code Review Complete ✅

Todo List:

  • Read and analyze VaultsHightlightCard.tsx changes
  • Read and analyze VaultsTable.tsx changes
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Review security implications
  • Provide comprehensive feedback

Summary

This PR contains two focused bug fixes that improve vault participant handling and capacity calculations. The changes are well-implemented and address specific edge cases.

Code Quality & Best Practices ✅

Positive aspects:

  • Clean, focused changes with clear intent
  • Proper TypeScript usage with null checks and optional chaining
  • Consistent code style following the existing patterns
  • Good separation of concerns in the refactored useVaultParticipants hook

Changes Analysis

VaultsHightlightCard.tsx

Key improvements:

  1. Better dependency management: useVaultParticipants now accepts sortedVaults as a parameter instead of fetching vault assets separately, reducing API calls and improving data consistency
  2. Enhanced null safety: Added optional chaining (?.) in delegatorIdStorage.args?.[0]?.toString() and operatorIdStorage.args?.[0]?.toString()
  3. Improved participant display logic: Added && vaultParticipants > 0 check to only show participant count when greater than zero

Code flow improvement:

// Before: Used separate useVaultAssets hook
const useVaultParticipants = () => {
  const { result: vaultAssets } = useVaultAssets();
  // ...
}

// After: Directly uses passed vaults parameter
const useVaultParticipants = (vaults: RestakeVault[] | null) => {
  // Eliminates extra API call and improves data consistency
}

VaultsTable.tsx

Edge case fix:

  • Added depositCap.isZero() check in capacity percentage calculation:
// Before
const capacityPercentage = tvl === undefined || depositCap === undefined
  ? null
  : calculateBnRatio(tvl, depositCap);

// After  
const capacityPercentage = tvl === undefined || depositCap === undefined || depositCap.isZero()
  ? null
  : calculateBnRatio(tvl, depositCap);

This prevents division by zero when depositCap is zero, which would cause calculation errors.

Potential Issues & Bugs 🔍

No significant bugs detected, but here are some observations:

  1. Minor: In VaultsHightlightCard.tsx:232, the function signature change is a breaking change for external consumers, but since this appears to be internal usage, it's acceptable.

  2. Edge case handled: The zero deposit cap handling in VaultsTable.tsx properly prevents mathematical errors.

Performance Considerations 🚀

Performance improvements:

  1. Reduced API calls: Eliminating the separate useVaultAssets call reduces network requests
  2. Better memoization: The useVaultParticipants hook now has cleaner dependencies [assetDelegators, assetOperators, vaults]
  3. Efficient data flow: Direct parameter passing instead of separate data fetching

Security Implications 🔒

Secure practices maintained:

  • Proper null/undefined checks prevent runtime errors
  • Type safety preserved with TypeScript
  • Optional chaining protects against accessing properties of null/undefined values
  • Input validation with isSubstrateAddress() checks remain intact

Suggestions for Improvement

  1. Consider adding JSDoc: The useVaultParticipants function could benefit from documentation explaining the parameter change:
/**
 * Hook to calculate vault participants from sorted vaults data
 * @param vaults - Sorted vaults to calculate participants for
 */
const useVaultParticipants = (vaults: RestakeVault[] | null) => {
  1. Type safety: The changes improve type safety, particularly with the optional chaining additions.

Overall Assessment

This is a well-executed bug fix PR that:

  • ✅ Addresses specific issues without introducing breaking changes
  • ✅ Improves performance by reducing API calls
  • ✅ Enhances null safety with proper checks
  • ✅ Maintains existing code patterns and conventions
  • ✅ Fixes the zero deposit capacity edge case

The changes are minimal, focused, and improve the robustness of the vault components. Recommended for approval pending any integration testing.


@devpavan04 devpavan04 marked this pull request as draft November 8, 2025 00:10
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