fix: add cycle block cache to InstantSendManager

[HashEngineering]

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

Summary by CodeRabbit

• Refactor
  • Improved instant send transaction verification performance through enhanced block lookup caching, reducing database scan overhead.

[coderabbitai]

📝 Walkthrough

Walkthrough

InstantSendManager adds two LRU-bounded, synchronized caches for deterministic islock cycle hash block lookups. A new getCycleBlock() helper centralizes cached retrieval logic, avoiding repeated expensive BlockStore.get() scans. The helper is integrated into islock processing and signature verification, with cache misses invalidated when blocks connect.

Changes

Cycle Hash Block Lookup Caching

Layer / File(s)	Summary
Cache Infrastructure and Lifecycle core/src/main/java/org/bitcoinj/quorums/InstantSendManager.java	LinkedHashMap import added. Two synchronized LRU caches (cycleHashCache for found blocks, cycleHashNotFoundCache for misses) are initialized with fixed size bounds and removeEldestEntry eviction. Both caches are cleared on shutdown to prevent stale state.
Cached Cycle Block Lookup Helper core/src/main/java/org/bitcoinj/quorums/InstantSendManager.java	New getCycleBlock(cycleHash) method returns cached blocks when present, returns null immediately for known-missing cycle hashes, otherwise queries the block store and caches either the found block or a miss entry with synchronized access.
Integration into Islock Processing and Cache Invalidation core/src/main/java/org/bitcoinj/quorums/InstantSendManager.java	Deterministic islock processing and signature verification call cached getCycleBlock() instead of direct store lookup. NewBestBlockListener removes newly connected block hashes from the "not found" cache, allowing subsequent deterministic islock processing to retry the lookup.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 Locks cache the blocks, misses cache too,
No more scanning the store for what we knew.
New blocks invalidate the "not found" dream,
InstantSend hops along, faster than it seems.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: add cycle block cache to InstantSendManager' directly and accurately describes the main change in the pull request: adding LRU-style caching for cycle block lookups in the InstantSendManager class.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/instantsend-cycle-block-cache

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

core/src/main/java/org/bitcoinj/quorums/InstantSendManager.java (1)

231-250: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add NPE handling for shutdown race condition.

The getCycleBlock() call at line 233 can throw NullPointerException if blockChain becomes null during shutdown. Line 624 has explicit NPE handling for this case, but this location does not. During a shutdown race condition, the NPE will propagate to the network thread, potentially causing errors.

🛡️ Proposed fix to add NPE handling

         if (isLock.isDeterministic()) {
             try {
                 StoredBlock blockIndex = getCycleBlock(isLock.cycleHash);
                 if (blockIndex == null) {
                     // Maybe we don't have the block yet or maybe some peer spams invalid values for cycleHash
                     // TODO: DashCore increases ban score by 1
                     return;
                 }

                 final LLMQParameters.LLMQType llmqType = context.getParams().isDIP0024Active(blockIndex) ?
                         context.getParams().getLlmqDIP0024InstantSend() :
                         context.getParams().getLlmqForInstantSend();
                 final int dkgInterval = LLMQParameters.fromType(llmqType).dkgInterval;
                 if (blockIndex.getHeight() % dkgInterval != 0) {
                     // TODO: Dash Core increases ban score by 100
                     return;
                 }
             } catch (BlockStoreException x) {
                 throw new RuntimeException(x);
+            } catch (NullPointerException e) {
+                // blockchain is now null?
+                if (isInitialized()) {
+                    throw e;
+                } else {
+                    return;
+                }
             }
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/src/main/java/org/bitcoinj/quorums/InstantSendManager.java` around lines
231 - 250, In InstantSendManager, wrap the getCycleBlock(isLock.cycleHash)
invocation in a try/catch that also catches NullPointerException (similar to the
handling at line 624) so a shutdown race where blockChain becomes null won't
propagate an NPE to the network thread; specifically, around the code that calls
getCycleBlock and the subsequent logic for llmqType/dkgInterval, add a catch
(NullPointerException npe) that logs or silently returns (same safe behavior
used elsewhere) and then keep the existing catch (BlockStoreException) that
throws a RuntimeException.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@core/src/main/java/org/bitcoinj/quorums/InstantSendManager.java`:
- Around line 231-250: In InstantSendManager, wrap the
getCycleBlock(isLock.cycleHash) invocation in a try/catch that also catches
NullPointerException (similar to the handling at line 624) so a shutdown race
where blockChain becomes null won't propagate an NPE to the network thread;
specifically, around the code that calls getCycleBlock and the subsequent logic
for llmqType/dkgInterval, add a catch (NullPointerException npe) that logs or
silently returns (same safe behavior used elsewhere) and then keep the existing
catch (BlockStoreException) that throws a RuntimeException.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b56c5f35-b959-464d-a792-05b8b2122d7e

📥 Commits

Reviewing files that changed from the base of the PR and between fece115 and 648e54b.

📒 Files selected for processing (1)

• core/src/main/java/org/bitcoinj/quorums/InstantSendManager.java