Skip to content

fix(db): fix reference#78

Open
317787106 wants to merge 25 commits into
developfrom
hotfix/fix_reference
Open

fix(db): fix reference#78
317787106 wants to merge 25 commits into
developfrom
hotfix/fix_reference

Conversation

@317787106
Copy link
Copy Markdown
Owner

@317787106 317787106 commented Jun 1, 2026

What does this PR do?

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details


Summary by cubic

Strengthens config validation and network protections, cleans up unused settings, and improves API rate limiting and shielded TRC20 burn handling. Adds CI checks for reference.conf, fixes storage property overrides, and improves Solidity node shutdown and solidity height retrieval.

  • New Features

    • CI: validate reference.conf key names and hierarchy depth using pyhocon.
    • Rate limiting: added rate.limiter.apiNonBlocking to choose blocking vs non-blocking permits across HTTP and gRPC.
    • Event plugins: require Plugin-Version >= 3.0.0 at startup.
    • Shielded TRC20: introduce V2 burn ciphertext bound to the nullifier with strict length checks and a new NoteSpent log topic.
    • Configuration: refreshed reference.conf and config.conf, removed unused storage index and JSON parsing options, and added clear docs.
  • Bug Fixes

    • Storage: correct per-database properties parsing and applying of overrides (only user-set fields override defaults).
    • Security and P2P: re-verify block witness signatures during fork replay; enforce strict signature length on RPC, P2P txs, and Hello; strip unknown protobuf fields from inbound blocks/txs; map merkle/sign failures to BAD_BLOCK.
    • VM: canonicalize ModExp zero-modulus output behind the Osaka switch; reset return buffer for CREATE2 when Osaka is enabled.
    • Consensus: write TIP-2935 parent hash after witness permission check; ensure permission-change effects force re-verification of later txs.
    • JSON-RPC: apply parser limits via a dedicated JsonFactory and improve spec compliance.
    • Solidity node: handle interrupts and shutdown in getBlock/getBlockByNum, and add checks in getLastSolidityBlockNum, to exit cleanly and reduce noisy logs.
    • Tests: reset DPoS task state in a shielded flow test to prevent flakiness.

Written for commit 05baeb8. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • CI-backed reference.conf validator added
    • Rate-limiter supports API non-blocking mode
    • Minimum event-plugin version enforcement (>= 3.0.0)
    • Shielded TRC20: v2 burn-message/encryption support
  • Bug Fixes

    • Adjusted ModExp / CREATE2 returndata behavior under Osaka
    • Stronger signature-length validation with earlier rejection
    • Sanitization added to strip unknown protobuf fields
  • Configuration

    • Removed legacy overlay/index settings; updated event and rate-limiter defaults
  • Documentation

    • New configuration guides and expanded reference.conf docs
  • Tests

    • Expanded unit/integration coverage across features and validators

317787106 and others added 20 commits May 10, 2026 16:09
* fix(config): remove redundant JSON-RPC config fields and consolidate parameter binding

Remove maxBlockFilterNum, maxAddressSize, and maxRequestTimeout from NodeConfig/CommonParameter
since they were superseded by the jsonRpcMaxBatchSize/jsonRpcMaxResponseSize parameters.
Consolidate the config binding path so all JSON-RPC size limits flow through a single
reference.conf entry, and clean up stale test-config fixtures.

* fix bug of NodeConfigTest

* remove allowShieldedTransactionApi from reference.conf

* add testcase of external.ip is null

* change comment

* fix the bug of --es and 7 failed testcases
…onprotocol#6760)

Pre-3.0.0(The previous event-plugin public release is 2.2.0)
event-plugin builds still link against com.alibaba.fastjson, which
was removed from java-tron in tronprotocol#6701. When such a plugin is loaded, the
NoClassDefFoundError surfaces inside the plugin's own worker thread and is
swallowed by its catch(Throwable) handler. The node keeps running while
silently dropping every trigger, leaving operators with no signal that the
event subscription is broken.

Enforce a minimum Plugin-Version at startup in EventPluginLoader.startPlugin
using pf4j's VersionManager (semver). When the descriptor version is below
3.0.0, log a clear upgrade hint and return false; the existing call chain in
Manager.startEventSubscribing wraps that into TronError(EVENT_SUBSCRIBE_INIT)
and aborts node startup instead of silently degrading.
…rs (tronprotocol#6761)

- Add `rate.limiter.apiNonBlocking` switch (default false). When off, callers wait for a permit; when on, they reject immediately and shed load.
- Wire the switch through `RateLimiterConfig` → `Args` → `CommonParameter`; update `reference.conf` / `config.conf`.
- Add blocking `acquire()` paths on `IRateLimiter`, `GlobalRateLimiter`, and `QpsStrategy` / `IPQpsStrategy` / `GlobalPreemptibleStrategy`. Only the semaphore-based strategy is bounded (2s); the QPS-based paths use unbounded Guava `RateLimiter.acquire()`.
- Introduce `acquirePermit()` dispatcher (default method on `IRateLimiter`; static on `GlobalRateLimiter`) that picks blocking vs non-blocking based on the switch.
- Swap `tryAcquire` → `acquirePermit` at the `RateLimiterServlet` and `RateLimiterInterceptor` call sites; preserve per-endpoint-before-global ordering to avoid consuming global quota on per-endpoint rejection.
- Extract `loadIpLimiter()` in `GlobalRateLimiter` and `loadLimiter()` in `IPQpsStrategy` to share cache+exception handling between `tryAcquire` and `acquire`.
- Add unit tests covering dispatcher routing (both directions), acquire IP-before-global ordering, and IP-loader-failure fail-closed behaviour.
…onprotocol#6791)

- P2pEventHandlerImpl.processException's switch did not include the
  BLOCK_MERKLE_ERROR type introduced by PR tronprotocol#6716. The new type fell through
  to the default branch, so peers whose blocks failed merkle root
  validation were disconnected with ReasonCode.UNKNOWN instead of
  BAD_BLOCK, missing the bad-peer ban window in
  PeerConnection.processDisconnect.
- Add the case alongside BAD_BLOCK and BLOCK_SIGN_INVALID so the
  disconnect reason is reported as BAD_BLOCK end-to-end.
- Rename the two enum constants for naming consistency:
  BLOCK_SIGN_ERROR    -> BLOCK_SIGN_INVALID,
  BLOCK_MERKLE_ERROR  -> BLOCK_MERKLE_INVALID,
  and reword their descriptions to 'sign failed' / 'merkle failed' to
  match the imperative style used by the other nearby enum entries.
  All call sites updated.
- Add two regression tests in P2pEventHandlerImplTest:
  one pins BLOCK_MERKLE_INVALID -> BAD_BLOCK (the actual fix);
  the other pins BLOCK_SIGN_INVALID -> BAD_BLOCK so a future switch
  refactor cannot silently drop either back to UNKNOWN.
…ocol#6777)

- Manager.switchFork now re-validates each replayed block's witness signature
  before applying. The witness account's permission can change between forks
  (via permission-update transactions), so a signature that was valid on the
  original chain may no longer be valid on the replay path.
- Use `if (!validateSignature(...)) throw new ValidateSignatureException`
  rather than discarding the boolean return: validateSignature only throws
  on malformed signature bytes; an attacker-supplied valid-format signature
  with a wrong-signer address returns false. Discarding the return would let
  that attack through.
- The existing switchFork catch list already includes
  ValidateSignatureException, so the new throw is wired into the existing
  switchback path with no additional handling.
- Add three BlockCapsule.validateSignature contract tests pinning the two
  failure modes the fix relies on: signer-mismatch returns false, signer-match
  returns true, and a 65-byte malformed signature throws
  ValidateSignatureException.
…ronprotocol#6796)

Initialize multiAddresses from ownerAddressSet so that getVerifyTxs
still forces re-verification of an owner's later txs even after the
owner's permission-change tx has been packed and is no longer present
in pendingTransactions.

Main changes:
- Manager.getVerifyTxs: seed multiAddresses with ownerAddressSet to
  cover the case where a permission-change tx has been consumed but
  ownerAddressSet still retains the owner (kept alive by in-flight
  txs in pushTransactionQueue / rePushTransactions via
  filterOwnerAddress).
- ManagerTest: add getVerifyTxsSkipsBlockWhenPermissionTxAlreadyConsumed
  reproducing the bypass — pending contains only B (transfer, old sig,
  isVerified=true), ownerAddressSet contains the owner, block contains
  only B without the permission-change tx. Assertion checks B is placed
  in the re-verify list instead of being short-circuited via setVerified.
…col#6780)

* fix(vm): canonicalize ModExp zero modulus output (TIP-871)

* func(vm): remove account name for history block hash contract

* test(db): align BlockHashHistory account name assertion
* feat(net): normalize inbound messages

* perf(net): skip wire-byte rewrite when sanitize is a no-op
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a CI reference.conf validator; refactors configuration beans and reference configs; moves JSON parse limits into constants; gates VM MODEXP/CREATE2 by Osaka flag; sanitizes protobuf unknown fields; tightens P2P signature checks; implements blocking/non-blocking rate-limiter API; introduces shielded TRC‑20 burn v2; updates tests and test resources.

Changes

End-to-end platform updates across config, VM, networking, rate limiting, and shielded TRC‑20

Layer / File(s) Summary
All changes (single checkpoint)
.(all updated files listed in PR)
Comprehensive change set: adds Python validator and CI step; edits reference.conf/config.conf and docs; restructures config beans and Args bridging; applies VM/JSON/runtime behavior changes; sanitizes protobufs; tightens P2P signature validation; introduces blocking-aware rate-limiter API and config flag; migrates shielded TRC‑20 burn to v2 with nonce/version and updates Wallet; large-scale test and test-resource updates.

Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes

"I hop the config tree at night,
keys in order, ports held tight.
Osaka whispers change the byte,
nonce and marker guard the light.
Tests hum softly — all is right."

✨ 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 hotfix/fix_reference

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

🧹 Nitpick comments (1)
framework/src/main/java/org/tron/core/services/ratelimiter/GlobalRateLimiter.java (1)

34-45: Blocking acquire() has no upper bound on wait time.

In blocking mode, r.acquire() and rateLimiter.acquire() (Guava RateLimiter) block the servlet request thread indefinitely until a permit is granted. Under sustained load with a low per-IP/global QPS, request threads accumulate in acquire(), which can exhaust the Tomcat worker pool and degrade the whole HTTP service rather than fast-failing. Since this path also holds a preemptible per-endpoint permit while blocking, the pressure compounds.

Consider documenting/operationally bounding blocking mode (e.g., a tryAcquire-with-timeout variant) so a slow global limiter can't stall the request pool.

🤖 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
`@framework/src/main/java/org/tron/core/services/ratelimiter/GlobalRateLimiter.java`
around lines 34 - 45, The blocking calls in acquire(RuntimeData) (notably
r.acquire() from loadIpLimiter(ip) and rateLimiter.acquire()) can block
indefinitely; change them to bounded waits using RateLimiter.tryAcquire with a
configurable timeout (e.g., tryAcquire(timeout, TimeUnit.MILLISECONDS)) and
fail-fast when the permit isn't obtained by returning false or logging and
aborting the request; update the method to use r.tryAcquire(...) for the per-IP
limiter and rateLimiter.tryAcquire(...) for the global limiter, ensure the
timeout value is configurable (system property or config field) and
document/handle the timeout path consistently (release any held state and return
false).
🤖 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.

Inline comments:
In @.github/scripts/check_reference_conf.py:
- Around line 165-175: The loop silently swallows errors from
tree.get(full_path) and int(raw); update the except Exception and the except
(TypeError, ValueError) handlers to log the exception and context (e.g.,
full_path and raw) before continuing, referencing tree.get, raw, value,
PORT_SENTINELS and by_value so you can locate the block — if these failures are
truly expected for unresolved substitutions, instead add a concise comment
explaining why silent suppression is acceptable and still log at debug level;
ensure logging uses the module logger rather than print.

In @.github/workflows/pr-check.yml:
- Around line 106-121: The workflow step uses the mutable tag
actions/setup-python@v5 which should be pinned to an immutable commit SHA to
prevent supply-chain risks; update the step that references
actions/setup-python@v5 to use the corresponding commit SHA for the v5 release
(replace the tag with the full commit SHA), and audit other uses of third-party
actions in this workflow (e.g., any subsequent steps that reference actions/*@*)
to pin them to known-good SHAs as well, verifying the chosen SHA matches the
intended v5 release before committing.

In `@actuator/src/main/java/org/tron/core/vm/program/Program.java`:
- Around line 1619-1621: The reset of returnDataBuffer is currently conditional
on VMConfig.allowTvmOsaka() which leaves stale data after pre-Osaka CREATE2;
change the logic so returnDataBuffer is cleared unconditionally for every
CREATE2 execution path in Program (remove the VMConfig.allowTvmOsaka() check
around the returnDataBuffer = null and ensure the same unconditional clear
occurs in all CREATE2 handling code paths so RETURNDATASIZE/RETURNDATACOPY
cannot observe previous callee data).

In `@chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java`:
- Around line 332-349: The sanitize() method currently only clears unknown
fields on the block and block header wrappers but not on block_header.raw_data,
which is used by getBlockId() and signature verification; update sanitize() in
BlockCapsule to also detect unknown fields in
block.getBlockHeader().getRawData() and clear them by building a new RawData via
rawData.toBuilder().setUnknownFields(UnknownFieldSet.getDefaultInstance()).build(),
then set that rawData into the block header builder (alongside clearing header
unknown fields) before rebuilding the block; ensure the rawData check
contributes to the boolean that decides whether to return true.

In `@chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java`:
- Around line 498-506: The sanitize method in TransactionCapsule currently only
clears unknown fields on the outer Transaction, but getTransactionId() hashes
transaction.getRawData(), so you must also clear unknown fields inside raw_data;
update TransactionCapsule.sanitize to build a new raw_data by calling
transaction.getRawData().toBuilder().setUnknownFields(UnknownFieldSet.getDefaultInstance()).build()
and set that cleaned raw_data on the Transaction builder before rebuilding the
transaction (retain the existing outer unknown-field clearing logic), so both
the outer Transaction and its raw_data have unknown fields removed.

In `@common/src/main/java/org/tron/core/config/args/CommitteeConfig.java`:
- Around line 82-90: normalizeNonStandardKeys currently unconditionally copies
legacy keys (allowPBFT, pBFTExpireNum) over canonical keys using
section.withValue, which overwrites canonical values; change the logic to only
rename/copy a legacy key if the canonical key (allowPbft or pbftExpireNum) is
NOT already present in the Config, i.e. check section.hasPath("allowPbft") and
section.hasPath("pbftExpireNum") before calling section.withValue, so
canonical-key precedence is preserved; update the function
normalizeNonStandardKeys and references to allowPBFT, allowPbft, pBFTExpireNum,
pbftExpireNum and withValue accordingly.

In `@common/src/main/java/org/tron/core/config/args/NodeConfig.java`:
- Around line 494-503: normalizeNonStandardKeys currently calls
section.getIsNull("discovery.external.ip") and section.getString(...) without
ensuring the path exists, which throws when the path is missing; change the
logic in normalizeNonStandardKeys to first check
section.hasPathOrNull("discovery.external.ip") (or section.hasPath(...)) and
only then call getIsNull/getString, and if the path exists and is null or equal
to "null" replace it with ConfigValueFactory.fromAnyRef("") (keep the existing
behavior for "isOpenFullTcpDisconnect" handling and the same path string
"discovery.external.ip").

In `@docs/configuration.md`:
- Around line 17-22: The fenced code blocks in docs/configuration.md (the
"startup resolution order" block and the reference path snippet at lines
255-257) are unlabeled and trigger markdownlint MD040; update those
triple-backtick fences to include an explicit language tag such as text (e.g.,
change ``` to ```text) for both the startup-order block and the reference path
snippet so the blocks are properly labeled and linting passes.

In `@docs/implement-a-customized-actuator-zh.md`:
- Around line 258-260: The inline comment inside the test method sumActuatorTest
is still in English; replace the English comment "// this key is defined in
config-test.conf as accountName=Sun" with a Chinese translation (e.g., "此键在
config-test.conf 中定义为 accountName=Sun") so the example remains fully localized
in the docs/implement-a-customized-actuator-zh.md sample and keeps the comment
next to the sumActuatorTest() method consistent with the rest of the Chinese
guide.

In `@framework/src/main/java/org/tron/core/config/args/Args.java`:
- Around line 355-358: The code in applyEventConfig currently preserves prior
state by OR-ing into PARAMETER.eventSubscribe, which prevents later setParam()
calls from disabling events and lets stale eventPluginConfig/eventFilter leak;
change applyEventConfig to compute PARAMETER.eventSubscribe solely from the
current inputs (e.g., set PARAMETER.eventSubscribe = ec.isEnable() or derive
only from the current CLI/config flags instead of OR-ing) and ensure that when
the effective value is false you explicitly clear/reset eventPluginConfig and
eventFilter so no previous event state persists across setParam() calls (adjust
callers of applyEventConfig/setParam if they rely on prior state).

In `@framework/src/main/java/org/tron/core/Wallet.java`:
- Around line 4127-4142: The note index is being incremented for burn-token logs
(logType == 4) even though only log types 1..3 produce note leaves; update the
Wallet logic so index is not advanced for burn-token entries: in the block where
getShieldedTRC20LogType(log, ...) is checked and index += 1 is executed, move or
guard the increment so it only runs for logType values that emit notes (1..3),
e.g., increment index after confirming noteTx was created for logType 1..3 (and
do not increment when logType == 4); ensure the pendingNf handling (pendingNf =
null) for logType == 4 remains unchanged.

In
`@framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java`:
- Around line 310-326: The builder currently reuses a mutable field ovk across
builds leading to stale keys; in ShieldedTRC20ParametersBuilder (the build path
handling burn encryption), stop mutating or reading a cached builder-level ovk
and instead derive a local ovk variable from spend.expsk (e.g. call
spend.expsk.getOvk()) for each build invocation before calling
Encryption.encryptBurnMessageByOvk; remove or avoid assigning to the builder's
ovk field so each burn encryption uses the freshly derived ovk and throw the
same ZksnarkException if that local ovk is null.

In `@framework/src/test/java/org/tron/core/db/ManagerTest.java`:
- Around line 894-918: The test mutates dbManager.getPendingTransactions() but
never restores it; capture the original pending list before modifying (e.g.
List<TransactionCapsule> backupPending = new
ArrayList<>(dbManager.getPendingTransactions())), then in the finally block
restore it (clear and addAll(backupPending)) alongside the existing
ownerAddressSet restore so the shared pending queue state isn't leaked to other
tests; update the ManagerTest method that manipulates ownerAddressSet and
pending transactions to use these backup/restore steps.

In
`@framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java`:
- Around line 37-43: The test installs a process-wide mock via
Message.setDynamicPropertiesStore(...) in setUp and never restores it,
potentially leaking into other tests; capture the previous store at the start of
setUp (e.g., save Message.getDynamicPropertiesStore() into a static field) and
add an `@AfterClass` tearDown that calls
Message.setDynamicPropertiesStore(previousStore) (or null if original was null)
to restore global state so the mock does not persist beyond
SanitizeUnknownFieldsTest.

---

Nitpick comments:
In
`@framework/src/main/java/org/tron/core/services/ratelimiter/GlobalRateLimiter.java`:
- Around line 34-45: The blocking calls in acquire(RuntimeData) (notably
r.acquire() from loadIpLimiter(ip) and rateLimiter.acquire()) can block
indefinitely; change them to bounded waits using RateLimiter.tryAcquire with a
configurable timeout (e.g., tryAcquire(timeout, TimeUnit.MILLISECONDS)) and
fail-fast when the permit isn't obtained by returning false or logging and
aborting the request; update the method to use r.tryAcquire(...) for the per-IP
limiter and rateLimiter.tryAcquire(...) for the global limiter, ensure the
timeout value is configurable (system property or config field) and
document/handle the timeout path consistently (release any held state and return
false).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1f129172-5748-48a8-a3d3-1cff01e3a571

📥 Commits

Reviewing files that changed from the base of the PR and between 381d369 and f6ccd65.

📒 Files selected for processing (122)
  • .github/scripts/check_reference_conf.py
  • .github/scripts/requirements.txt
  • .github/workflows/pr-check.yml
  • README.md
  • actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java
  • actuator/src/main/java/org/tron/core/vm/program/Program.java
  • chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java
  • chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java
  • codecov.yml
  • common/src/main/java/org/tron/common/parameter/CommonParameter.java
  • common/src/main/java/org/tron/core/Constant.java
  • common/src/main/java/org/tron/core/config/args/CommitteeConfig.java
  • common/src/main/java/org/tron/core/config/args/EventConfig.java
  • common/src/main/java/org/tron/core/config/args/NodeConfig.java
  • common/src/main/java/org/tron/core/config/args/Overlay.java
  • common/src/main/java/org/tron/core/config/args/RateLimiterConfig.java
  • common/src/main/java/org/tron/core/config/args/Storage.java
  • common/src/main/java/org/tron/core/config/args/StorageConfig.java
  • common/src/main/java/org/tron/core/config/args/VmConfig.java
  • common/src/main/java/org/tron/core/exception/P2pException.java
  • common/src/main/java/org/tron/json/JSON.java
  • common/src/main/resources/reference.conf
  • common/src/test/java/org/tron/core/config/args/CommitteeConfigTest.java
  • common/src/test/java/org/tron/core/config/args/EventConfigTest.java
  • common/src/test/java/org/tron/core/config/args/NodeConfigTest.java
  • common/src/test/java/org/tron/core/config/args/RateLimiterConfigTest.java
  • common/src/test/java/org/tron/core/config/args/StorageConfigTest.java
  • common/src/test/java/org/tron/core/config/args/VmConfigTest.java
  • crypto/src/main/java/org/tron/common/crypto/SignUtils.java
  • docs/configuration-conventions.md
  • docs/configuration.md
  • docs/implement-a-customized-actuator-en.md
  • docs/implement-a-customized-actuator-zh.md
  • framework/src/main/java/org/tron/common/logsfilter/EventPluginLoader.java
  • framework/src/main/java/org/tron/core/Wallet.java
  • framework/src/main/java/org/tron/core/config/args/Args.java
  • framework/src/main/java/org/tron/core/db/HistoryBlockHashUtil.java
  • framework/src/main/java/org/tron/core/db/Manager.java
  • framework/src/main/java/org/tron/core/net/P2pEventHandlerImpl.java
  • framework/src/main/java/org/tron/core/net/TronNetDelegate.java
  • framework/src/main/java/org/tron/core/net/message/adv/BlockMessage.java
  • framework/src/main/java/org/tron/core/net/messagehandler/BlockMsgHandler.java
  • framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java
  • framework/src/main/java/org/tron/core/net/peer/PeerConnection.java
  • framework/src/main/java/org/tron/core/net/service/relay/RelayService.java
  • framework/src/main/java/org/tron/core/net/service/sync/SyncService.java
  • framework/src/main/java/org/tron/core/services/http/RateLimiterServlet.java
  • framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/GlobalRateLimiter.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/RateLimiterInterceptor.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/adapter/DefaultBaseQqsAdapter.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/adapter/GlobalPreemptibleAdapter.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/adapter/IPQPSRateLimiterAdapter.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/adapter/IRateLimiter.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/adapter/QpsRateLimiterAdapter.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/strategy/GlobalPreemptibleStrategy.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/strategy/IPQpsStrategy.java
  • framework/src/main/java/org/tron/core/services/ratelimiter/strategy/QpsStrategy.java
  • framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java
  • framework/src/main/java/org/tron/core/zen/note/NoteEncryption.java
  • framework/src/main/java/org/tron/program/FullNode.java
  • framework/src/main/java/org/tron/program/SolidityNode.java
  • framework/src/main/resources/config.conf
  • framework/src/test/java/org/tron/common/ParameterTest.java
  • framework/src/test/java/org/tron/common/TestConstants.java
  • framework/src/test/java/org/tron/common/logsfilter/EventLoaderTest.java
  • framework/src/test/java/org/tron/common/runtime/vm/AllowTvmOsakaTest.java
  • framework/src/test/java/org/tron/common/runtime/vm/BandWidthRuntimeOutOfTimeTest.java
  • framework/src/test/java/org/tron/common/runtime/vm/BandWidthRuntimeOutOfTimeWithCheckTest.java
  • framework/src/test/java/org/tron/common/runtime/vm/BandWidthRuntimeTest.java
  • framework/src/test/java/org/tron/common/runtime/vm/BandWidthRuntimeWithCheckTest.java
  • framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsVerifyProofTest.java
  • framework/src/test/java/org/tron/common/runtime/vm/TvmIssueVerifierTest.java
  • framework/src/test/java/org/tron/common/utils/client/utils/Sha256Sm3Hash.java
  • framework/src/test/java/org/tron/core/ShieldedTRC20BuilderTest.java
  • framework/src/test/java/org/tron/core/WalletMockTest.java
  • framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java
  • framework/src/test/java/org/tron/core/config/args/ArgsTest.java
  • framework/src/test/java/org/tron/core/config/args/LocalWitnessTest.java
  • framework/src/test/java/org/tron/core/config/args/OverlayTest.java
  • framework/src/test/java/org/tron/core/config/args/StorageTest.java
  • framework/src/test/java/org/tron/core/db/AccountIndexStoreTest.java
  • framework/src/test/java/org/tron/core/db/AccountStoreTest.java
  • framework/src/test/java/org/tron/core/db/HistoryBlockHashIntegrationTest.java
  • framework/src/test/java/org/tron/core/db/ManagerMockTest.java
  • framework/src/test/java/org/tron/core/db/ManagerTest.java
  • framework/src/test/java/org/tron/core/db/TransactionHistoryTest.java
  • framework/src/test/java/org/tron/core/db/TransactionRetStoreTest.java
  • framework/src/test/java/org/tron/core/db/TransactionStoreTest.java
  • framework/src/test/java/org/tron/core/db/TransactionTraceTest.java
  • framework/src/test/java/org/tron/core/db/TxCacheDBTest.java
  • framework/src/test/java/org/tron/core/db/api/AssetUpdateHelperTest.java
  • framework/src/test/java/org/tron/core/jsonrpc/ApiUtilTest.java
  • framework/src/test/java/org/tron/core/metrics/MetricsApiServiceTest.java
  • framework/src/test/java/org/tron/core/net/P2pEventHandlerImplTest.java
  • framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java
  • framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java
  • framework/src/test/java/org/tron/core/net/messagehandler/ChainInventoryMsgHandlerTest.java
  • framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java
  • framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java
  • framework/src/test/java/org/tron/core/services/http/RateLimiterServletTest.java
  • framework/src/test/java/org/tron/core/services/jsonrpc/JsonRpcServletTest.java
  • framework/src/test/java/org/tron/core/services/ratelimiter/GlobalRateLimiterTest.java
  • framework/src/test/java/org/tron/core/services/ratelimiter/RateLimiterInterceptorTest.java
  • framework/src/test/java/org/tron/core/services/ratelimiter/adaptor/AdaptorTest.java
  • framework/src/test/java/org/tron/core/zen/note/BurnCipherTest.java
  • framework/src/test/java/org/tron/core/zksnark/LibrustzcashTest.java
  • framework/src/test/java/org/tron/core/zksnark/MerkleTreeTest.java
  • framework/src/test/java/org/tron/core/zksnark/NoteEncDecryTest.java
  • framework/src/test/java/org/tron/core/zksnark/SendCoinShieldTest.java
  • framework/src/test/java/org/tron/core/zksnark/ShieldedReceiveTest.java
  • framework/src/test/java/org/tron/json/JsonTest.java
  • framework/src/test/java/org/tron/program/SolidityNodeTest.java
  • framework/src/test/resources/args-test.conf
  • framework/src/test/resources/config-shield.conf
  • framework/src/test/resources/config-test-index.conf
  • framework/src/test/resources/config-test-mainnet.conf
  • framework/src/test/resources/config-test-storagetest.conf
  • framework/src/test/resources/config-test.conf
  • plugins/src/test/java/org/tron/plugins/DbLiteTest.java
  • plugins/src/test/resources/config-duplicate.conf
  • protocol/src/main/protos/api/api.proto
💤 Files with no reviewable changes (17)
  • codecov.yml
  • plugins/src/test/resources/config-duplicate.conf
  • framework/src/test/resources/config-test-index.conf
  • common/src/main/java/org/tron/core/config/args/Overlay.java
  • framework/src/test/resources/config-test-mainnet.conf
  • framework/src/test/resources/config-test-storagetest.conf
  • framework/src/test/java/org/tron/core/metrics/MetricsApiServiceTest.java
  • framework/src/test/resources/args-test.conf
  • common/src/test/java/org/tron/core/config/args/StorageConfigTest.java
  • framework/src/test/java/org/tron/core/config/args/OverlayTest.java
  • framework/src/test/java/org/tron/core/db/TransactionStoreTest.java
  • protocol/src/main/protos/api/api.proto
  • common/src/test/java/org/tron/core/config/args/NodeConfigTest.java
  • framework/src/test/java/org/tron/common/ParameterTest.java
  • common/src/main/java/org/tron/core/config/args/Storage.java
  • framework/src/test/java/org/tron/common/utils/client/utils/Sha256Sm3Hash.java
  • framework/src/test/resources/config-shield.conf

Comment on lines +165 to +175
try:
raw = tree.get(full_path)
except Exception:
continue
try:
value = int(raw)
except (TypeError, ValueError):
continue
if value in PORT_SENTINELS:
continue
by_value.setdefault(value, []).append(full_path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Log exceptions before suppressing them.

The bare except Exception: continue on lines 167-168 silently suppresses all errors when reading port values from the config tree. If tree.get(full_path) fails due to a type mismatch, missing substitution, or corrupted structure, or if int(raw) coercion fails unexpectedly, the validator silently skips that port without any indication that something went wrong. This could mask real configuration issues.

📝 Proposed fix to add logging before continue
         try:
             raw = tree.get(full_path)
         except Exception:
+            # Skip entries that cannot be read (e.g., unresolved substitutions)
             continue
         try:
             value = int(raw)
-        except (TypeError, ValueError):
+        except (TypeError, ValueError) as e:
+            # Skip non-integer port values (e.g., unresolved ${PORT} substitutions)
             continue

Alternatively, if these exceptions are genuinely expected for unresolved substitutions, add a comment explaining why silent suppression is safe.

As per static analysis (Ruff S112), consider logging the exception before continuing to aid debugging.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try:
raw = tree.get(full_path)
except Exception:
continue
try:
value = int(raw)
except (TypeError, ValueError):
continue
if value in PORT_SENTINELS:
continue
by_value.setdefault(value, []).append(full_path)
try:
raw = tree.get(full_path)
except Exception:
# Skip entries that cannot be read (e.g., unresolved substitutions)
continue
try:
value = int(raw)
except (TypeError, ValueError) as e:
# Skip non-integer port values (e.g., unresolved ${PORT} substitutions)
continue
if value in PORT_SENTINELS:
continue
by_value.setdefault(value, []).append(full_path)
🧰 Tools
🪛 Ruff (0.15.14)

[error] 167-168: try-except-continue detected, consider logging the exception

(S112)


[warning] 167-167: Do not catch blind exception: Exception

(BLE001)

🤖 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 @.github/scripts/check_reference_conf.py around lines 165 - 175, The loop
silently swallows errors from tree.get(full_path) and int(raw); update the
except Exception and the except (TypeError, ValueError) handlers to log the
exception and context (e.g., full_path and raw) before continuing, referencing
tree.get, raw, value, PORT_SENTINELS and by_value so you can locate the block —
if these failures are truly expected for unresolved substitutions, instead add a
concise comment explaining why silent suppression is acceptable and still log at
debug level; ensure logging uses the module logger rather than print.

Comment on lines +106 to +121
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.11'
cache: 'pip'
cache-dependency-path: .github/scripts/requirements.txt

- name: Install pyhocon
run: pip install --quiet -r .github/scripts/requirements.txt

- name: Validate reference.conf key names and depth
shell: bash
run: |
python3 .github/scripts/check_reference_conf.py \
common/src/main/resources/reference.conf

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pin GitHub Actions to commit SHAs for security.

The actions/setup-python@v5 action is referenced by a mutable tag rather than an immutable commit SHA. This exposes the workflow to supply-chain attacks if the action's v5 tag is moved to point at malicious code. Since this step runs with repository access and installs dependencies, a compromised action could exfiltrate secrets or inject malicious code into the validation process.

🔒 Recommended fix to pin to commit SHA

Pin actions/setup-python@v5 to its current v5 commit SHA:

-      - name: Set up Python
-        uses: actions/setup-python@v5
+      - name: Set up Python (pinned to v5.5.0)
+        uses: actions/setup-python@0b93645e9e50ab7c8f56c3d3b7e6e202ce966f53 # v5.5.0
         with:
           python-version: '3.11'
           cache: 'pip'
           cache-dependency-path: .github/scripts/requirements.txt

Verify the SHA corresponds to the v5.5.0 release you intend to use. Periodically review and update pinned SHAs as part of dependency maintenance.

As per static analysis (zizmor unpinned-uses), consider pinning all GitHub Actions to commit SHAs in security-sensitive workflows.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.11'
cache: 'pip'
cache-dependency-path: .github/scripts/requirements.txt
- name: Install pyhocon
run: pip install --quiet -r .github/scripts/requirements.txt
- name: Validate reference.conf key names and depth
shell: bash
run: |
python3 .github/scripts/check_reference_conf.py \
common/src/main/resources/reference.conf
- name: Set up Python (pinned to v5.5.0)
uses: actions/setup-python@0b93645e9e50ab7c8f56c3d3b7e6e202ce966f53 # v5.5.0
with:
python-version: '3.11'
cache: 'pip'
cache-dependency-path: .github/scripts/requirements.txt
- name: Install pyhocon
run: pip install --quiet -r .github/scripts/requirements.txt
- name: Validate reference.conf key names and depth
shell: bash
run: |
python3 .github/scripts/check_reference_conf.py \
common/src/main/resources/reference.conf
🧰 Tools
🪛 zizmor (1.25.2)

[error] 107-107: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 107-107: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): this step

(cache-poisoning)

🤖 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 @.github/workflows/pr-check.yml around lines 106 - 121, The workflow step
uses the mutable tag actions/setup-python@v5 which should be pinned to an
immutable commit SHA to prevent supply-chain risks; update the step that
references actions/setup-python@v5 to use the corresponding commit SHA for the
v5 release (replace the tag with the full commit SHA), and audit other uses of
third-party actions in this workflow (e.g., any subsequent steps that reference
actions/*@*) to pin them to known-good SHAs as well, verifying the chosen SHA
matches the intended v5 release before committing.

Comment on lines +1619 to +1621
if (VMConfig.allowTvmOsaka()) {
returnDataBuffer = null; // reset return buffer right before the call
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear returnDataBuffer for every CREATE2 path.

Conditioning this reset on allowTvmOsaka() leaves stale return data observable after pre-Osaka CREATE2 executions, while the other call/create entry points in this class always clear it first. That can leak the previous callee's buffer into subsequent RETURNDATASIZE / RETURNDATACOPY reads.

Suggested fix
   public void createContract2(DataWord value, DataWord memStart, DataWord memSize, DataWord salt) {
-    if (VMConfig.allowTvmOsaka()) {
-      returnDataBuffer = null; // reset return buffer right before the call
-    }
+    returnDataBuffer = null; // reset return buffer right before the call
 
     byte[] senderAddress;
🤖 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 `@actuator/src/main/java/org/tron/core/vm/program/Program.java` around lines
1619 - 1621, The reset of returnDataBuffer is currently conditional on
VMConfig.allowTvmOsaka() which leaves stale data after pre-Osaka CREATE2; change
the logic so returnDataBuffer is cleared unconditionally for every CREATE2
execution path in Program (remove the VMConfig.allowTvmOsaka() check around the
returnDataBuffer = null and ensure the same unconditional clear occurs in all
CREATE2 handling code paths so RETURNDATASIZE/RETURNDATACOPY cannot observe
previous callee data).

Comment on lines +332 to +349
public boolean sanitize() {
boolean blockHasUnknown = !this.block.getUnknownFields().asMap().isEmpty();
boolean headerHasUnknown = !this.block.getBlockHeader().getUnknownFields().asMap().isEmpty();
if (!blockHasUnknown && !headerHasUnknown) {
return false;
}
UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance();
Block.Builder builder = this.block.toBuilder();
if (blockHasUnknown) {
builder.setUnknownFields(empty);
}
if (headerHasUnknown) {
builder.setBlockHeader(this.block.getBlockHeader().toBuilder()
.setUnknownFields(empty)
.build());
}
this.block = builder.build();
return true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Sanitize block_header.raw_data too, not just the wrappers.

getBlockId() and signature verification hash block.getBlockHeader().getRawData().toByteArray(). With the current implementation, unknown fields inside block_header.raw_data survive unchanged, so the canonical bytes that consensus logic actually uses are still unsanitized.

Suggested fix
   public boolean sanitize() {
     boolean blockHasUnknown = !this.block.getUnknownFields().asMap().isEmpty();
     boolean headerHasUnknown = !this.block.getBlockHeader().getUnknownFields().asMap().isEmpty();
-    if (!blockHasUnknown && !headerHasUnknown) {
+    boolean rawHasUnknown = !this.block.getBlockHeader().getRawData().getUnknownFields().asMap().isEmpty();
+    if (!blockHasUnknown && !headerHasUnknown && !rawHasUnknown) {
       return false;
     }
     UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance();
     Block.Builder builder = this.block.toBuilder();
     if (blockHasUnknown) {
@@
     if (headerHasUnknown) {
       builder.setBlockHeader(this.block.getBlockHeader().toBuilder()
           .setUnknownFields(empty)
           .build());
     }
+    if (rawHasUnknown) {
+      builder.setBlockHeader(builder.getBlockHeaderBuilder()
+          .setRawData(this.block.getBlockHeader().getRawData().toBuilder()
+              .setUnknownFields(empty)
+              .build())
+          .build());
+    }
     this.block = builder.build();
     return true;
   }
🤖 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 `@chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java` around lines
332 - 349, The sanitize() method currently only clears unknown fields on the
block and block header wrappers but not on block_header.raw_data, which is used
by getBlockId() and signature verification; update sanitize() in BlockCapsule to
also detect unknown fields in block.getBlockHeader().getRawData() and clear them
by building a new RawData via
rawData.toBuilder().setUnknownFields(UnknownFieldSet.getDefaultInstance()).build(),
then set that rawData into the block header builder (alongside clearing header
unknown fields) before rebuilding the block; ensure the rawData check
contributes to the boolean that decides whether to return true.

Comment on lines +498 to +506
public boolean sanitize() {
if (this.transaction.getUnknownFields().asMap().isEmpty()) {
return false;
}
this.transaction = this.transaction.toBuilder()
.setUnknownFields(UnknownFieldSet.getDefaultInstance())
.build();
return true;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Sanitize raw_data as well, otherwise tx canonicalization is incomplete.

getTransactionId() hashes transaction.getRawData().toByteArray(), but this method only strips unknown fields from the outer Transaction. Unknown fields inside raw_data still survive and continue to affect the bytes used for tx ids and signature checks.

Suggested fix
   public boolean sanitize() {
-    if (this.transaction.getUnknownFields().asMap().isEmpty()) {
+    boolean txHasUnknown = !this.transaction.getUnknownFields().asMap().isEmpty();
+    boolean rawHasUnknown = !this.transaction.getRawData().getUnknownFields().asMap().isEmpty();
+    if (!txHasUnknown && !rawHasUnknown) {
       return false;
     }
-    this.transaction = this.transaction.toBuilder()
-        .setUnknownFields(UnknownFieldSet.getDefaultInstance())
-        .build();
+    UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance();
+    Transaction.Builder builder = this.transaction.toBuilder();
+    if (txHasUnknown) {
+      builder.setUnknownFields(empty);
+    }
+    if (rawHasUnknown) {
+      builder.setRawData(this.transaction.getRawData().toBuilder()
+          .setUnknownFields(empty)
+          .build());
+    }
+    this.transaction = builder.build();
     return true;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public boolean sanitize() {
if (this.transaction.getUnknownFields().asMap().isEmpty()) {
return false;
}
this.transaction = this.transaction.toBuilder()
.setUnknownFields(UnknownFieldSet.getDefaultInstance())
.build();
return true;
}
public boolean sanitize() {
boolean txHasUnknown = !this.transaction.getUnknownFields().asMap().isEmpty();
boolean rawHasUnknown = !this.transaction.getRawData().getUnknownFields().asMap().isEmpty();
if (!txHasUnknown && !rawHasUnknown) {
return false;
}
UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance();
Transaction.Builder builder = this.transaction.toBuilder();
if (txHasUnknown) {
builder.setUnknownFields(empty);
}
if (rawHasUnknown) {
builder.setRawData(this.transaction.getRawData().toBuilder()
.setUnknownFields(empty)
.build());
}
this.transaction = builder.build();
return true;
}
🤖 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 `@chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java` around
lines 498 - 506, The sanitize method in TransactionCapsule currently only clears
unknown fields on the outer Transaction, but getTransactionId() hashes
transaction.getRawData(), so you must also clear unknown fields inside raw_data;
update TransactionCapsule.sanitize to build a new raw_data by calling
transaction.getRawData().toBuilder().setUnknownFields(UnknownFieldSet.getDefaultInstance()).build()
and set that cleaned raw_data on the Transaction builder before rebuilding the
transaction (retain the existing outer unknown-field clearing logic), so both
the outer Transaction and its raw_data have unknown fields removed.

Comment on lines 355 to +358
private static void applyEventConfig(EventConfig ec) {
PARAMETER.eventSubscribe = ec.isEnable();
// contractParse belongs to event.subscribe but Storage object holds it
PARAMETER.storage.setContractParseSwitch(ec.isContractParse());

// PARAMETER.eventPluginConfig and PARAMETER.eventFilter are only consumed by
// Manager.startEventSubscribing(), which itself is gated by isEventSubscribe()
// (= ec.isEnable()) at Manager.java:564. When subscribe is disabled, building
// these objects has no observable effect — skip both early so PARAMETER stays
// consistent with the runtime intent.
if (!ec.isEnable()) {
// cmd parameter has higher priority
PARAMETER.eventSubscribe = PARAMETER.eventSubscribe || ec.isEnable();
if (!PARAMETER.eventSubscribe) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't carry eventSubscribe across setParam() calls.

Line 357 derives the new value from the previous singleton state. Once an earlier init enables events, a later Args.setParam() call cannot cleanly disable them, and stale eventPluginConfig / eventFilter can leak into later tests or re-init flows. Compute the effective value from the current CLI/config inputs only, or clear the event state before applying it.

🤖 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 `@framework/src/main/java/org/tron/core/config/args/Args.java` around lines 355
- 358, The code in applyEventConfig currently preserves prior state by OR-ing
into PARAMETER.eventSubscribe, which prevents later setParam() calls from
disabling events and lets stale eventPluginConfig/eventFilter leak; change
applyEventConfig to compute PARAMETER.eventSubscribe solely from the current
inputs (e.g., set PARAMETER.eventSubscribe = ec.isEnable() or derive only from
the current CLI/config flags instead of OR-ing) and ensure that when the
effective value is false you explicitly clear/reset eventPluginConfig and
eventFilter so no previous event state persists across setParam() calls (adjust
callers of applyEventConfig/setParam if they rely on prior state).

Comment on lines 4127 to +4142
int logType = getShieldedTRC20LogType(log, shieldedTRC20ContractAddress);
if (logType > 0) {
if (logType == 5) {
byte[] logData = log.getData().toByteArray();
if (logData.length >= 32) {
pendingNf = ByteArray.subArray(logData, 0, 32);
}
} else if (logType > 0) {
noteBuilder = DecryptNotesTRC20.NoteTx.newBuilder();
noteBuilder.setTxid(ByteString.copyFrom(txid));
noteBuilder.setIndex(index);
index += 1;
noteTx = getNoteTxFromLogListByOvk(noteBuilder, log, ovk, logType);
noteTx = getNoteTxFromLogListByOvk(noteBuilder, log, ovk, logType, pendingNf);
noteTx.ifPresent(builder::addNoteTxs);
if (logType == 4) {
pendingNf = null;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not advance the note index for burn-token logs.

logType == 4 is still going through the index += 1 path here, even though only log types 1..3 emit note leaves. That makes OVK scans drift after the first burn and returns wrong indices for later notes in the same transaction stream.

Suggested fix
             for (TransactionInfo.Log log : logList) {
               int logType = getShieldedTRC20LogType(log, shieldedTRC20ContractAddress);
               if (logType == 5) {
                 byte[] logData = log.getData().toByteArray();
                 if (logData.length >= 32) {
                   pendingNf = ByteArray.subArray(logData, 0, 32);
                 }
               } else if (logType > 0) {
                 noteBuilder = DecryptNotesTRC20.NoteTx.newBuilder();
                 noteBuilder.setTxid(ByteString.copyFrom(txid));
                 noteBuilder.setIndex(index);
-                index += 1;
+                if (logType < 4) {
+                  index += 1;
+                }
                 noteTx = getNoteTxFromLogListByOvk(noteBuilder, log, ovk, logType, pendingNf);
                 noteTx.ifPresent(builder::addNoteTxs);
                 if (logType == 4) {
                   pendingNf = null;
                 }
🤖 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 `@framework/src/main/java/org/tron/core/Wallet.java` around lines 4127 - 4142,
The note index is being incremented for burn-token logs (logType == 4) even
though only log types 1..3 produce note leaves; update the Wallet logic so index
is not advanced for burn-token entries: in the block where
getShieldedTRC20LogType(log, ...) is checked and index += 1 is executed, move or
guard the increment so it only runs for logType values that emit notes (1..3),
e.g., increment index after confirming noteTx was created for logType 1..3 (and
do not increment when logType == 4); ensure the pendingNf handling (pendingNf =
null) for logType == 4 remains unchanged.

Comment on lines +310 to +326
if (ovk == null && spend.expsk != null) {
ovk = spend.expsk.getOvk();
}
if (ovk == null) {
throw new ZksnarkException("missing ovk for burn encryption");
}
byte[] nf = spendDescription.getNullifier().toByteArray();
byte[] transparentToAddressTvm = normalizeTransparentToAddress(transparentToAddress);
byte[] addr21 = new byte[21];
addr21[0] = Wallet.getAddressPreFixByte();
System.arraycopy(transparentToAddressTvm, 0, addr21, 1, 20);
Optional<byte[]> cipherOpt = Encryption.encryptBurnMessageByOvk(
ovk, transparentToAmount, addr21, nf);
if (!cipherOpt.isPresent()) {
throw new ZksnarkException("encrypt burn message failed");
}
burnCiphertext = cipherOpt.get();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Derive the burn ovk per build instead of caching it on the builder.

ovk is mutable builder state. After one burn build it stays populated, so a later build with a different spend.expsk skips recomputing it and encrypts the new burn record with the stale key.

♻️ Proposed fix
-          if (ovk == null && spend.expsk != null) {
-            ovk = spend.expsk.getOvk();
-          }
-          if (ovk == null) {
+          byte[] ovkForBurn = ovk;
+          if (ovkForBurn == null && spend.expsk != null) {
+            ovkForBurn = spend.expsk.getOvk();
+          }
+          if (ovkForBurn == null) {
             throw new ZksnarkException("missing ovk for burn encryption");
           }
           byte[] nf = spendDescription.getNullifier().toByteArray();
           byte[] transparentToAddressTvm = normalizeTransparentToAddress(transparentToAddress);
           byte[] addr21 = new byte[21];
           addr21[0] = Wallet.getAddressPreFixByte();
           System.arraycopy(transparentToAddressTvm, 0, addr21, 1, 20);
           Optional<byte[]> cipherOpt = Encryption.encryptBurnMessageByOvk(
-              ovk, transparentToAmount, addr21, nf);
+              ovkForBurn, transparentToAmount, addr21, nf);
🤖 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
`@framework/src/main/java/org/tron/core/zen/ShieldedTRC20ParametersBuilder.java`
around lines 310 - 326, The builder currently reuses a mutable field ovk across
builds leading to stale keys; in ShieldedTRC20ParametersBuilder (the build path
handling burn encryption), stop mutating or reading a cached builder-level ovk
and instead derive a local ovk variable from spend.expsk (e.g. call
spend.expsk.getOvk()) for each build invocation before calling
Encryption.encryptBurnMessageByOvk; remove or avoid assigning to the builder's
ovk field so each burn encryption uses the freshly derived ovk and throw the
same ZksnarkException if that local ovk is null.

Comment on lines +894 to +918
dbManager.getPendingTransactions().clear();
dbManager.getPendingTransactions().add(bTx);

Field field = Manager.class.getDeclaredField("ownerAddressSet");
field.setAccessible(true);
@SuppressWarnings("unchecked")
Set<String> ownerAddressSet = (Set<String>) field.get(dbManager);
Set<String> backup = new HashSet<>(ownerAddressSet);
ownerAddressSet.clear();
ownerAddressSet.add(hexOwner);

try {
List<Transaction> blockTxs = new ArrayList<>();
blockTxs.add(bTx.getInstance());
BlockCapsule capsule = new BlockCapsule(0, ByteString.EMPTY, 0, blockTxs);

List<TransactionCapsule> txs = dbManager.getVerifyTxs(capsule);

Assert.assertEquals(1, txs.size());
Assert.assertEquals(bTx.getTransactionId(), txs.get(0).getTransactionId());
} finally {
ownerAddressSet.clear();
ownerAddressSet.addAll(backup);
dbManager.getPendingTransactions().clear();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Restore pendingTransactions in finally as well.

This test snapshots and restores ownerAddressSet, but it leaves the shared pending queue empty. That leaks state into later tests and makes failures depend on execution order.

🧪 Suggested fix
+    List<TransactionCapsule> pendingBackup = new ArrayList<>(dbManager.getPendingTransactions());
     dbManager.getPendingTransactions().clear();
     dbManager.getPendingTransactions().add(bTx);
@@
     } finally {
       ownerAddressSet.clear();
       ownerAddressSet.addAll(backup);
       dbManager.getPendingTransactions().clear();
+      dbManager.getPendingTransactions().addAll(pendingBackup);
     }
🤖 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 `@framework/src/test/java/org/tron/core/db/ManagerTest.java` around lines 894 -
918, The test mutates dbManager.getPendingTransactions() but never restores it;
capture the original pending list before modifying (e.g.
List<TransactionCapsule> backupPending = new
ArrayList<>(dbManager.getPendingTransactions())), then in the finally block
restore it (clear and addAll(backupPending)) alongside the existing
ownerAddressSet restore so the shared pending queue state isn't leaked to other
tests; update the ManagerTest method that manipulates ownerAddressSet and
pending transactions to use these backup/restore steps.

Comment on lines +37 to +43
@BeforeClass
public static void setUp() {
// BlockMessage(byte[]) calls Message.isFilter() which dereferences the
// static DynamicPropertiesStore. The mock's primitive-long getter returns
// 0L by default, so isFilter() returns false.
Message.setDynamicPropertiesStore(Mockito.mock(DynamicPropertiesStore.class));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reset the static Message store after this class.

Message.setDynamicPropertiesStore(...) changes process-wide state. Leaving the mock installed can leak into unrelated networking tests and make them order-dependent.

🤖 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
`@framework/src/test/java/org/tron/core/net/message/adv/SanitizeUnknownFieldsTest.java`
around lines 37 - 43, The test installs a process-wide mock via
Message.setDynamicPropertiesStore(...) in setUp and never restores it,
potentially leaking into other tests; capture the previous store at the start of
setUp (e.g., save Message.getDynamicPropertiesStore() into a static field) and
add an `@AfterClass` tearDown that calls
Message.setDynamicPropertiesStore(previousStore) (or null if original was null)
to restore global state so the mock does not persist beyond
SanitizeUnknownFieldsTest.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 issues found across 122 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java">

<violation number="1" location="chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java:499">
P2: sanitize() only removes top-level unknown fields and can miss unknown fields in nested transaction messages (e.g., raw_data).</violation>
</file>

<file name="actuator/src/main/java/org/tron/core/vm/program/Program.java">

<violation number="1" location="actuator/src/main/java/org/tron/core/vm/program/Program.java:1619">
P2: `CREATE2` can run with a stale `returnDataBuffer` because the reset is incorrectly gated by `allowTvmOsaka()`. This can leak previous call return data into subsequent RETURNDATA operations.</violation>
</file>

<file name="framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java">

<violation number="1" location="framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java:195">
P1: Batch size limiting is bypassed for invalid sub-requests: the code marks overflow but still appends `INVALID_REQUEST` entries, so responses can exceed `jsonRpcMaxResponseSize`.</violation>
</file>

<file name="framework/src/main/java/org/tron/common/logsfilter/EventPluginLoader.java">

<violation number="1" location="framework/src/main/java/org/tron/common/logsfilter/EventPluginLoader.java:473">
P2: Unsupported plugins are returned early without unloading after `loadPlugin`, leaving rejected plugins in the manager state.</violation>
</file>

<file name="framework/src/main/java/org/tron/core/db/Manager.java">

<violation number="1" location="framework/src/main/java/org/tron/core/db/Manager.java:1238">
P2: Copying `ownerAddressSet` here introduces a concurrent-iteration risk on a non-thread-safe `HashSet`; this can throw `ConcurrentModificationException` during block verification.</violation>
</file>

<file name="common/src/main/java/org/tron/core/config/args/CommitteeConfig.java">

<violation number="1" location="common/src/main/java/org/tron/core/config/args/CommitteeConfig.java:83">
P2: Do not overwrite `allowPbft` when it is already present; otherwise mixed configs cannot override legacy aliases with the canonical key.</violation>
</file>

<file name="framework/src/main/java/org/tron/core/Wallet.java">

<violation number="1" location="framework/src/main/java/org/tron/core/Wallet.java:4133">
P2: Restrict this branch to note-producing log types (1..3); including type 4 here shifts subsequent note indices.</violation>
</file>

Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.

Re-trigger cubic

ObjectNode errNode = buildErrorNode(JsonRpcError.INVALID_REQUEST, "Invalid Request", null);
byte[] errBytes = MAPPER.writeValueAsBytes(errNode);
int addition = errBytes.length + (!batchResult.isEmpty() ? 1 : 0);
if (maxResponseSize > 0 && accumulatedSize + addition > maxResponseSize) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Batch size limiting is bypassed for invalid sub-requests: the code marks overflow but still appends INVALID_REQUEST entries, so responses can exceed jsonRpcMaxResponseSize.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java, line 195:

<comment>Batch size limiting is bypassed for invalid sub-requests: the code marks overflow but still appends `INVALID_REQUEST` entries, so responses can exceed `jsonRpcMaxResponseSize`.</comment>

<file context>
@@ -159,15 +177,30 @@ private void handleBatch(HttpServletResponse resp, JsonNode rootNode, int maxRes
+        ObjectNode errNode = buildErrorNode(JsonRpcError.INVALID_REQUEST, "Invalid Request", null);
+        byte[] errBytes = MAPPER.writeValueAsBytes(errNode);
+        int addition = errBytes.length + (!batchResult.isEmpty() ? 1 : 0);
+        if (maxResponseSize > 0 && accumulatedSize + addition > maxResponseSize) {
+          overflow = true;
+        } else {
</file context>

Comment on lines +499 to +505
if (this.transaction.getUnknownFields().asMap().isEmpty()) {
return false;
}
this.transaction = this.transaction.toBuilder()
.setUnknownFields(UnknownFieldSet.getDefaultInstance())
.build();
return true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: sanitize() only removes top-level unknown fields and can miss unknown fields in nested transaction messages (e.g., raw_data).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java, line 499:

<comment>sanitize() only removes top-level unknown fields and can miss unknown fields in nested transaction messages (e.g., raw_data).</comment>

<file context>
@@ -494,6 +495,16 @@ public static boolean validateSignature(Transaction transaction,
   }
 
+  public boolean sanitize() {
+    if (this.transaction.getUnknownFields().asMap().isEmpty()) {
+      return false;
+    }
</file context>
Suggested change
if (this.transaction.getUnknownFields().asMap().isEmpty()) {
return false;
}
this.transaction = this.transaction.toBuilder()
.setUnknownFields(UnknownFieldSet.getDefaultInstance())
.build();
return true;
boolean txHasUnknown = !this.transaction.getUnknownFields().asMap().isEmpty();
boolean rawHasUnknown = !this.transaction.getRawData().getUnknownFields().asMap().isEmpty();
if (!txHasUnknown && !rawHasUnknown) {
return false;
}
UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance();
Transaction.Builder builder = this.transaction.toBuilder();
if (txHasUnknown) {
builder.setUnknownFields(empty);
}
if (rawHasUnknown) {
builder.setRawData(this.transaction.getRawData().toBuilder()
.setUnknownFields(empty)
.build());
}
this.transaction = builder.build();
return true;

Comment on lines +1619 to +1621
if (VMConfig.allowTvmOsaka()) {
returnDataBuffer = null; // reset return buffer right before the call
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: CREATE2 can run with a stale returnDataBuffer because the reset is incorrectly gated by allowTvmOsaka(). This can leak previous call return data into subsequent RETURNDATA operations.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At actuator/src/main/java/org/tron/core/vm/program/Program.java, line 1619:

<comment>`CREATE2` can run with a stale `returnDataBuffer` because the reset is incorrectly gated by `allowTvmOsaka()`. This can leak previous call return data into subsequent RETURNDATA operations.</comment>

<file context>
@@ -1616,6 +1616,10 @@ public ProgramTrace getTrace() {
   }
 
   public void createContract2(DataWord value, DataWord memStart, DataWord memSize, DataWord salt) {
+    if (VMConfig.allowTvmOsaka()) {
+      returnDataBuffer = null; // reset return buffer right before the call
+    }
</file context>
Suggested change
if (VMConfig.allowTvmOsaka()) {
returnDataBuffer = null; // reset return buffer right before the call
}
returnDataBuffer = null; // reset return buffer right before the call

Comment on lines +473 to +475
if (!isPluginVersionSupported(pluginManager, pluginId)) {
return false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Unsupported plugins are returned early without unloading after loadPlugin, leaving rejected plugins in the manager state.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/common/logsfilter/EventPluginLoader.java, line 473:

<comment>Unsupported plugins are returned early without unloading after `loadPlugin`, leaving rejected plugins in the manager state.</comment>

<file context>
@@ -457,6 +470,10 @@ protected CompoundPluginDescriptorFinder createPluginDescriptorFinder() {
       return false;
     }
 
+    if (!isPluginVersionSupported(pluginManager, pluginId)) {
+      return false;
+    }
</file context>
Suggested change
if (!isPluginVersionSupported(pluginManager, pluginId)) {
return false;
}
if (!isPluginVersionSupported(pluginManager, pluginId)) {
pluginManager.unloadPlugin(pluginId);
return false;
}

List<TransactionCapsule> txs = new ArrayList<>();
Map<String, TransactionCapsule> txMap = new HashMap<>();
Set<String> multiAddresses = new HashSet<>();
Set<String> multiAddresses = new HashSet<>(ownerAddressSet);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Copying ownerAddressSet here introduces a concurrent-iteration risk on a non-thread-safe HashSet; this can throw ConcurrentModificationException during block verification.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/db/Manager.java, line 1238:

<comment>Copying `ownerAddressSet` here introduces a concurrent-iteration risk on a non-thread-safe `HashSet`; this can throw `ConcurrentModificationException` during block verification.</comment>

<file context>
@@ -1230,7 +1235,7 @@ public List<TransactionCapsule> getVerifyTxs(BlockCapsule block) {
     List<TransactionCapsule> txs = new ArrayList<>();
     Map<String, TransactionCapsule> txMap = new HashMap<>();
-    Set<String> multiAddresses = new HashSet<>();
+    Set<String> multiAddresses = new HashSet<>(ownerAddressSet);
 
     pendingTransactions.forEach(capsule -> {
</file context>

// Remap them to standard camelCase keys so ConfigBeanFactory binds them normally.
// Config is immutable; withValue() returns a new object.
private static Config normalizeNonStandardKeys(Config section) {
if (section.hasPath("allowPBFT")) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Do not overwrite allowPbft when it is already present; otherwise mixed configs cannot override legacy aliases with the canonical key.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At common/src/main/java/org/tron/core/config/args/CommitteeConfig.java, line 83:

<comment>Do not overwrite `allowPbft` when it is already present; otherwise mixed configs cannot override legacy aliases with the canonical key.</comment>

<file context>
@@ -85,32 +67,30 @@ public class CommitteeConfig {
+  // Remap them to standard camelCase keys so ConfigBeanFactory binds them normally.
+  // Config is immutable; withValue() returns a new object.
+  private static Config normalizeNonStandardKeys(Config section) {
+    if (section.hasPath("allowPBFT")) {
+      ConfigValue v = section.getValue("allowPBFT");
+      section = section.withValue("allowPbft", v); // rename allowPBFT -> allowPbft
</file context>
Suggested change
if (section.hasPath("allowPBFT")) {
if (!section.hasPath("allowPbft") && section.hasPath("allowPBFT")) {

if (logData.length >= 32) {
pendingNf = ByteArray.subArray(logData, 0, 32);
}
} else if (logType > 0) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Restrict this branch to note-producing log types (1..3); including type 4 here shifts subsequent note indices.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/Wallet.java, line 4133:

<comment>Restrict this branch to note-producing log types (1..3); including type 4 here shifts subsequent note indices.</comment>

<file context>
@@ -4091,15 +4122,24 @@ public DecryptNotesTRC20 scanShieldedTRC20NotesByOvk(long startNum, long endNum,
+                if (logData.length >= 32) {
+                  pendingNf = ByteArray.subArray(logData, 0, 32);
+                }
+              } else if (logType > 0) {
                 noteBuilder = DecryptNotesTRC20.NoteTx.newBuilder();
                 noteBuilder.setTxid(ByteString.copyFrom(txid));
</file context>
Suggested change
} else if (logType > 0) {
} else if (logType > 0 && logType < 4) {

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
common/src/main/java/org/tron/core/config/args/StorageConfig.java (1)

203-274: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject unsupported LevelDB keys instead of silently ignoring them.

This parser only consumes blockSize, writeBufferSize, cacheSize, and maxOpenFiles. Any extra keys under storage.default* or storage.properties[] are accepted by startup but dropped on the floor, so stale configs quietly run with defaults instead of the operator’s intended overrides.

Suggested direction
+  private static final java.util.Set<String> SUPPORTED_LEVELDB_KEYS =
+      java.util.Set.of("blockSize", "writeBufferSize", "cacheSize", "maxOpenFiles");
+
   private static void readLevelDbOptions(ConfigObject conf, DbOptionOverride o) {
+    for (String key : conf.keySet()) {
+      if (!SUPPORTED_LEVELDB_KEYS.contains(key)
+          && !"name".equals(key)
+          && !"path".equals(key)) {
+        throw new IllegalArgumentException(
+            String.format("Unsupported LevelDB option: %s", key));
+      }
+    }
     if (conf.containsKey("blockSize")) {
       String param = conf.get("blockSize").unwrapped().toString();
       try {
         o.setBlockSize(Integer.parseInt(param));
       } catch (NumberFormatException e) {
🤖 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 `@common/src/main/java/org/tron/core/config/args/StorageConfig.java` around
lines 203 - 274, The parser currently ignores unknown LevelDB option keys
silently; update readLevelDbOptions(ConfigObject conf, DbOptionOverride o) to
detect unexpected keys and throw an IllegalArgumentException listing them
(allowed set: "blockSize","writeBufferSize","cacheSize","maxOpenFiles") so bad
keys are rejected; likewise ensure callers readDbOption and readProperties
validate their ConfigObject inputs (in readProperties each item must only
contain "name","path" plus the allowed LevelDB keys) and reject entries with
extra keys (include the offending key names in the exception) to prevent
silently dropped configuration overrides.
🤖 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.

Inline comments:
In `@common/src/main/java/org/tron/core/config/README.md`:
- Line 43: Replace the misspelled identifier "accout" with "account" in the
storage example and any matching occurrences (specifically the paths
'/path/to/accout/database' and '/path/to/accout/index' and nearby descriptive
text); update the example text that references the `accout` database to read
`account` so the example can be copied verbatim without confusion.

---

Outside diff comments:
In `@common/src/main/java/org/tron/core/config/args/StorageConfig.java`:
- Around line 203-274: The parser currently ignores unknown LevelDB option keys
silently; update readLevelDbOptions(ConfigObject conf, DbOptionOverride o) to
detect unexpected keys and throw an IllegalArgumentException listing them
(allowed set: "blockSize","writeBufferSize","cacheSize","maxOpenFiles") so bad
keys are rejected; likewise ensure callers readDbOption and readProperties
validate their ConfigObject inputs (in readProperties each item must only
contain "name","path" plus the allowed LevelDB keys) and reject entries with
extra keys (include the offending key names in the exception) to prevent
silently dropped configuration overrides.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3d377392-1c7a-41c4-93e4-409e9ccb0668

📥 Commits

Reviewing files that changed from the base of the PR and between 3e4ddbb and 05baeb8.

📒 Files selected for processing (8)
  • common/src/main/java/org/tron/core/config/README.md
  • common/src/main/java/org/tron/core/config/args/Storage.java
  • common/src/main/java/org/tron/core/config/args/StorageConfig.java
  • common/src/main/resources/reference.conf
  • common/src/test/java/org/tron/core/config/args/StorageConfigTest.java
  • framework/src/main/java/org/tron/program/SolidityNode.java
  • framework/src/test/java/org/tron/core/config/args/StorageTest.java
  • framework/src/test/resources/config-test.conf
🚧 Files skipped from review as they are similar to previous changes (3)
  • framework/src/test/resources/config-test.conf
  • framework/src/main/java/org/tron/program/SolidityNode.java
  • common/src/main/resources/reference.conf

```

As shown in the example above, the `accout` database will be stored in the path of `/path/to/accout/database` while the index be stored in `/path/to/accout/index`. And, the example also shows our default value of LevelDB options(Start from `createIfMissing` and end at `maxOpenFiles`). Please refer to the docs of [LevelDB](https://github.com/google/leveldb/blob/master/doc/index.md#performance) to figure out the details of these options.
As shown in the example above, the `accout` database will be stored in the path of `/path/to/accout/database` while the index be stored in `/path/to/accout/index`. And, the example also shows our default value of LevelDB options(Start from `blockSize` and end at `maxOpenFiles`). Please refer to the docs of [LevelDB](https://github.com/google/leveldb/blob/master/doc/index.md#performance) to figure out the details of these options.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the accout typo in the storage example.

Line 43 still tells operators to use accout in the database name/path, which makes the example misleading to copy verbatim.

🧰 Tools
🪛 LanguageTool

[grammar] ~43-~43: Ensure spelling is correct
Context: ...``` As shown in the example above, the accout database will be stored in the path of ...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~43-~43: Ensure spelling is correct
Context: ... database will be stored in the path of /path/to/accout/database while the index be stored in `/path/to/...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~43-~43: Ensure spelling is correct
Context: .../databasewhile the index be stored in/path/to/accout/index`. And, the example also shows our...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 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 `@common/src/main/java/org/tron/core/config/README.md` at line 43, Replace the
misspelled identifier "accout" with "account" in the storage example and any
matching occurrences (specifically the paths '/path/to/accout/database' and
'/path/to/accout/index' and nearby descriptive text); update the example text
that references the `accout` database to read `account` so the example can be
copied verbatim without confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants