subql icon indicating copy to clipboard operation
subql copied to clipboard

feat: implemented dynamic datasource removing themseleves and adding them at end blocks

Open Gmin2 opened this issue 3 months ago β€’ 2 comments

Description

Implemented dynamic datasource removing themseleves and adding them at end blocks

Fixes #2099

Type of change

Please delete options that are not relevant.

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update

Checklist

  • [x] I have tested locally
  • [x] I have performed a self review of my changes
  • [ ] Updated any relevant documentation
  • [x] Linked to any relevant issues
  • [x] I have added tests relevant to my changes
  • [ ] Any dependent changes have been merged and published in downstream modules
  • [x] My code is up to date with the base branch
  • [ ] I have updated relevant changelogs. We suggest using chan

Summary by CodeRabbit

  • New Features

    • Add ability to destroy dynamic datasources by template and index, and to query active datasources by template.
    • Introduce optional endBlock on datasources for explicit lifecycle termination and improved indexing consistency.
    • Expose destroy/get dynamic-datasource operations to runtime/worker host functions and global bindings for in-block updates.
  • Tests

    • Expanded tests covering destruction flows, endBlock behavior, template-scoped queries, error cases, and in-memory state consistency.

Gmin2 avatar Sep 04 '25 21:09 Gmin2

@stwiname just implemented listing of dynamic datasources and to remove it by index no

Gmin2 avatar Oct 17 '25 06:10 Gmin2

Walkthrough

Adds lifecycle controls for dynamic datasources: datasources gain optional endBlock; services, indexer manager, worker host/VM bindings, and types expose destroy/get operations by template/index; tests expanded to validate destruction, endBlock propagation, and in-memory/persisted metadata consistency.

Changes

Cohort / File(s) Change Summary
Type Definitions
packages/types-core/src/interfaces.ts
Added DynamicDatasourceDestructor, DynamicDatasourceGetter, and DynamicDatasourceInfo types/interfaces describing destructor signature and datasource metadata (index, templateName, startBlock, optional endBlock, args).
Global API Exports
packages/types-core/src/global.ts
Exposed global VM handler bindings destroyDynamicDatasource and getDynamicDatasources (typed to new interfaces).
Core Dynamic DS Service
packages/node-core/src/indexer/dynamic-ds.service.ts
Added optional endBlock?: number to DatasourceParams; extended public API with destroyDynamicDatasource, getDynamicDatasourcesByTemplate, getDatasourceParamByIndex; updated getTemplate/getDatasource to accept/propagate endBlock; implemented destruction flow that sets endBlock, updates in-memory state, persists metadata, and validates template/index consistency.
Indexer VM Sandbox
packages/node-core/src/indexer/indexer.manager.ts
Injected VM functions getDynamicDatasources(templateName) and destroyDynamicDatasource(templateName, blockHeight, index) bound to dynamic DS service; destruction updates datasource endBlock in filteredDataSources so subsequent handlers in the same block respect endBlock.
Worker Dynamic DS Support
packages/node-core/src/indexer/worker/worker.dynamic-ds.service.ts
Extended HostDynamicDS interface and hostDynamicDsKeys with dynamicDsDestroyDynamicDatasource, dynamicDsGetDynamicDatasourcesByTemplate, and dynamicDsGetDatasourceParamByIndex; added corresponding methods on WorkerDynamicDsService and bound them in dynamicDsHostFunctions.
Node Indexer Flow
packages/node/src/indexer/indexer.manager.ts
Threaded blockHeight into indexContent calls and added guard to skip processing for datasources with endBlock defined and <= current blockHeight.
Tests
packages/node-core/src/indexer/dynamic-ds.service.spec.ts
Expanded test project templates to include a second template and testParamOther; updated test helper getTemplate to accept endBlock; added tests covering destruction (valid, non-existent, already-destroyed), creation after destruction, resets, endBlock propagation, multi-template indexing, and in-memory _datasourceParams/_datasources consistency.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Handler as Handler
    participant VM as VM Sandbox
    participant Manager as Indexer Manager
    participant Service as DynamicDsService
    participant DB as Metadata Store

    Handler->>VM: destroyDynamicDatasource(templateName, index)
    VM->>Manager: destroyDynamicDatasource(templateName, blockHeight, index)
    Manager->>Service: destroyDynamicDatasource(templateName, blockHeight, index)
    Service->>Service: getDatasourceParamByIndex(index)
    alt found & active
        Service->>Service: set param.endBlock = blockHeight
        Service->>Service: update in-memory datasource (endBlock)
        Service->>DB: persist updated params (metadata)
        Service-->>Manager: success
        Manager->>Manager: re-filter filteredDataSources (apply endBlock)
        Manager-->>VM: ack
        VM-->>Handler: continue (datasource excluded for future handlers)
    else not found / already destroyed
        Service-->>Manager: throw error
        Manager-->>VM: propagate error
        VM-->>Handler: error returned
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review synchronization between updating _datasourceParams and _datasources and persistence steps.
  • Validate index vs template-name checks and error messages for off-by-one or template-local/global index mismatches.
  • Confirm VM/worker host binding names and cross-thread delegation correctness.
  • Check indexContent gating logic to ensure datasources with endBlock are skipped exactly when intended.
  • Inspect tests for coverage correctness and for any brittle assumptions about ordering/indices.

Poem

πŸ‡ I nibble code and mark the end,

a little hop, a tidy friend.
Templates closed when blocks align,
metadata neat, indices fine.
Hooray β€” the lifecycle's now in line!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues check ❓ Inconclusive The PR implements core requirements from #2099: destructor injection, endBlock support, and methods to list/query dynamic datasources. However, documentation, codegen updates, and example project are not evidenced in code changes. Confirm that documentation updates, codegen changes, and example project have been addressed in separate PRs or branches, or document why they were deferred.
βœ… Passed checks (3 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The title accurately describes the main feature: allowing dynamic datasources to remove themselves and be added at end blocks.
Out of Scope Changes check βœ… Passed All code changes directly support the feature requirements: new methods for destroying datasources, endBlock handling, interface extensions, and VM injection for handler access.
✨ Finishing touches
  • [ ] πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

πŸ“œ Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 87dca656fe6ae553ba8bd5c074916a68ab406278 and 2d3559179da6079f6f080d3739b9dbbd994063d9.

πŸ“’ Files selected for processing (2)
  • packages/node-core/src/indexer/dynamic-ds.service.spec.ts (6 hunks)
  • packages/node-core/src/indexer/indexer.manager.ts (1 hunks)
🧰 Additional context used
πŸ““ Path-based instructions (3)
**/*.{ts,tsx}

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use TypeScript throughout with strict configuration in the monorepo Use Ethers.js for Ethereum integration

Files:

  • packages/node-core/src/indexer/indexer.manager.ts
  • packages/node-core/src/indexer/dynamic-ds.service.spec.ts
packages/**/src/**/*.{ts,tsx}

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Run ESLint with TypeScript support, Prettier, and various plugins across all TypeScript files using configured husky pre-commit hooks and lint-staged

Files:

  • packages/node-core/src/indexer/indexer.manager.ts
  • packages/node-core/src/indexer/dynamic-ds.service.spec.ts
**/*.{test,spec}.{ts,tsx}

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Use custom Jest configuration with module name mapping for workspace packages, enforce UTC timezone for tests, and handle Polkadot packages in transform ignore patterns

Files:

  • packages/node-core/src/indexer/dynamic-ds.service.spec.ts
🧬 Code graph analysis (1)
packages/node-core/src/indexer/dynamic-ds.service.spec.ts (1)
packages/node-core/src/indexer/dynamic-ds.service.ts (1)
  • DatasourceParams (18-23)
πŸ”‡ Additional comments (12)
packages/node-core/src/indexer/indexer.manager.ts (2)

119-122: LGTM: getDynamicDatasources injection is clean.

The VM freeze correctly exposes the service method to retrieve datasources by template name.


124-129: Critical issue: Destroyed datasource remains in filteredDataSources for current block.

Setting endBlock doesn't remove the datasource from the filteredDataSources array during the current block's processing. The comment "child class implementation should check ds.endBlock" pushes responsibility to subclasses, which is fragile and error-prone.

Issues:

  1. indexBlockData (line 101) iterates over filteredDataSources, and setting endBlock doesn't affect the ongoing iteration.
  2. Subsequent handlers in the same block will still process the destroyed datasource unless they manually check endBlock.
  3. filterDataSources (line 154) uses >= nextProcessingHeight, so datasources with endBlock === blockHeight are included.

This issue was previously flagged in past reviews but appears unresolved.

Solution: Remove the destroyed datasource from filteredDataSources in-place to prevent subsequent handlers in this block from processing it.

packages/node-core/src/indexer/dynamic-ds.service.spec.ts (10)

18-19: LGTM: getTemplate signature updated for endBlock support.

The test helper correctly exposes the protected method with the new endBlock parameter.


27-27: LGTM: Additional test constant for multi-template scenarios.

The testParamOther constant enables testing cross-template operations.


74-87: LGTM: Basic destruction test is comprehensive.

The test correctly verifies that destroying a datasource sets its endBlock and leaves other datasources unaffected.


89-106: LGTM: Error handling tests cover key edge cases.

Both tests validate appropriate error messages for invalid destruction attempts.


108-135: LGTM: Lifecycle test validates destroy-then-create flow.

The test correctly verifies that destroyed datasources remain in history while new ones are added.


150-168: LGTM: Reset behavior correctly preserves destruction state.

The test validates that reset operations don't undo previous destructions.


194-216: LGTM: Metadata and persistence tests validate endBlock handling.

Both tests correctly verify that endBlock values persist and load from metadata.


231-237: LGTM: Template creation with endBlock is tested.

The test correctly validates the new endBlock parameter in template creation.


239-260: LGTM: Edge case tests for initialization and multi-template scenarios.

Tests correctly validate index-based destruction across templates and initialization requirements.


262-457: LGTM: Comprehensive test coverage for dynamic datasource lifecycle.

The test suites thoroughly validate:

  • Datasource retrieval by template with proper filtering of destroyed datasources
  • Index-based destruction with global index handling
  • Error conditions: out of bounds, negative indices, template mismatches, already destroyed
  • In-memory state consistency across operations
  • Cross-template operations
  • Metadata persistence

The test coverage exceeds the β‰₯80% requirement from the PR objectives and validates all critical paths.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Oct 23 '25 10:10 coderabbitai[bot]

Take a look @stwiname

Gmin2 avatar Dec 01 '25 22:12 Gmin2