fix(jsonrpc): enforce log filter cap and improve match efficiency

[317787106]

Problem

Three issues exist in the JSON-RPC log filter subsystem (eth_newFilter / eth_getLogs), reported in #6510:

1. Unbounded memory growth — eth_newFilter imposes no limit on the number of active filters held in memory. A client can register filters indefinitely and eventually exhaust heap on the node.

2. Correctness bug — LogMatch.matchBlockOneByOne evaluated the MAX_RESULT guard after addAll, so the result list could transiently contain more than MAX_RESULT entries before the exception was thrown.

3. Performance bottleneck under high filter count — handleLogsFilter iterated the filter map with an Iterator and called result.add() once per element, which is slow and contention-prone when thousands of filters are active.

---

Changes

1. Enforce a configurable cap on active log filters

Adds node.jsonrpc.maxLogFilterNum (default: 20000). When the cap is reached, eth_newFilter immediately returns JsonRpcExceedLimitException (JSON-RPC code -32005) instead of growing without bound.

node {
  jsonrpc {
    # Maximum number of concurrent eth_newFilter registrations (0 = unlimited)
    maxLogFilterNum = 20000
  }
}

2. Fix the MAX_RESULT boundary check in LogMatch.matchBlockOneByOne (correctness)

Moves the size + matchedLog.size() > MAX_RESULT guard before addAll. The result list now never exceeds MAX_RESULT entries regardless of how many logs a single block contributes.

3. Optimize handleLogsFilter for large filter maps

Condition	Before	After
filter count ≤ 10,000 (common case)	Iterator loop, result.add() per element	ConcurrentHashMap.forEach with local list + single addAll
filter count > 10,000	same slow path	bounded ForkJoinPool(3) + parallelStream

Key improvements:

• Bounded parallelism: logsFilterPool is a ForkJoinPool(3) created once per TronJsonRpcImpl instance and shut down with it, avoiding unbounded thread creation under spike load.
• Observability: a logger.debug timing line records dispatch cost and filter-map size.

4. Decouple filter state from static class scope

The four filter maps (eventFilter2ResultFull, blockFilter2ResultFull, eventFilter2ResultSolidity, blockFilter2ResultSolidity), logsFilterPool, and filterParallelThreshold are now instance fields. handleBLockFilter, handleLogsFilter, and processLogFilterEntry are now instance methods.

Motivation: with static shared state, any test that constructs a TronJsonRpcImpl instance would silently read and write the same filter maps used by the running node, making test isolation impossible. Switching to instance state means each instance owns its own filter maps; tests create a lightweight instance with no Spring dependencies (new TronJsonRpcImpl(null, null, null)) and operate on an isolated map.

As a consequence, BlockFilterCapsule and LogsFilterCapsule now receive the TronJsonRpcImpl instance at construction time and call handleBLockFilter / handleLogsFilter on it directly, eliminating the static imports that were the only way to invoke those methods before.

5. Behavor changes when node.jsonrpc.maxBlockFilterNum = 0

It forbid all new block filters when maxBlockFilterNum = 0 in release-4.8.1, now, it is not limited.

It has the same meaning as node.jsonrpc.maxLogFilterNum =0 for log filter number.

---

Testing

• Unit tests (new / updated):
  • HandleLogsFilterTest — 10 cases covering event dispatch, block-range filtering, expired-filter removal, solidified/non-solidified routing, and 3 parallel-path cases (all-filters receive events, expired eviction, solidified routing — each with filterParallelThreshold lowered via reflection to keep the test fast)
  • LogMatchOverLimitTest — 4 cases covering under-limit, exact-limit, exceed-limit (verifies exception is thrown before addAll), and empty-block skip
  • WalletCursorTest.testNewFilter_exceedsCapThrowsException — verifies the cap throws the correct exception with the limit value in the message; fixed to populate the map on the same instance that calls newFilter (the original test was populating the static map then constructing a new instance, so it never actually tested the cap)
  • BlockFilterCapsuleTest, LogsFilterCapsuleTest, ConcurrentHashMapTest — updated to pass a TronJsonRpcImpl instance to constructors after the static-to-instance refactoring
• Manual testing

Closes #6510

[bladehan1]

[SHOULD] Avoid @lazy circular dependency from Manager into TronJsonRpcImpl

Manager is the core db / block-processing component of java-tron. Its dependency graph is supposed to be inbound-only (other components depend on Manager). This PR reverses that by injecting TronJsonRpcImpl (an API-layer @component) into Manager, forming a cycle that can only be resolved with @Lazy. The cycle exists because the new BlockFilterCapsule / LogsFilterCapsule carry a TronJsonRpcImpl reference, so Manager has to obtain that reference before constructing each capsule.

Why this matters even though it compiles:

• @Lazy is a band-aid for circular dependencies, not a fix. It defers bean resolution to first method call and hides container init-order problems.
• Spring 6 / Spring Boot 3 default spring.main.allow-circular-references=false. Any Spring upgrade can fail on this exact cycle.
• Once Manager accepts a reverse dependency on the API layer, future capsule types follow the same path — the breach is contagious.
• postBlockFilter is on a hot path; if TronJsonRpcImpl is mid-initialization (e.g. during a restart), the proxy can throw BeanCurrentlyInCreationException.

Two options to resolve before merge:

Option A — drop the reverse dependency (smallest change, matches the hotfix/fix_newFilter branch name):

• Revert capsules to plain POJOs (no TronJsonRpcImpl field, original ctor signature).
• Drop @Lazy @Autowired TronJsonRpcImpl from Manager.
• Keep the three real fixes — maxLogFilterNum cap, LogMatch.matchBlockOneByOne MAX_RESULT check, and forEach + ForkJoinPool performance — none of them require static→instance.
• For test isolation, expose a static clearAllFilters() helper (or use @Before / @After to clear the four maps).
• Pros: minimal blast radius for a hotfix; production cache stays cross-instance; no @Lazy.
• Cons: test isolation depends on test discipline; multi-instance scenarios still share state (not an issue today since the bean is a singleton).

Option C — Visitor / Dispatcher (cleanest, may warrant splitting from this PR):

• Keep capsules as POJOs (no service reference).
• Dispatch in Manager.filterProcessLoop directly, e.g.:

  FilterTriggerCapsule c = filterCapsuleQueue.poll(...);
  if (c instanceof LogsFilterCapsule)  tronJsonRpcImpl.handleLogsFilter((LogsFilterCapsule) c);
  else if (c instanceof BlockFilterCapsule) tronJsonRpcImpl.handleBLockFilter((BlockFilterCapsule) c);
  

  Or — preferred — introduce @Component FilterTriggerDispatcher that holds TronJsonRpcImpl (forward dep). Manager @Autowireds the dispatcher (also forward), so all edges Manager → Dispatcher → JsonRpcImpl are forward. Cycle gone.
• Pros: capsules become genuinely independent, testable, serializable; Manager has no API-layer awareness; future capsule types just extend the dispatcher.
• Cons: slightly more code than the current PR (one extra bean or one instanceof block).

Suggestion: pick A or C. If A, this PR is lighter than today; if C, the architecture is durable and the @Lazy ban is enforced naturally.

[317787106]

Very good suggestion. To keep durable, @lazy is removed and use instanceof.

[317787106]

Summary

It don't works if remove @lazy so i add it back. FullNode.java:50 disables Spring circular dependency resolution:

beanFactory.setAllowCircularReferences(false);

After removing @Lazy, Spring detects the following dependency cycle during startup and rejects it directly:

Manager → TronJsonRpcImpl (required by constructor ↓)
               NodeInfoService → @Autowired Manager  ← cycle!
               Wallet          → @Autowired Manager  ← cycle!

Why FilterTriggerDispatcher Alone Does Not Break the Cycle ?

The Proposed Dependency Graph

 Manager  ──field──▶  FilterTriggerDispatcher  ──field──▶  TronJsonRpcImpl                                                                                                                                                        

At first glance, all three edges point in one direction — no cycle.

The Hidden Transitive Dependencies

TronJsonRpcImpl uses constructor injection for two dependencies:

@Autowired
public TronJsonRpcImpl(NodeInfoService nodeInfoService, Wallet wallet) { … }
                                                                                                                                                                                                                                 
Both of those beans carry a hard dependency back to Manager:
                                                                                                                                                                                                                                 
// NodeInfoService.java    
@Autowired private Manager dbManager;

// Wallet.java
@Autowired private Manager dbManager;

Expanding the full graph:

Manager                                                                                                                                                                                                                          
  └─field──▶ FilterTriggerDispatcher
               └─field──▶ TronJsonRpcImpl
                            ├─ctor──▶ NodeInfoService ──field──▶ Manager  ✗
                            └─ctor──▶ Wallet          ──field──▶ Manager  ✗                                                                                                                                                      

Why Spring Rejects This

FullNode.java explicitly disables circular-reference resolution:

beanFactory.setAllowCircularReferences(false);   // FullNode.java:50

With this flag, Spring does not expose an early singleton reference for beans that are
still being created. The creation sequence becomes:

1. Spring begins creating Manager → added to "currently in creation" set.
2. Manager needs FilterTriggerDispatcher → Spring creates it.
3. FilterTriggerDispatcher needs TronJsonRpcImpl → Spring creates it.
4. TronJsonRpcImpl's constructor requires NodeInfoService → Spring creates it.
5. NodeInfoService needs Manager → already in "currently in creation", no early reference available.
6. Spring attempts to create Manager again → BeanCurrentlyInCreationException.

The new class does not appear anywhere in the cycle; the cycle runs entirely through
TronJsonRpcImpl → NodeInfoService/Wallet → Manager.

What Would Actually Fix It

The FilterTriggerDispatcher design only eliminates the cycle if one of the following is
also true:

  │                                         Additional Change                                          │                                          Effect                                          │
  ├────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────────────────┤
  │ FilterTriggerDispatcher injects TronJsonRpcImpl with @Lazy                                         │ Defers creation past Manager's init; equivalent to the current @Lazy fix, just relocated │
  ├────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────────────────┤
  │ TronJsonRpcImpl switches NodeInfoService and Wallet to setter injection                            │ Constructor no longer forces eager creation of Manager-dependent beans                   │                                
  ├────────────────────────────────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────────────────┤
  │ FilterTriggerDispatcher looks up TronJsonRpcImpl via ApplicationContext.getBean() at dispatch time │ Bypasses Spring DI for this edge entirely                                                │                                
  └────────────────────────────────────────────────────────────────────────────────────────────────────┴──────────────────────────────────────────────────────────────────────────────────────────┘                                

Without one of these additions, introducing FilterTriggerDispatcher changes only the
name of the path through which the cycle travels, not the cycle itself.

What to do next ?

If a more systematic architectural refactor is planned in the future, FilterTriggerDispatcher is worth tracking in a separate issue and addressing together with changing NodeInfoService / Wallet to setter injection.

[bladehan1]

Thanks for the detailed analysis — you are right that introducing FilterTriggerDispatcher alone does not break the cycle, because the cycle actually runs through TronJsonRpcImpl → NodeInfoService / Wallet → Manager regardless of whether a Dispatcher sits in between. My original suggestion was incomplete on that point.

But I think the conversation we're having so far is still framed at the wrong level. The three workarounds you listed (@Lazy on the Dispatcher edge, switching NodeInfoService / Wallet to setter injection, ApplicationContext.getBean() lookup) are all techniques to make Spring tolerate the cycle. None of them remove the underlying design problem, which is:

Manager should not depend on TronJsonRpcImpl at all.

The reason this dependency exists today is that Manager is hosting both the producer side (postBlockFilter / postLogsFilter → queue.offer(...)) and the consumer side (filterProcessLoop → calls into TronJsonRpcImpl) of the same queue. The consumer side is where the reverse edge comes from. Move it out and the entire cycle disappears — no @Lazy, no setter-injection rewrite, no ApplicationContext.getBean().

Concrete plan: move filterProcessLoop into TronJsonRpcImpl

Manager keeps only the producer role:

@Component
public class Manager {
    private final BlockingQueue<FilterTriggerCapsule> filterCapsuleQueue =
        new LinkedBlockingQueue<>();

    public BlockingQueue<FilterTriggerCapsule> getFilterCapsuleQueue() {
        return filterCapsuleQueue;
    }

    private void postBlockFilter(...) { filterCapsuleQueue.offer(new BlockFilterCapsule(blk, sol)); }
    private void postLogsFilter(...)  { filterCapsuleQueue.offer(new LogsFilterCapsule(...));    }

    // delete: @Lazy @Autowired private TronJsonRpcImpl tronJsonRpcImpl;
    // delete: filterProcessLoop Runnable
    // delete: filterEs / filterEsName / isRunFilterProcessThread
}

TronJsonRpcImpl owns the consumer role:

@Component
public class TronJsonRpcImpl implements TronJsonRpc, Closeable {
    private final Manager manager;  // back to ctor injection (forward edge)
    private volatile boolean running = true;
    private ExecutorService filterEs;

    @Autowired
    public TronJsonRpcImpl(NodeInfoService nodeInfoService, Wallet wallet, Manager manager) { ... }

    @PostConstruct
    public void start() {
        if (!CommonParameter.getInstance().isJsonRpcFilterEnabled()) return;
        filterEs = ExecutorServiceManager.newSingleThreadExecutor("filter-process");
        ExecutorServiceManager.submit(filterEs, this::filterProcessLoop);
    }

    private void filterProcessLoop() {
        BlockingQueue<FilterTriggerCapsule> queue = manager.getFilterCapsuleQueue();
        while (running) {
            try {
                FilterTriggerCapsule c = queue.poll(1, TimeUnit.SECONDS);
                if (c instanceof LogsFilterCapsule)        handleLogsFilter((LogsFilterCapsule) c);
                else if (c instanceof BlockFilterCapsule)  handleBLockFilter((BlockFilterCapsule) c);
            } catch (InterruptedException e) { Thread.currentThread().interrupt(); }
              catch (Throwable t) { logger.error("filterProcessLoop error", t); }
        }
    }

    @Override public void close() {
        running = false;
        ExecutorServiceManager.shutdownAndAwaitTermination(filterEs, "filter-process");
        // existing logsFilterPool / sectionExecutor shutdown
    }
}

Resulting Spring graph (all edges forward, no cycle):

TronJsonRpcImpl ──► Manager
TronJsonRpcImpl ──► NodeInfoService ──► Manager
TronJsonRpcImpl ──► Wallet          ──► Manager
Manager ──► (nothing in jsonrpc)

Why this cycle showed up now, and why it is worth fixing properly

Before this PR, BlockFilterCapsule / LogsFilterCapsule called TronJsonRpcImpl.handleBLockFilter / handleLogsFilter via import static. Those are Java static dispatches, not Spring bean references — Spring's container had no idea Manager depended on TronJsonRpcImpl. The cycle existed in the call graph the whole time; Spring just couldn't see it.

This PR turning those methods into instance methods (which is the right thing to do for test isolation of the filter maps) forces the dependency to be expressed as @Autowired, and only then does Spring complain. So the cycle isn't introduced here — it's exposed here. Adding @Lazy papers over an existing layering issue that the previous static-import pattern was hiding.

If we ship @Lazy as the final state, the next time someone does the same cleanup on another piece of jsonrpc state (a counter, a metrics object, another cache), they'll add another @Lazy field for the same reason, and the layering breach compounds.

Suggestion

For this PR: consider Plan X above. The diff is small (~30 lines net), the cycle is genuinely removed (no @Lazy), and the fix matches the existing branch name (hotfix/fix_newFilter).

If the PR is already too large in scope to add this: leave the current @Lazy and open a follow-up issue framed as "Move filterProcessLoop into TronJsonRpcImpl so Manager no longer depends on jsonrpc" rather than "switch NodeInfoService / Wallet to setter injection". The latter only quiets Spring; the former actually fixes the layering.

[317787106]

Very good suggestion. The PR is enough large now, maybe need to refactor TronJsonRpcImpl ( including remove @Lazy) in next ISSUE or PR. The main goal of removing modifier static from four ConcurrentHashMap is to solve 10 testcase failure that happens in parallel test introduced by PR t.maxParallelForks https://github.com/tronprotocol/java-tron/pull/6447/changes, so TronJsonRpcImpl needs to be optimized in current PR.

[bladehan1]

[SHOULD] Highlight maxBlockFilterNum=0 as a behavior reversal in release notes

The new check is maxBlockFilterNum > 0 && size >= maxBlockFilterNum. Previously it was size >= maxBlockFilterNum, so maxBlockFilterNum = 0 rejected every eth_newBlockFilter call (effectively a kill switch). After this change, 0 means unlimited. Operators who set 0 to disable the endpoint will silently flip from "forbid all" to "allow all" on upgrade.

The PR description and reference.conf already mention 0 = unlimited, but neither flags this as a backward-incompatible change.

Suggestion: Add an explicit "behavior change" note in the PR body / release notes ("was: forbid all, now: unlimited"), and consider grep'ing known deployment configs for maxBlockFilterNum = 0 before release.

[317787106]

Thanks for your correction. To keep the meaning of several numeric parameters under JSON-RPC consistent, a value of 0 is uniformly treated as having no size limit. The PR description has been revised with emphasis.

[bladehan1]

[NIT] Cap is best-effort under concurrent newFilter

The size() >= cap check and the subsequent put are not atomic, so concurrent eth_newFilter calls can land between the check and the insert. In the worst case the map can briefly hold cap + concurrent_requests entries. This matches the pre-existing maxBlockFilterNum pattern, so it is not a regression, but the security framing of the cap (DoS hardening) implies hard limits.

Suggestion: Document the cap as approximate in reference.conf ("may be exceeded by concurrent registrations"). A stricter implementation would track size with an AtomicInteger and CAS-increment before put.

[317787106]

Yes, the maxLogFilterNum is not matched exactly, it's an approximate value. Generally, its effect is little, overly detailed configuration files may increase operational overhead; comments can be added in the code instead.

[bladehan1]

[NIT] Restrict @Setter visibility on the manager field to package-private

The refactor moved Manager manager from constructor injection to field injection with @Setter @Autowired private Manager manager;. Lombok's default @Setter is public, so production callers can now invoke tronJsonRpc.setManager(other) and silently swap the core db reference. The setter only exists for tests (which need tronJsonRpc.setManager(dbManager) after the constructor stops accepting Manager).

Suggestion: @Setter(AccessLevel.PACKAGE) or @VisibleForTesting (Guava) so production code cannot mutate the wired Manager. Tests in the same package still get access.

[317787106]

Add following code instead:

  @VisibleForTesting
  public void setManager(Manager manager) {
    this.manager = manager;
  }

Not all the testcase invoking it are belong to package org.tron.core.services.jsonrpc, so cannot restrict @Setter visibility on the manager field to package-private

[bladehan1]

[NIT] Add isDaemon overload to newForkJoinPool to match sibling factories

All the other factories in this class come in pairs — newSingleThreadExecutor(name) / newSingleThreadExecutor(name, isDaemon), same for newSingleThreadScheduledExecutor and newFixedThreadPool. The new newForkJoinPool(String, int) only has the single-arg form, with no way to opt into daemon threads.

Suggestion: Add newForkJoinPool(String name, int parallelism, boolean isDaemon) that calls thread.setDaemon(isDaemon) inside the worker factory; have the two-arg form delegate with isDaemon=false (matching newFixedThreadPool's default).

[317787106]

Add following in ExecutorServiceManager:

public static ForkJoinPool newForkJoinPool(String name, int parallelism, boolean isDaemon) {...}

isDaemon is false default.

[bladehan1]

[NIT] Restore Args singleton in @after to avoid cross-test pollution

Args.getInstance().setJsonRpcMaxLogFilterNum(5) mutates a JVM-wide singleton. The test does not restore the original value on exit, so any later test (in this class or — when surefire does not fork — in any other test class) that reads jsonRpcMaxLogFilterNum will see 5 instead of the configured default (20000). This is the exact pattern the project's knowledge/test-singleton-pollution.md warns against.

Suggestion: Save the original cap before the mutation and restore it in finally (or in an @After method): int original = Args.getInstance().getJsonRpcMaxLogFilterNum(); ... finally { Args.getInstance().setJsonRpcMaxLogFilterNum(original); }.

[317787106]

Restored it. Really, Args.clearParams() will restore it automatically, there is no need to restore it manually.

[bladehan1]

[NIT] Restrict @Setter visibility on filterParallelThreshold to package-private

Replacing reflection with @Setter is a clear improvement, but Lombok's default @Setter is public, so production callers can change the threshold at runtime. The setter is only needed by tests (which already live in the same package).

Suggestion: @Setter(AccessLevel.PACKAGE) or annotate with @VisibleForTesting.

[317787106]

Remove @setter, add following code instead:

  @VisibleForTesting
  public void setFilterParallelThreshold(int filterParallelThreshold) {
    this.filterParallelThreshold = filterParallelThreshold;
  }