Sam/correctness issues #13
Conversation
- Use Pino for performant logging - Use Pino's .child() pattern for efficient scoped logging - Add pino-pretty support for human-readable development output - Disable logging in tests via NODE_ENV check - Automatically rename conflicting fields (time → time_, scope → scope_)
Remove the following plugins to reduce load: - abstract, amoy, arbitrum, avalanche, base - bobevm, celo, ethereumclassic, ethereumpow, fantom - filecoinfevm, filecoinfevmcalibration, holesky, hyperevm - pulsechain, rsk, sepolia, sonic
Add serviceKeysFromUrl utility that matches API keys to URLs by hostname
with subdomain fallback support (e.g., api.example.com → example.com).
Keys can be configured with or without ports.
- Add src/util/serviceKeys.ts with ServiceKeys type and matching logic
- Add src/util/replaceUrlParams.ts for {{param}} placeholder substitution
- Add src/util/authenticateUrl.ts to combine key lookup with URL templating
- Add comprehensive tests for serviceKeysFromUrl
Refactor RPC and scan adapter configuration:
- Switch to authenticated RPC endpoints using {{apiKey}} templates
- Make scanAdapters required in EvmRpcOptions
- Update EtherscanV1/V2 adapters to use serviceKeysFromUrl
- Fix pickRandom return type to T | undefined for empty arrays
- Improve error handling when no URLs or API keys are configured
This reduces complexity and re-uses the new infrastructure. The only trade-off is to remove the logging of the URL without the key, which we can accept since these logs should be treated as company sensitive.
Add txlistinternal action to EtherscanV1ScanAdapter for parity with V2. This enables detection of contract-to-contract calls that were previously missed by the V1 adapter.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| .then(result => { | ||
| if (result.subscribed) { | ||
| logger.log('Block connection initialized') | ||
| logger.info({ scope: 'foo' }, 'Block connection initialized') |
There was a problem hiding this comment.
Accidentally committed debug value in logger call
Medium Severity
The log call passes { scope: 'foo' } which appears to be leftover debug code. The scope field is reserved by the logger (as documented in logger.ts), and the value 'foo' is clearly a placeholder. This will cause the log output to include a warning field about the reserved scope field being overwritten, resulting in confusing log messages like "Warning: 'scope' field because it's reserved for logging context": "foo".
| // Shuffles array and returns a new array. | ||
| export const shuffleArray = <T>(array: T[]): T[] => { | ||
| return array.sort(() => Math.random() - 0.5) | ||
| } |
There was a problem hiding this comment.
shuffleArray mutates original array despite documentation
Medium Severity
The shuffleArray function's comment states it "returns a new array," but Array.sort() mutates the original array in place and returns a reference to the same array. In evmRpc.ts, this means scanAdapters (from the plugin options) is mutated on every call to scanAddress, potentially causing unexpected side effects if the configuration is shared or inspected elsewhere.
Additional Locations (1)
| const apiKey = apiKeys == null ? undefined : pickRandom(apiKeys) | ||
| if (apiKey != null) { | ||
| params.set('apikey', apiKey) | ||
| } |
There was a problem hiding this comment.
API key persists across retry attempts to different URLs
Medium Severity
In the retry loop, the code only sets params.set('apikey', apiKey) when an API key is found, but doesn't remove the key when one isn't found. Since serviceKeysFromUrl returns an empty array (not null) when no keys exist, pickRandom([]) returns undefined, and the conditional if (apiKey != null) fails without clearing the previous key. If a retry picks a different URL with no configured API key, the previous iteration's API key remains in params and gets sent to the wrong service.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
Description
noneNote
Introduces structured logging and flexible auth plus transport improvements, with targeted plugin updates and stronger scans.
makeLogger, adding scoped (server,socket,blockbook,evmRpc) and IP/sockeId context; adddocs/logging.md; update server/hub for connection/subscription/update/close/error events.serviceKeysconfig andauthenticateUrlwith{{apiKey}}replacement and flexible host matching; removenowNodesApiKey; use templated URLs in Blockbook and EVM RPC plugins.getLogsretries, internal transfer tracing, andscanAddressadapter fallback/shuffle.serviceKeysFromUrl, robust JSON cleaning, rate-limit backoff/retries, and startblock =checkpoint + 1.pinodependency and minor utilities (addressUtils,serviceKeys,replaceUrlParams).Written by Cursor Bugbot for commit 24d80de. This will update automatically on new commits. Configure here.