1.3.4-STG #353
1.3.4-STG #353
Conversation
fix: show future terms correctly
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| ); | ||
| const lmrCoinPrice = param[1]; | ||
| const btcCoinPrice = param[2]; | ||
| const reward = formatBtcPerTh(param[4]); | ||
|
|
||
| const networkDifficulty = param[4]; | ||
| const blockReward = param[5]; | ||
| const reward = formatBtcPerTh(networkDifficulty, blockReward); | ||
| const deviation = +settings.deviation; |
There was a problem hiding this comment.
The code fragment uses array indexing directly (param[1], param[2], etc.) to access values passed to a function. This approach is error-prone and makes the code hard to read and maintain, as it's not immediately clear what each index represents without additional context. It's recommended to use object destructuring when dealing with multiple parameters to enhance code readability and maintainability. This way, each parameter can be named, making the code self-documenting and reducing the risk of accessing the wrong index.
| count: contractsToShow.length ?? 0, | ||
| rented: rentedContracts.reduce(speedReducer, 0), | ||
| totalPosted: contractsToShow.reduce(speedReducer, 0), | ||
| networkReward: formatBtcPerTh(networkDifficulty) | ||
| networkReward: formatBtcPerTh(networkDifficulty, blockReward) | ||
| }; | ||
| const showArchive = deadContracts?.length; | ||
| const onArchiveOpen = () => setIsArchiveModalActive(true); |
There was a problem hiding this comment.
The use of the optional chaining operator (?.) and the nullish coalescing operator (??) in line 486 suggests that there might be a scenario where contractsToShow is undefined. However, the same level of safety is not applied when using reduce on rentedContracts in line 487, which could potentially lead to a runtime error if rentedContracts is undefined. To ensure consistency and prevent potential runtime errors, it's recommended to either ensure that rentedContracts is always an array before this point in the code or to apply the same level of safety checks (?.reduce(...)) as done with contractsToShow.
| const speed = contract.futureTerms?.speed || contract.speed; | ||
| const length = contract.futureTerms?.length || contract.length; | ||
| const price = contract.futureTerms?.price || contract.price; | ||
| const version = contract.futureTerms?.version || contract.version; |
There was a problem hiding this comment.
The code is using a fallback mechanism to assign values to speed, length, price, and version by checking if contract.futureTerms exists and then using its properties or falling back to contract's properties. This approach can lead to unexpected behavior if contract.futureTerms is defined but any of its properties (speed, length, price, version) are explicitly set to undefined or null, as the fallback will not trigger in these cases, potentially leading to the use of undefined values.
To ensure that the fallback mechanism works as intended even when contract.futureTerms is defined but contains undefined or null values for any of the properties, it's recommended to use a more robust way of assigning fallback values that checks for undefined or null explicitly.
| @@ -79,9 +84,9 @@ | |||
| <ContractValue onClick={() => window.openLink(explorerUrl)}> | |||
There was a problem hiding this comment.
The use of window.openLink directly within an onClick handler without any validation or sanitization of the explorerUrl can pose a security risk, particularly if explorerUrl is derived from user input or external sources. This could potentially expose the application to cross-site scripting (XSS) attacks if explorerUrl contains malicious JavaScript code.
To mitigate this risk, it's recommended to validate and sanitize explorerUrl before using it in window.openLink. Additionally, consider using a more secure method to open external links that ensures only valid URLs are processed and reduces the risk of executing malicious code.
| {isLmrSelected(converters.price, selectedCurrency) | ||
| ? `${formatPrice(contract.price, 'LMR')}` | ||
| : `${convertLmrToBtc(contract.price, btcRate, lmrRate).toFixed( | ||
| 10 | ||
| )} BTC`} | ||
| ? `${formatPrice(price, 'LMR')}` | ||
| : `${convertLmrToBtc(price, btcRate, lmrRate).toFixed(10)} BTC`} |
There was a problem hiding this comment.
The ternary operation inside the JSX can lead to performance issues and makes the code harder to read. This operation involves calling functions and performing a conditional check to decide which string to render. Performing such operations directly inside the JSX can cause unnecessary re-renders because the function calls and conditional checks are executed every time the component re-renders.
Recommended Solution: It would be more efficient and cleaner to calculate this value before the return statement of the render function and store it in a variable. Then, simply use this variable inside the JSX. This approach will make the code more readable and improve performance by avoiding unnecessary calculations during each render.
| export const formatBtcPerTh = (networkDifficulty, blockReward) => { | ||
| const networkHashrate = | ||
| (networkDifficulty * Math.pow(2, 32)) / 600 / Math.pow(10, 12); | ||
|
|
||
| const miningHours = 24; | ||
| const blockReward = 6.25; | ||
| const hashrate = 1; // TH/s | ||
| const blocksExpected = 1 / 0.1667; // 1 block every 10 minutes |
There was a problem hiding this comment.
The calculation of networkHashrate in this function does not account for potential floating-point arithmetic issues, which can lead to inaccurate results due to the way JavaScript handles floating-point numbers. This can significantly affect the accuracy of the hashrate calculation, especially with large numbers involved in cryptocurrency computations. To improve accuracy, consider using a library designed for arbitrary-precision arithmetic to handle these calculations.
| const networkHashrate = | ||
| (networkDifficulty * Math.pow(2, 32)) / 600 / Math.pow(10, 12); | ||
|
|
||
| const miningHours = 24; | ||
| const blockReward = 6.25; | ||
| const hashrate = 1; // TH/s | ||
| const blocksExpected = 1 / 0.1667; // 1 block every 10 minutes |
There was a problem hiding this comment.
The variable hashrate is set to a constant value of 1 (TH/s) without any explanation or context. This hardcoding can limit the function's flexibility and usability, as users might need to calculate profits for different hashrates. It's recommended to modify the function to accept hashrate as a parameter, allowing for more versatile and practical usage.
halving ready
future terms fix