Magic number

[Mati0x]

#406

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
gardens-v2	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 26, 2024 6:32pm

[github-actions]

Review of Changes

This PR introduces a potential issue in how spending limits are calculated. It appears to be attempting to use a different formula, but the implementation is incomplete and may lead to incorrect results.

File: apps/web/components/Forms/PoolForm.tsx

Issue:

• A new variable maxRatioNum2 is introduced but never used in the subsequent calculation of weightNum. This suggests either an incomplete implementation or dead code.

Suggested Fix:

• Clarify the intended use of maxRatioNum2.
• If the intention is to use this new calculation for weightNum, update the formula accordingly. If not, remove the unused variable.

// apps/web/components/Forms/PoolForm.tsx
const maxRatioNum = spendingLimit / MAX_RATIO_CONSTANT;
// Remove or clarify the purpose of this calculation
// const maxRatioNum2 = spendingLimit / (1 - Math.sqrt(minimumConviction)); 

const weightNum = minimumConviction * maxRatioNum ** 2; 

File: apps/web/components/PoolHeader.tsx

Issue:

• The original calculation of spendingLimit is commented out and replaced with a new formula. However, it's unclear if this new formula is intended to be a direct replacement or serves a different purpose.

Suggested Fix:

• Provide context for the change in spendingLimit calculation.
• Explain the reasoning behind the new formula and its relation to the original.
• If the new calculation is intended to replace the old one, ensure it produces the expected results and doesn't introduce logic errors.

// apps/web/components/PoolHeader.tsx
// Explain why this is commented out and the reasoning behind the new calculation
// const spendingLimit = spendingLimitPct * MAX_RATIO_CONSTANT; 
const spendingLimit = 
  (strategy.config.maxRatio / CV_SCALE_PRECISION) * 
  (1 - Math.sqrt(minimumConviction / 100)) * 
  100; 

Review Summary

This PR needs clarification on the intended changes to the spending limit calculation. The addition of unused variables and commented-out code without explanation makes the intent unclear and raises concerns about potential errors.

Please address the issues outlined above to ensure the changes are correctly implemented and the code remains maintainable.

[Corantin]

GG