Fix 7 critical/high-severity bugs#67
Open
Mayne-X wants to merge 2 commits into
Open
Conversation
- Fix Express 3 route pattern (/{*splat}) for Express 5 compatibility (nodeapi.js)
- Fix null dereference in JSON-RPC error handler (jsonrpc.js)
- Fix duplicate error listener causing double callback (jsonrpc.js)
- Fix null pointer crash in delete_and_cleanup_tx when tx not found (block_sync.js)
- Fix hardcoded MongoDB/RPC credentials, use env vars only (settings.js)
- Fix global variable leaks across 4 files (explorer.js, database.js, nodeapi.js, block_sync.js)
- Fix operator precedence bug and replace unsafe eval() calls (app.js, routes/index.js)
## Problem
The address page AJAX endpoint (/ext/getaddresstxs/:hash/:start/:length) used an N+1 query pattern: for each of N transactions returned, it issued a separate Tx.findOne() query sequentially via async.eachSeries.
## Impact
For a page showing 50 transactions: 53 MongoDB queries (1 count + 1 aggregate + 1 find + 50 individual lookups).
For 100 transactions: 103 queries.
## Fix
Replace the per-tx find_tx calls with a single Tx.find({txid: {$in: txids}}) batched query using a hash map for O(1) lookup. Keeps identical behavior: same .select('-_id -__v').lean() projection with Decimal128 total conversion.
## Result
- 96% fewer MongoDB round trips (53->4 for 50 txs, 103->4 for 100 txs)
- Eliminates async.eachSeries serial overhead
- Running balance calculation preserved exactly
- Both /ext/getaddress/:hash and /ext/getaddresstxs endpoints benefit
## Additional
- Removed dead $sort in aggregation pipeline (blockindex field already excluded by $project)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RPC API not working at all The /api/getblockcount, /api/getblockhash, etc. pages all returned 404 because the route pattern used old Express 3 syntax that doesn't work on the current Express version. Fixed by updating the route pattern.
Crash when wallet sends weird response If the coin wallet returned an unexpected response (missing both error and result fields), the explorer would crash with "Cannot read property of undefined". Fixed by handling that case properly.
Error handler fires twice When a network error occurred (like wallet being offline), the error handler was called twice, which could corrupt internal state. Fixed by removing the duplicate listener.
Crash when cleaning up missing transactions During block re-sync, if the code tried to clean up a transaction that didn't exist in the database, it crashed with "Cannot read property of null". Fixed by adding a simple null check before proceeding.
Public passwords in the code The settings file contained real-looking passwords for MongoDB and the wallet RPC as default values. Anyone who deployed without setting environment variables would be using publicly known passwords. Fixed by removing the default passwords — you must now set them via environment variables.
Loop variables leaking everywhere Multiple for loops across 4 files used undeclared variables like i, r, b, m which leaked into the global scope. When multiple operations ran at the same time, they would overwrite each other's counters, causing unpredictable data corruption. Fixed by adding let to all loop variables.