Conversation
When an RPC returns a stale block number (fromBlock > toBlock), the previous fix in PR #80 would query the submissions table for the last event block and reset the chain's sync position to that block. The problem: On chains with low debridge activity (like Zilliqa), the last event could be millions of blocks behind the current sync position, causing massive unnecessary resyncs (e.g., 2.9M blocks on Zilliqa). This fix simply uses toBlock as the fallback, which is the RPC's current confirmed block number. This prevents the resync while maintaining consistency with the RPC's view of the chain. The old code already used toBlock when there were NO events - this fix makes the behavior consistent for all cases. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds a dedicated test that verifies the fix for stale RPC responses: - Simulates scenario where DB has latestBlock=200 but RPC returns block 100 - Verifies no events are fetched when fromBlock > toBlock - Verifies latestBlock is reset to toBlock (99) to prevent massive resync Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
fix: prevent massive resyncs when RPC returns stale block number
… jobs firing multiple times
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps the project version to 2.8.4, updates various dependencies, and introduces several improvements to the node's resilience and security. Key changes include refactoring the block scanning logic to better handle stale or invalid RPC responses, implementing application-level timeout guards for Web3 and gRPC requests, and adding a sanitization utility to mask sensitive headers and RPC URLs in logs. Feedback identifies critical issues with non-existent package versions in the "overrides" section of package.json and a potential runtime crash in the logging sanitization logic when encountering BigInt values.
| "overrides": { | ||
| "elliptic": "6.6.1", | ||
| "fastify": "5.8.5", | ||
| "@grpc/grpc-js": "1.8.22", | ||
| "tar": "7.5.13", | ||
| "tough-cookie": "4.1.4", | ||
| "undici": "6.25.0", | ||
| "ws": "8.20.0", | ||
| "form-data": "4.0.5", | ||
| "qs": "6.15.1", | ||
| "axios": "1.15.0" | ||
| }, |
There was a problem hiding this comment.
Several package versions specified in the overrides block (e.g., axios@1.15.0, ws@8.20.0, undici@6.25.0, qs@6.15.1, form-data@4.0.5, tar@7.5.13) do not appear to exist on the public npm registry. Using non-existent versions will cause npm install to fail. Please verify if these are typos or if they refer to a private registry.
|
|
||
| let clone: any; | ||
| try { | ||
| clone = JSON.parse(JSON.stringify(config)); |
There was a problem hiding this comment.
Using JSON.parse(JSON.stringify(config)) will cause a crash if the config object contains BigInt values, which is common in Web3 applications (e.g., in request bodies or parameters). Since this is used for logging, consider a safer way to clone the object or handle BigInt serialization.
There was a problem hiding this comment.
Pull request overview
This PR merges the develop branch into master for the debridge_node service, bumping the version from 2.8.1 to 2.8.4. The changes focus on hardening RPC handling (timeouts, masking secrets in logs), improving error/telemetry behavior, dependency bumps, and significantly expanding test coverage across the codebase (replacing live-network tests with mocked variants and adding regression tests).
Changes:
- Introduces new utilities (
withTimeout,parsePositiveInt,maskRpcUrl,sanitizeHttpLogging) and applies them inWeb3Service,CheckAssetsEventAction, and HTTP logging interceptors to enforce application-level RPC timeouts and prevent leaking credentials/API keys. - Reworks
AddNewEventsActionstale-RPC handling (skip whentoBlock <= 0, choosemax(lastEvent.blockNumber, toBlock)whenfromBlock > toBlock) and improves theLoggerto send properErrorobjects to Sentry. - Modernizes many test suites (Web3Service, SolanaReaderService, TransformService, AddNewEventsAction, SignAction, CheckAssetsEventAction, getEvmTokenName/Symbol, Logger) and bumps several
@nestjs/*,typeorm,@ardrive/turbo-sdkdependencies; adds anoverridesblock pinning transitive deps for security/compatibility.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| debridge_node/package.json | Version bump, dep upgrades, new overrides for transitive security pins |
| debridge_node/src/Logger.ts | Send real Error instances to Sentry; preserve full super.error signature |
| debridge_node/src/utils/withTimeout.ts | New helper wrapping a promise with a timeout |
| debridge_node/src/utils/parsePositiveInt.ts | New helper + DEFAULT_WEB3_TIMEOUT_MS constant |
| debridge_node/src/utils/maskRpcUrl.ts | New helpers to mask credentials/paths in RPC URLs |
| debridge_node/src/modules/external/common/sanitizeHttpLogging.ts | New helpers to redact sensitive HTTP headers when logging |
| debridge_node/src/modules/external/common/addHttpServiceLogging.ts | Use sanitizer in request/response/error interceptors |
| debridge_node/src/modules/web3/services/Web3Service.ts | Application-level timeout wrapper around HttpProvider.send; mask RPC URLs in logs; safer env parsing |
| debridge_node/src/modules/jobs/services/actions/CheckAssetsEventAction.ts | Use withTimeout around Solana gRPC calls; configurable rpcTimeout |
| debridge_node/src/modules/chain/scanning/services/AddNewEventsAction.ts | Robust handling of broken/stale RPC responses; fixed import path |
| debridge_node/src/modules/chain/scanning/ChainScanningModule.ts | Drop duplicate ScheduleModule.forRoot() import |
| debridge_node/src/modules/monitoring/MonitoringModule.ts | Drop duplicate ScheduleModule.forRoot() import |
| debridge_node/src/{tests,utils,modules}/...spec.ts | Replace network-dependent tests with mocked equivalents and add regression/edge-case tests |
| debridge_node/src/modules/chain/config/services/tests/ChainConfigService.spec.ts | Update expected JSON snapshot to include maxAttemptsSubmissionIdCalculation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.