Improve chain offset logic

marc-aurele-besner commented 1 week ago

User description

Improve chain offset logic

Add a constant, change to 10 instead of 100

ENV are not directly available inside the handler

PR Type

enhancement

Description

Introduced a constant DEFAULT_CHAIN_HEAD_OFFSET set to 10 for better maintainability.
Replaced the hardcoded value of 100 with DEFAULT_CHAIN_HEAD_OFFSET in the function preventIndexingTooCloseToTheHeadOfTheChain.
Improved the readability and flexibility of the chain offset logic.

Changes walkthrough 📝

Relevant files

Enhancement

helper.ts

Refactor chain offset logic to use a constant

indexers/taurus/consensus/src/mappings/helper.ts

Introduced a new constant DEFAULT_CHAIN_HEAD_OFFSET with a value of
10.

Replaced the hardcoded value of 100 with DEFAULT_CHAIN_HEAD_OFFSET in
the logic.

+3/-1

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

netlify[bot] commented 1 week ago

Deploy Preview for dev-astral ready!

Name	Link
Latest commit	0b91d9c02981038b9f08d93d9fc6a39fa0e0160c
Latest deploy log	https://app.netlify.com/sites/dev-astral/deploys/672aa4c08310650008c323c0
Deploy Preview	https://deploy-preview-905--dev-astral.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

github-actions[bot] commented 1 week ago

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Magic Number
The constant `DEFAULT_CHAIN_HEAD_OFFSET` is introduced without explanation for its value (10). Consider adding documentation or comments explaining why this value is chosen.

github-actions[bot] commented 1 week ago

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible bug	Prevent negative values in block height calculations by ensuring sufficient target height ___ Add a check to ensure `targetHeight` is greater than `DEFAULT_CHAIN_HEAD_OFFSET` before subtracting to avoid potential negative values in block height calculations. [indexers/taurus/consensus/src/mappings/helper.ts [88]](https://github.com/autonomys/astral/pull/905/files#diff-f0ffacaa27f2236bc750ead02caf99cd374df87d2a9102a7250742856250fb02R88-R88) ```diff -if (indexingBlockHeight > BigInt(targetHeight - DEFAULT_CHAIN_HEAD_OFFSET)) +if (targetHeight <= DEFAULT_CHAIN_HEAD_OFFSET) throw new Error("Target height must be greater than chain head offset"); ``` Suggestion importance[1-10]: 8 Why: Adding a check to ensure 'targetHeight' is greater than 'DEFAULT_CHAIN_HEAD_OFFSET' is critical to avoid negative values in block height calculations, which could lead to runtime errors or incorrect behavior.	8
Possible issue	Ensure the chain head offset is a positive integer to avoid calculation errors ___ Consider validating `DEFAULT_CHAIN_HEAD_OFFSET` to ensure it is a positive integer to prevent logical errors in chain head offset calculations. [indexers/taurus/consensus/src/mappings/helper.ts [8]](https://github.com/autonomys/astral/pull/905/files#diff-f0ffacaa27f2236bc750ead02caf99cd374df87d2a9102a7250742856250fb02R8-R8) ```diff -const DEFAULT_CHAIN_HEAD_OFFSET = 10; +if (DEFAULT_CHAIN_HEAD_OFFSET <= 0) throw new Error("Chain head offset must be a positive integer"); ``` Suggestion importance[1-10]: 7 Why: Validating the 'DEFAULT_CHAIN_HEAD_OFFSET' as a positive integer is crucial to avoid logical errors in subsequent calculations. This suggestion enhances the robustness of the code by preventing invalid configurations.	7
Enhancement	Add logging to provide context before throwing errors for easier debugging ___ Consider adding logging before throwing errors to aid in debugging and provide more context about the error conditions. [indexers/taurus/consensus/src/mappings/helper.ts [89]](https://github.com/autonomys/astral/pull/905/files#diff-f0ffacaa27f2236bc750ead02caf99cd374df87d2a9102a7250742856250fb02R89-R89) ```diff +console.error(`Indexing block height: ${indexingBlockHeight}, Target height: ${targetHeight}, Offset: ${DEFAULT_CHAIN_HEAD_OFFSET}`); throw new Error("Indexing too close to the head of the chain, skipping..."); ``` Suggestion importance[1-10]: 5 Why: Adding logging before error throwing can significantly aid in debugging by providing more context about the error conditions. This is a useful enhancement, though not as critical as preventing errors or incorrect calculations.	5

autonomys / astral