search

autonomys / astral

Home of our Block Explorer
https://explorer.subspace.network
11 stars 9 forks source link

Prevent indexing too close to the head of the chain #903

Closed marc-aurele-besner closed 1 week ago

marc-aurele-besner commented 1 week ago

User description

Prevent indexing too close to the head of the chain

If the current block being indexed is less than 100 blocks from the head of the chain, we throw an error, forcing the indexer worker to bail on this set of blocks and re-spawn, this is a dirty fix but will prevent the index too close to the head.

PR Type

enhancement, bug fix

Description

Changes walkthrough 📝

Relevant files
Enhancement
helper.ts
Add function to prevent indexing near chain head                 
indexers/taurus/consensus/src/mappings/helper.ts
  • Added preventIndexingTooCloseToTheHeadOfTheChain function.
  • Included blockNumber and ApiPromise imports.
  • Implemented logic to prevent indexing close to the head of the chain.
    		•  +19/-2   
    mappingHandlers.ts
    Integrate indexing prevention in mapping handlers               
    indexers/taurus/consensus/src/mappings/mappingHandlers.ts
  • Integrated preventIndexingTooCloseToTheHeadOfTheChain function in
    block, call, and event handlers.
  • Added height conversion to BigInt.
  • Ensured indexing is skipped if too close to the chain head.
    		•  +13/-3   

    💡 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 canceled.

    Name Link
    Latest commit fa659e83fc6176c6f4fb5ece2710846433786c7e
    Latest deploy log https://app.netlify.com/sites/dev-astral/deploys/6724d18201cf66000828cf6c
    github-actions[bot] commented 1 week ago

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling
    The error message "Unsafe API not found" is vague and might not provide enough context for debugging. Consider including more details about what the unsafe API refers to or how to resolve the issue. Type Checking
    The function `preventIndexingTooCloseToTheHeadOfTheChain` performs redundant type checks on `indexingBlockHeight`. Since TypeScript supports type definitions, consider enforcing the type at the function parameter level to simplify the code. Exception Handling
    The error "Indexing too close to the head of the chain, skipping..." could be handled more gracefully than throwing an error, perhaps by logging a warning and skipping the indexing operation without disrupting the flow.
    github-actions[bot] commented 1 week ago

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Ensure consistent data types in comparisons to prevent logical errors ___ **Convert indexingBlockHeight to BigInt before comparing it to targetHeight to ensure
    consistent data types for comparison.** [indexers/taurus/consensus/src/mappings/helper.ts [86]](https://github.com/autonomys/astral/pull/903/files#diff-f0ffacaa27f2236bc750ead02caf99cd374df87d2a9102a7250742856250fb02R86-R86) ```diff -if (indexingBlockHeight > BigInt(targetHeight - 100)) +if (BigInt(indexingBlockHeight) > BigInt(targetHeight - 100)) ```
    Suggestion importance[1-10]: 8 Why: This suggestion is valuable as it ensures consistent data types during comparison, which can prevent potential logical errors. Converting `indexingBlockHeight` to `BigInt` before comparison enhances the robustness of the code.
    		8
    Simplify type casting to enhance code clarity and type safety ___ **Use ApiPromise directly instead of casting unsafeApi as unknown and then to
    ApiPromise to simplify the code and improve type safety.** [indexers/taurus/consensus/src/mappings/helper.ts [85]](https://github.com/autonomys/astral/pull/903/files#diff-f0ffacaa27f2236bc750ead02caf99cd374df87d2a9102a7250742856250fb02R85-R85) ```diff -const targetHeight = await blockNumber(unsafeApi as unknown as ApiPromise); +const targetHeight = await blockNumber(unsafeApi); ```
    Suggestion importance[1-10]: 5 Why: While simplifying type casting can improve code clarity, the suggestion assumes `unsafeApi` is already of type `ApiPromise`. Without additional context, this change could introduce type safety issues if `unsafeApi` is not guaranteed to be of type `ApiPromise`.
    		5
    Maintainability
    Use a constant instead of a magic number to clarify the purpose of the value ___ **Replace the magic number 100 with a well-named constant to improve code readability
    and maintainability.** [indexers/taurus/consensus/src/mappings/helper.ts [86]](https://github.com/autonomys/astral/pull/903/files#diff-f0ffacaa27f2236bc750ead02caf99cd374df87d2a9102a7250742856250fb02R86-R86) ```diff -if (indexingBlockHeight > BigInt(targetHeight - 100)) +const SAFE_DISTANCE_FROM_HEAD = 100; +if (indexingBlockHeight > BigInt(targetHeight - SAFE_DISTANCE_FROM_HEAD)) ```
    Suggestion importance[1-10]: 7 Why: Replacing the magic number with a named constant improves code readability and maintainability by clarifying the purpose of the value. This is a good practice for enhancing code clarity.
    		7