Merge pull request #163 from akordavid373/fix/accuracy-collision-magic-numbers

Fix: Weekly to monthly calculation accuracy, ID collision risk, and m…

Author: Smartdevs17

src/services/walletService.ts

@@ -3,6 +3,12 @@ import { Framework, SFError } from '@superfluid-finance/sdk-core';
 
 import { ERC20__factory, getContractAddress } from '../contracts';
 import { getEvmRpcUrl } from '../config/evm';
+import {
+  TIME_CONSTANTS,
+  CRYPTO_CONSTANTS,
+  CHAIN_IDS,
+  ADDRESS_CONSTANTS,
+} from '../utils/constants/values';
 
 export interface WalletConnection {
   address: string;
@@ -44,7 +50,7 @@ export interface SuperfluidStreamResult {
   streamId: string;
 }
 
-const SECONDS_PER_MONTH = 30 * 24 * 60 * 60;
+const SECONDS_PER_MONTH = TIME_CONSTANTS.SECONDS_PER_MONTH;
 
 function isUserRejectedError(error: unknown): boolean {
   if (error == null || typeof error !== 'object') return false;
@@ -59,7 +65,7 @@ function superTokenResolverSymbol(chainId: number, tokenSymbol: string): string
   if (s === 'USDC' || s === 'USDC.E') return 'USDCx';
   if (s === 'MATIC') return 'MATICx';
   if (s === 'ETH') {
-    if (chainId === 137) return 'MATICx';
+    if (chainId === CHAIN_IDS.POLYGON) return 'MATICx';
     return 'ETHx';
   }
   if (s === 'ARB') {
@@ -154,11 +160,11 @@ export class WalletServiceManager {
         name: this.getNativeName(chainId),
         address: '0x0000000000000000000000000000000000000000',
         balance: ethers.utils.formatEther(nativeBalance),
-        decimals: 18,
+        decimals: CRYPTO_CONSTANTS.ETH_DECIMALS,
       });
 
       // Get USDC balance if on supported chains
-      if (chainId === 1 || chainId === 137 || chainId === 42161) {
+      if (chainId === CHAIN_IDS.ETHEREUM || chainId === CHAIN_IDS.POLYGON || chainId === CHAIN_IDS.ARBITRUM) {
         const usdcAddress = getContractAddress(chainId, 'usdc');
         if (!usdcAddress) {
           return balances;
@@ -171,8 +177,8 @@ export class WalletServiceManager {
             symbol: 'USDC',
             name: 'USD Coin',
             address: usdcAddress,
-            balance: ethers.utils.formatUnits(usdcBalance, 6),
-            decimals: 6,
+            balance: ethers.utils.formatUnits(usdcBalance, CRYPTO_CONSTANTS.USDC_DECIMALS),
+            decimals: CRYPTO_CONSTANTS.USDC_DECIMALS,
           });
         } catch {
           console.log('USDC not available on this chain');
@@ -211,11 +217,11 @@ export class WalletServiceManager {
           value: ethers.utils.parseEther(value || '0'),
         });
         // Network-specific buffer: higher for Polygon due to congestion variability
-        const bufferMultiplier = chainId === 137 ? 130 : 120;
+        const bufferMultiplier = chainId === CHAIN_IDS.POLYGON ? CRYPTO_CONSTANTS.POLYGON_GAS_BUFFER_MULTIPLIER : CRYPTO_CONSTANTS.DEFAULT_GAS_BUFFER_MULTIPLIER;
         gasLimit = estimated.mul(bufferMultiplier).div(100);
       } catch (err) {
         console.warn('Gas estimation failed, using safe fallback:', err);
-        gasLimit = ethers.BigNumber.from(100000);
+        gasLimit = ethers.BigNumber.from(CRYPTO_CONSTANTS.FALLBACK_GAS_LIMIT);
       }
     }
 
@@ -393,7 +399,7 @@ export class WalletServiceManager {
       const amountBn = ethers.utils.parseUnits(amount, decimals);
 
       // Sablier V2 LockupLinear is consistently deployed at this address across major EVM networks
-      const SABLIER_V2_LOCKUP_LINEAR = '0xAFb979d9afAd1aD27C5eFf4E27226E3AB9e5dCC9';
+      const SABLIER_V2_LOCKUP_LINEAR = ADDRESS_CONSTANTS.SABLIER_V2_LOCKUP_LINEAR;
 
       // 2. Approve Token Spending
       const txApprove = await erc20.approve(SABLIER_V2_LOCKUP_LINEAR, amountBn);
@@ -421,7 +427,7 @@ export class WalletServiceManager {
           cliff: 0,
           total: totalDuration,
         },
-        broker: ethers.constants.AddressZero,
+        broker: ADDRESS_CONSTANTS.ZERO_ADDRESS,
       };
 
       const txCreate = await sablierContract.createWithDurations(params);
@@ -447,18 +453,18 @@ export class WalletServiceManager {
 
   private getNativeSymbol(chainId: number): string {
     const symbols: Record<number, string> = {
-      1: 'ETH',
-      137: 'MATIC',
-      42161: 'ETH',
+      [CHAIN_IDS.ETHEREUM]: 'ETH',
+      [CHAIN_IDS.POLYGON]: 'MATIC',
+      [CHAIN_IDS.ARBITRUM]: 'ETH',
     };
     return symbols[chainId] || 'ETH';
   }
 
   private getNativeName(chainId: number): string {
     const names: Record<number, string> = {
-      1: 'Ethereum',
-      137: 'Polygon',
-      42161: 'Arbitrum',
+      [CHAIN_IDS.ETHEREUM]: 'Ethereum',
+      [CHAIN_IDS.POLYGON]: 'Polygon',
+      [CHAIN_IDS.ARBITRUM]: 'Arbitrum',
     };
     return names[chainId] || 'Ethereum';
   }

src/store/subscriptionStore.ts

@@ -10,6 +10,7 @@ import {
 } from '../types/subscription';
 import { dummySubscriptions } from '../utils/dummyData'; // eslint-disable-line
 import { advanceBillingDate } from '../utils/billingDate';
+import { BILLING_CONVERSIONS, CACHE_CONSTANTS } from '../utils/constants/values';
 import {
   syncRenewalReminders,
   presentChargeSuccessNotification,
@@ -18,7 +19,17 @@ import {
 
 const STORAGE_KEY = 'subtrackr-subscriptions';
 const STORE_VERSION = 1;
-const WRITE_DEBOUNCE_MS = 400;
+const WRITE_DEBOUNCE_MS = CACHE_CONSTANTS.WRITE_DEBOUNCE_MS;
+
+/**
+ * Generate a unique ID for subscriptions
+ * Uses timestamp + random component to prevent collisions
+ */
+const generateUniqueId = (): string => {
+  const timestamp = Date.now().toString(36);
+  const randomComponent = Math.random().toString(36).substring(2, 8);
+  return `${timestamp}-${randomComponent}`;
+};
 
 type PersistedSubscriptionSlice = Pick<SubscriptionState, 'subscriptions'>;
 
@@ -34,7 +45,7 @@ const toValidDate = (value: unknown, fallback = new Date()): Date => {
 const normalizeSubscription = (raw: Partial<Subscription>): Subscription => {
   const now = new Date();
   return {
-    id: raw.id ?? Date.now().toString(),
+    id: raw.id ?? generateUniqueId(),
     name: raw.name ?? 'Untitled',
     description: raw.description,
     category: raw.category ?? SubscriptionCategory.OTHER,
@@ -158,7 +169,7 @@ export const useSubscriptionStore = create<SubscriptionState>()(
         set({ isLoading: true, error: null });
         try {
           const newSubscription: Subscription = {
-            id: Date.now().toString(),
+            id: generateUniqueId(),
             ...data,
             isActive: true,
             notificationsEnabled: data.notificationsEnabled !== false,
@@ -300,15 +311,15 @@ export const useSubscriptionStore = create<SubscriptionState>()(
         const totalMonthlySpend = activeSubs.reduce((total, sub) => {
           if (sub.billingCycle === 'monthly') return total + sub.price;
           if (sub.billingCycle === 'yearly') return total + sub.price / 12;
-          if (sub.billingCycle === 'weekly') return total + sub.price * 4;
+          if (sub.billingCycle === 'weekly') return total + sub.price * BILLING_CONVERSIONS.WEEKS_PER_MONTH;
           return total + sub.price;
         }, 0);
 
         const totalYearlySpend = activeSubs.reduce((total, sub) => {
           if (sub.billingCycle === 'yearly') return total + sub.price;
-          if (sub.billingCycle === 'monthly') return total + sub.price * 12;
-          if (sub.billingCycle === 'weekly') return total + sub.price * 52;
-          return total + sub.price * 12;
+          if (sub.billingCycle === 'monthly') return total + sub.price * BILLING_CONVERSIONS.MONTHS_PER_YEAR;
+          if (sub.billingCycle === 'weekly') return total + sub.price * BILLING_CONVERSIONS.WEEKS_PER_YEAR;
+          return total + sub.price * BILLING_CONVERSIONS.MONTHS_PER_YEAR;
         }, 0);
 
         const categoryBreakdown = activeSubs.reduce(

src/utils/constants/values.ts

@@ -0,0 +1,118 @@
+/**
+ * Centralized constants for SubTrackr application
+ * Replaces magic numbers throughout the codebase for better maintainability
+ */
+
+// Time constants
+export const TIME_CONSTANTS = {
+  /** Seconds in a minute */
+  SECONDS_PER_MINUTE: 60,
+  /** Minutes in an hour */
+  MINUTES_PER_HOUR: 60,
+  /** Hours in a day */
+  HOURS_PER_DAY: 24,
+  /** Days in a week */
+  DAYS_PER_WEEK: 7,
+  /** Average days in a month (30.44) */
+  DAYS_PER_MONTH: 30.44,
+  /** Days in a year (365.25 accounting for leap years) */
+  DAYS_PER_YEAR: 365.25,
+  /** Seconds in an hour */
+  SECONDS_PER_HOUR: 60 * 60,
+  /** Seconds in a day */
+  SECONDS_PER_DAY: 24 * 60 * 60,
+  /** Seconds in a week */
+  SECONDS_PER_WEEK: 7 * 24 * 60 * 60,
+  /** Approximate seconds in a month (30 days) */
+  SECONDS_PER_MONTH: 30 * 24 * 60 * 60,
+  /** Seconds in a year */
+  SECONDS_PER_YEAR: 365 * 24 * 60 * 60,
+  /** Milliseconds in a second */
+  MS_PER_SECOND: 1000,
+  /** Milliseconds in a minute */
+  MS_PER_MINUTE: 60 * 1000,
+  /** Milliseconds in an hour */
+  MS_PER_HOUR: 60 * 60 * 1000,
+  /** Milliseconds in a day */
+  MS_PER_DAY: 24 * 60 * 60 * 1000,
+  /** Milliseconds in a week */
+  MS_PER_WEEK: 7 * 24 * 60 * 60 * 1000,
+} as const;
+
+// Billing cycle conversion factors
+export const BILLING_CONVERSIONS = {
+  /** Average weeks per month for accurate conversion */
+  WEEKS_PER_MONTH: 52 / 12, // 4.333...
+  /** Weeks per year */
+  WEEKS_PER_YEAR: 52,
+  /** Months per year */
+  MONTHS_PER_YEAR: 12,
+} as const;
+
+// Cryptocurrency and blockchain constants
+export const CRYPTO_CONSTANTS = {
+  /** Standard ETH decimals */
+  ETH_DECIMALS: 18,
+  /** Standard USDC decimals */
+  USDC_DECIMALS: 6,
+  /** Wei to ETH conversion factor */
+  WEI_TO_ETHER: 1e18,
+  /** Gas limit fallback for estimation failures */
+  FALLBACK_GAS_LIMIT: 100000,
+  /** Gas buffer multiplier for Polygon (130%) */
+  POLYGON_GAS_BUFFER_MULTIPLIER: 130,
+  /** Gas buffer multiplier for other networks (120%) */
+  DEFAULT_GAS_BUFFER_MULTIPLIER: 120,
+} as const;
+
+// Cache and performance constants
+export const CACHE_CONSTANTS = {
+  /** Cache TTL in milliseconds (1 minute) */
+  CACHE_TTL_MS: 60_000,
+  /** Write debounce delay in milliseconds */
+  WRITE_DEBOUNCE_MS: 400,
+} as const;
+
+// Default values and limits
+export const DEFAULT_VALUES = {
+  /** Default currency */
+  DEFAULT_CURRENCY: 'USD',
+  /** Default billing cycle */
+  DEFAULT_BILLING_CYCLE: 'monthly',
+  /** Maximum length for text truncation */
+  MAX_TEXT_LENGTH: 50,
+  /** Default flow rate buffer percentage */
+  FLOW_RATE_BUFFER_PERCENTAGE: 20,
+} as const;
+
+// Address constants
+export const ADDRESS_CONSTANTS = {
+  /** Zero address for Ethereum */
+  ZERO_ADDRESS: '0x0000000000000000000000000000000000000000',
+  /** Sablier V2 LockupLinear contract address */
+  SABLIER_V2_LOCKUP_LINEAR: '0xAFb979d9afAd1aD27C5eFf4E27226E3AB9e5dCC9',
+} as const;
+
+// Chain ID constants
+export const CHAIN_IDS = {
+  /** Ethereum Mainnet */
+  ETHEREUM: 1,
+  /** Polygon Mainnet */
+  POLYGON: 137,
+  /** Arbitrum Mainnet */
+  ARBITRUM: 42161,
+} as const;
+
+// Formatting constants
+export const FORMATTING_CONSTANTS = {
+  /** Default address start characters */
+  ADDRESS_START_CHARS: 6,
+  /** Default address end characters */
+  ADDRESS_END_CHARS: 4,
+  /** Maximum fraction digits for compact currency */
+  COMPACT_MAX_FRACTION_DIGITS: 1,
+  /** Maximum fraction digits for regular currency */
+  REGULAR_MAX_FRACTION_DIGITS: 2,
+  /** Default crypto decimals for formatting */
+  DEFAULT_CRYPTO_DECIMALS: 18,
+} as const;