Repository navigation
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
❌ Math Usage Detection ResultsFound forbidden usage of Please review if this usage is intended. Caution Note: You should use |
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
5 similar comments
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
bladehan1
changed the title
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
1 similar comment
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
57 issues found across 525 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="chainbase/src/main/java/org/tron/core/db2/archive/history/model/HistoryLocation.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/history/model/HistoryLocation.java:17">
P2: The constructor accepts any `bodyDigest` length, but the marker codec writes it raw and reads back exactly 32 bytes. Reject anything other than 32 bytes here so a malformed location cannot be written.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/history/model/HistoryLocation.java:49">
P2: Location end offsets can overflow and wrap negative, defeating file-bound checks for corrupted or untrusted archive metadata. Reject overflowing locations or use overflow-safe addition in both location models.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/stateroot/PathStatePhysicalRuntimeAdmission.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/stateroot/PathStatePhysicalRuntimeAdmission.java:26">
P2: An existing empty configured directory throws here instead of returning `REBUILD_REQUIRED`, so a pre-created path prevents first startup from reaching `rebuildPathStateRoot`. Detect an empty real directory as a fresh rebuild target before validating `MANIFEST`.</violation>
</file>
<file name="actuator/src/main/java/org/tron/core/archive/README.md">
<violation number="1" location="actuator/src/main/java/org/tron/core/archive/README.md:11">
P2: This describes the wrong call path. `RuntimeImpl` only picks the historical repository provider and clears the VM config snapshot in its finally block. It does not invoke `HistoricalConstantExecution`, bind the session, enter `HistoricalExecutionScope`, or clear the operation table. The coordinator is called from `Wallet`, so name that caller here and drop the claim that `RuntimeImpl` does the full sequence.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/PersistentServingKeyIndexCatalog.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/PersistentServingKeyIndexCatalog.java:215">
P1: If `persistCurrent` fails after the `CURRENT` rename, the in-memory `currentId` is not advanced, but the durable pointer already names the new generation. A later `publish` runs `discoverRetired()`, which reaps that generation as unreferenced. Update `currentId` once the rename succeeds, or treat a post-rename failure as fatal for this catalog instance.</violation>
</file>
<file name="framework/src/main/java/org/tron/core/services/jsonrpc/HistoricalRpcException.java">
<violation number="1" location="framework/src/main/java/org/tron/core/services/jsonrpc/HistoricalRpcException.java:28">
P2: This projection duplicates `HistoricalFailureCategory`, which this PR also adds, and the two give different codes for the same reasons (UNAVAILABLE, CANCELLED, DEADLINE, OVERLOADED, BUDGET, CAPABILITY, DATA_ACCESS, CORRUPT). Derive the code and category from `HistoricalFailureCategory.of(failure)` so the RPC layer and the failure taxonomy cannot diverge. If the prototype codes are intentional, say so in `HistoricalFailureCategory` as well.</violation>
<violation number="2" location="framework/src/main/java/org/tron/core/services/jsonrpc/HistoricalRpcException.java:35">
P2: `HistoricalRpcException.from` misses the base `HistoricalQueryCapabilityException` thrown directly by `HistoricalQuerySession`, so unsupported historical capabilities become generic `EXECUTION` failures. Classify the base capability exception (and retain its message/detail) here.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveProgressEnvelopeCodec.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveProgressEnvelopeCodec.java:47">
P2: `encode` can emit envelopes that its own `decode` rejects because it never enforces `MAX_PARTICIPANTS` before writing the count. Reject oversized participant lists here so a generated progress file is always decodable.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/HistoryCommitStore.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/HistoryCommitStore.java:102">
P1: Mark the batch as uncertain before writing, not after. If `writeFully` throws after a partial write, the store currently reports no uncertainty, and the next commit appends after the partial bytes. Move the two `uncertainFrom`/`uncertainThrough` assignments above the write loop so that a failed write blocks later commits.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveBootstrapAnchor.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveBootstrapAnchor.java:77">
P2: This read bypasses the anchor's 1 MiB bound and can allocate the entire file before validation, allowing a damaged `bootstrap.anchor` to cause excessive memory use or an out-of-memory startup failure. Check the file size and use a bounded read before decoding.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryWriter.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryWriter.java:64">
P2: If recovery or the anchor persist step throws, the already-opened body, index, and commit stores are never closed. Wrap the steps after the store construction so that failures call `closeAfterFailedConstruction(failure)` and rethrow, as the bootstrap block does.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryWriter.java:108">
P1: A commit-log write failure can leave partial marker bytes behind because this failure path rolls back only body and index files. Truncate the uncommitted commit-log suffix before allowing restart or subsequent writes.</violation>
<violation number="3" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryWriter.java:118">
P1: A crash during head reversion leaves `history.scan-anchor` pointing past the shortened commit log, causing the next startup to fail before recovery runs. Use the durable truncation-intent protocol for reverts, or atomically update the commit authority and scan anchor.</violation>
</file>
<file name="actuator/src/main/java/org/tron/core/vm/program/Storage.java">
<violation number="1" location="actuator/src/main/java/org/tron/core/vm/program/Storage.java:233">
P1: Deleting the legacy row for a key that was written but never read can wipe the legacy value of an aliasing key that was never migrated. Limit the legacy-row delete to keys read as OLD, since the new row already shadows any stale legacy row.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/HistorySegmentStore.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/HistorySegmentStore.java:135">
P2: If `truncateFrom` or `scan` throws here, `openAppendChannel()` never runs and `appendChannel` remains null. Every later `append` or `sync` then fails with a NullPointerException. Reopen the channel in a `finally` block, or handle the error so the store is not left without an append channel.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/HistorySegmentStore.java:163">
P2: When `last` equals the current head, `truncateAfter` keeps the old `scanResult` and its `invalidTail`, even though the bytes after `last` were just removed. Later `append` calls will keep failing until `truncateInvalidTail` runs. Rebuild the head-only `ScanResult` with a null invalid tail in this branch.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/HistoryIndexStore.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/HistoryIndexStore.java:74">
P2: `read` allocates the persisted record length before validating the offset/length range. A corrupt history location can therefore request an enormous buffer and crash recovery with `OutOfMemoryError`; validate the range using subtraction before allocating.</violation>
</file>
<file name="build.gradle">
<violation number="1" location="build.gradle:24">
P2: The JDK 17 x86 candidate build does not carry its opt-in through to runtime. A distribution built with `-Pjava17Candidate=true` is rejected by `Arch.throwIfUnsupportedJavaVersion()` unless users separately add `-Dtron.java17.x86.candidate=true`; propagate the candidate marker into the generated runtime options or otherwise keep the build and runtime switches consistent.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveRuntimeAttachment.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveRuntimeAttachment.java:46">
P2: These null guards silently turn required archive flush publication stages into no-ops for the two- and three-argument constructors. Fail fast when either publisher is absent, as the earlier forward-flush contract did, instead of allowing history to become durable but unavailable to historical queries.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/stateroot/PathStateRuntimeAdmission.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/stateroot/PathStateRuntimeAdmission.java:26">
P2: This admission reports `CURRENT_READY` for non-regular `CURRENT` entries, although the current-store loader rejects them. Check `CURRENT` with `Files.isRegularFile(..., LinkOption.NOFOLLOW_LINKS)` so malformed filesystem entries are not admitted as ready.</violation>
</file>
<file name="actuator/src/main/java/org/tron/core/vm/repository/HistoricalPropertyLog.java">
<violation number="1" location="actuator/src/main/java/org/tron/core/vm/repository/HistoricalPropertyLog.java:34">
P2: The warning throttle is not atomic, so concurrent historical requests can all pass this check and emit a burst of WARN records for the same key. Update the timestamp and decide whether to log atomically, for example with `ConcurrentHashMap.compute`.</violation>
</file>
<file name="framework/src/main/java/org/tron/core/archive/query/StateArchiveQuerySessions.java">
<violation number="1" location="framework/src/main/java/org/tron/core/archive/query/StateArchiveQuerySessions.java:35">
P2: A node configured with `maxHttpConnectNumber=0` cannot serve any historical query: this admission call turns the HTTP server's unlimited setting into a zero-capacity archive limiter. Map nonpositive values to an effectively unlimited positive admission capacity before calling `acquire`.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/query/HistoricalQueryControl.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/query/HistoricalQueryControl.java:61">
P2: `markTerminal` stores the first terminal failure but returns a later failure when the terminal is already set. Callers throw this return value, so later session failures can escape instead of rethrowing the sticky cause; return the stored terminal value after the CAS.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryReadHandles.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryReadHandles.java:43">
P2: A checkpoint-validation failure leaks the index (and similarly commit) file descriptor. Close each store’s channel when post-open scanning fails, or expose cleanup from construction, before `open()` loses the partially constructed resource; repeated failed reader opens can exhaust file descriptors.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveBaseManifest.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveBaseManifest.java:169">
P2: The loader reads the entire `MANIFEST` before applying its 1 MiB limit, so a corrupt or unexpectedly large regular manifest can exhaust memory during archive startup. Check `Files.size(path)` against `MAX_LENGTH` before `readAllBytes` (and retain the decode-time check for races).</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTargetMutationPlanFile.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTargetMutationPlanFile.java:36">
P1: The mutation-plan file is read-only in production: nothing calls `store` or `retire`, so coupled target mutations are never durably recorded or cleared and restart recovery cannot handle an in-flight transition. Wire these methods into the mutation materialization and successful-commit lifecycle before relying on `mutationPlanPresent()` for admission.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/core/CommonCheckpointRuntimeAttachment.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/core/CommonCheckpointRuntimeAttachment.java:88">
P1: `close()` can close the runtime while an already-pinned historical snapshot is still in use. Because `pinPoint` is not synchronized with shutdown and the snapshot defers its archive access until each `get`, in-flight historical RPCs can fail against closed archive resources; track active read leases and drain them before closing, or make shutdown wait for request-owned snapshots.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveAuthoritySnapshotCollector.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveAuthoritySnapshotCollector.java:37">
P2: `collect` never re-reads `history.first()`, so a first-marker change during collection bypasses drift detection. Read it again and include it in the authority-change comparison before returning the bundle.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveAuthoritySnapshotCollector.java:46">
P2: `collect` samples `applyCheckpoint()` and `participantProgress()` only once, so an atomic progress replacement during collection is never detected and the returned bundle can mix authorities from different states. Re-read and compare both values before returning, just as this method does for `readerVisible()`.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/HistoricalForkView.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/HistoricalForkView.java:37">
P2: `pass` returns cached decisions without checking the session. A retained `ForkView` can therefore ignore cancellation, terminal failures, and closure; call `session.checkActive()` before the cache lookup.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryTruncator.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryTruncator.java:46">
P1: This truncation leaves the durable scan anchor pointing at deleted records, so restarting a node after `truncateAfter` fails instead of reopening the shortened archive. Persist a target scan anchor as part of the truncation protocol, or route this operation through `ArchiveTruncationIntent` and `ArchiveTruncationRecovery`.</violation>
</file>
<file name="actuator/src/main/java/org/tron/core/vm/repository/HistoricalArchiveStateSource.java">
<violation number="1" location="actuator/src/main/java/org/tron/core/vm/repository/HistoricalArchiveStateSource.java:69">
P1: Historical storage rows lose their physical key here, so cloning the storage cache for a child repository dereferences a null `rowKey`. Preserve `physicalKey` on the returned capsule; otherwise nested historical calls can fail after a legacy storage read.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/DurableHistoryMarkerRangeEvidence.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/DurableHistoryMarkerRangeEvidence.java:28">
P2: This evidence gate rejects markers from any `ArchiveHistoryWriter` configured with a non-current participant set, even though the writer accepts and persists that set. Obtain the participant identity from the source/writer, or enforce the exact-27 precondition at the writer boundary instead of making this reader silently reject valid configured archives.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/PhysicalSnapshotPathStateCollector.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/PhysicalSnapshotPathStateCollector.java:19">
P1: This collapses a 0→1 activation into `P66_ON` while forwarding only changed physical rows. On an activation block with no changed Accounts, untouched P66-off Accounts retain embedded balances and no direct rows, so the published P66-on PathState root is inconsistent with the canonical representation; preserve the activation/migration semantics or materialize the required projections before publishing.</violation>
</file>
<file name="actuator/src/main/java/org/tron/core/archive/execution/HistoricalConstantExecution.java">
<violation number="1" location="actuator/src/main/java/org/tron/core/archive/execution/HistoricalConstantExecution.java:41">
P2: A nested historical invocation can fail before acquiring this scope but still clear the outer execution's thread-local VM configuration and operation table. Make cleanup conditional on successfully acquiring this scope (or move scope acquisition outside the cleanup-owning `try`) so a rejected nested call cannot corrupt the active execution.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/common/storage/EngineSourceIdentityFile.java">
<violation number="1" location="chainbase/src/main/java/org/tron/common/storage/EngineSourceIdentityFile.java:79">
P2: `Files.move(..., ATOMIC_MOVE)` replaces an existing destination on Unix and Windows, so the `FileAlreadyExistsException` catch never fires and the create is not no-replace. Immutability currently relies only on the `FileLock` and the `exists` check. Use `Files.createLink`, which fails atomically if the destination exists; the `finally` already removes the temp file.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveWalStartupValidator.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveWalStartupValidator.java:71">
P1: This fixed 500-marker ceiling is not enforced by the flush path, so a configured or oversized flush can persist a valid binding that every subsequent archive startup rejects. Enforce the same cap before publishing a checkpoint or validate the binding in bounded chunks without rejecting supported flush sizes.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db/TransactionContext.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db/TransactionContext.java:46">
P2: The new constructor allows `executionMode` and `isStatic` to disagree. `RuntimeImpl` uses `isStatic` for current modes, so contradictory arguments select the wrong VM behavior; reject mismatched combinations here.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/history/BlockSnapshotContinuity.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/history/BlockSnapshotContinuity.java:31">
P2: This accepts inconsistent previous layers when one state database has no block metadata: the null candidate is skipped, so continuity is checked only against the other database. Track whether a previous layer was observed and compare nullness as well as metadata.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/query/ArchiveRuntimeQueryGate.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/query/ArchiveRuntimeQueryGate.java:99">
P2: `getSnapshot()` exposes the independently closable raw snapshot, allowing callers to bypass lease accounting and leave the gate permanently undrained. Keep the raw snapshot private and have consumers use the lease’s `ArchivePointSnapshot` delegation instead.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveReadContext.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveReadContext.java:133">
P1: `resolveAccountAssets` is unusable for common-checkpoint contexts because `StateArchiveCheckpointReadSnapshot` does not support range reads. Implement the range path for that snapshot generation or reject this API for point-only snapshots before exposing it.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveReadContext.java:161">
P2: `getStorage` ignores the historical `ALLOW_OPTIMIZE_TVM_STORAGE` switch, so a state containing both row formats returns the 48-byte row even when the historical VM must use the legacy row. Read and honor that property before probing the optimized key, matching `HistoricalQuerySession.storage`.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTruncationIntent.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTruncationIntent.java:46">
P1: Production truncation never writes this intent, so a crash during `ArchiveHistoryWriter.revert` can leave the commit log ahead of the persisted scan anchor and make the next startup fail before recovery can run. Wire intent preparation into the live truncation path before shrinking commit authority.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTruncationIntent.java:83">
P2: This loader reads an unbounded intent file before applying the 2 MiB limit, allowing a corrupt oversized file to exhaust startup memory. Check `Files.size(path)` against `HEADER_LENGTH + Integer.BYTES` and `MAX_LENGTH` before `readAllBytes`, as the scan-anchor loader does.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/P66AccountAssetCodec.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/P66AccountAssetCodec.java:92">
P1: Historical prefix queries reject valid lazily migrated P66_ON Accounts instead of serving their inline asset balances. Use the same phase-aware fallback as the balance resolver: accept unoptimized inline maps and return them for prefix queries, while retaining direct-row handling for optimized Accounts.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveFormatAdmissionValidator.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveFormatAdmissionValidator.java:50">
P2: An existing archive whose metadata cannot be read can be classified as `EMPTY_NEW` here, allowing startup to take the fresh-bootstrap path instead of quarantining the unreadable state. Distinguish a definite `NoSuchFileException` from other attribute/probe failures and quarantine the latter.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTargetMutationPlanCodec.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTargetMutationPlanCodec.java:45">
P1: `encode` writes participant counts above `MAX_PARTICANTS`, but `decode` rejects them. A plan with more than 1024 participants can be persisted and then fail archive recovery; reject the count before writing.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTargetMutationPlanCodec.java:55">
P2: `encode` writes mutation values larger than `MAX_FIELD_LENGTH`, but `decode` rejects them. Validate non-null values before writing so oversized plans fail before persistence.</violation>
</file>
<file name="framework/src/main/java/org/tron/core/archive/startup/StateArchiveStartup.java">
<violation number="1" location="framework/src/main/java/org/tron/core/archive/startup/StateArchiveStartup.java:38">
P2: `attach` treats `QUARANTINE_REQUIRED` as an existing archive and proceeds into recovery, bypassing the admission contract. Reject quarantined (and null) admission results before opening the archive.</violation>
</file>
<file name="platform/src/main/java/common/org/tron/common/utils/MarketComparator.java">
<violation number="1" location="platform/src/main/java/common/org/tron/common/utils/MarketComparator.java:12">
P1: This fallback does not provide a consistent comparator: mixing unsigned ordering for malformed keys with price ordering for canonical keys violates transitivity, which can misorder entries or trigger Java’s comparison-contract failure during sorting. Treat only exactly 54-byte keys as canonical, deterministically order canonical versus noncanonical keys, and compare noncanonical keys unsigned; otherwise overlong keys also compare equal when only their suffix differs.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/query/HistoricalFailureCategory.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/query/HistoricalFailureCategory.java:82">
P2: The classifier can never produce five declared categories, so corresponding validation, header, range, indexing, or VM-timeout failures receive the generic execution code and retryability instead of their documented mappings. Add the missing typed reasons or map the relevant exception classes here.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/PartitionedHistoryBodyStore.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/PartitionedHistoryBodyStore.java:227">
P2: `select` silently drops every duplicate group after the first for a dedicated database. Reject duplicate database groups or merge all matching groups before writing, otherwise a caller supplying a non-canonical diff loses historical entries without an error.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/HistoricalAccountAssetPrefixResolver.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/HistoricalAccountAssetPrefixResolver.java:54">
P2: The prefix resolver rejects valid lazily migrated P66_ON accounts with inline asset balances. Use the same P66_ON inline/direct precedence as `HistoricalAccountAssetBalanceResolver` instead of requiring the fully migrated canonical layout.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/HistoryCommitMarkerCodec.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/HistoryCommitMarkerCodec.java:110">
P2: Bound `dbCount` by the remaining marker bytes before allocating the list; otherwise a malformed commit marker can trigger an oversized allocation and crash archive startup.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/PersistentServingKeyIndexGeneration.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/PersistentServingKeyIndexGeneration.java:375">
P1: Range queries skip the first index entry of the next raw key when a key has no change after `targetBlock`. Keep the landed candidate for processing, rather than continuing after advancing the cursor.</violation>
</file>
<file name="actuator/src/main/java/org/tron/core/vm/repository/RepositoryImpl.java">
<violation number="1" location="actuator/src/main/java/org/tron/core/vm/repository/RepositoryImpl.java:239">
P2: Historical roots reject direct dynamic-store access, but GASPRICE and BASEFEE still use that accessor. Route their `ENERGY_FEE` reads through the historical typed-property path so valid pure opcodes read S(T) rather than failing closed.</violation>
</file>
<file name="platform/src/main/java/common/org/tron/common/arch/Arch.java">
<violation number="1" location="platform/src/main/java/common/org/tron/common/arch/Arch.java:81">
P2: The `tron.java17.x86.candidate` opt-in does not check the Java version. With it set, x86 nodes on any non-8 JVM (for example 11 or 21) pass the startup guard, not only Java 17. Gate the flag on `isJava17()` so the bypass applies only to the intended Java 17 candidate.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Tip: Get a Slack DM when cubic finishes reviewing your PRs. Connect Slack
View guided diff | Turn on auto-fix | Re-trigger cubic
| "Archive truncation target identity does not match index and body"); | ||
| } | ||
|
|
||
| commits.truncateAfter(lastEpoch); |
There was a problem hiding this comment.
P1: This truncation leaves the durable scan anchor pointing at deleted records, so restarting a node after truncateAfter fails instead of reopening the shortened archive. Persist a target scan anchor as part of the truncation protocol, or route this operation through ArchiveTruncationIntent and ArchiveTruncationRecovery.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryTruncator.java, line 46:
<comment>This truncation leaves the durable scan anchor pointing at deleted records, so restarting a node after `truncateAfter` fails instead of reopening the shortened archive. Persist a target scan anchor as part of the truncation protocol, or route this operation through `ArchiveTruncationIntent` and `ArchiveTruncationRecovery`.</comment>
<file context>
@@ -0,0 +1,72 @@
+ "Archive truncation target identity does not match index and body");
+ }
+
+ commits.truncateAfter(lastEpoch);
+ faultHook.afterDurableStage(Stage.COMMIT_AUTHORITY);
+ index.truncateAfter(target.getIndexLocation(), commits.size());
</file context>
| this.encodedMarker = Arrays.copyOf(encodedMarker, encodedMarker.length); | ||
| } | ||
|
|
||
| static ArchiveTruncationIntent prepare(Path archiveDirectory, HistoryCommitStore commits, |
There was a problem hiding this comment.
P1: Production truncation never writes this intent, so a crash during ArchiveHistoryWriter.revert can leave the commit log ahead of the persisted scan anchor and make the next startup fail before recovery can run. Wire intent preparation into the live truncation path before shrinking commit authority.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTruncationIntent.java, line 46:
<comment>Production truncation never writes this intent, so a crash during `ArchiveHistoryWriter.revert` can leave the commit log ahead of the persisted scan anchor and make the next startup fail before recovery can run. Wire intent preparation into the live truncation path before shrinking commit authority.</comment>
<file context>
@@ -0,0 +1,231 @@
+ this.encodedMarker = Arrays.copyOf(encodedMarker, encodedMarker.length);
+ }
+
+ static ArchiveTruncationIntent prepare(Path archiveDirectory, HistoryCommitStore commits,
+ HistoryIndexStore index, HistoryBodyStore bodies, long targetEpoch,
+ HistoryCommitMarkerCodec markerCodec) throws IOException {
</file context>
| HistoryCommitMarker head = commits.head(); | ||
| if (head != null && head.getMeta().equals(meta)) { | ||
| HistoryCommitMarker previous = commits.get(meta.getEpoch() - 1); | ||
| commits.removeHead(meta); |
There was a problem hiding this comment.
P1: A crash during head reversion leaves history.scan-anchor pointing past the shortened commit log, causing the next startup to fail before recovery runs. Use the durable truncation-intent protocol for reverts, or atomically update the commit authority and scan anchor.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryWriter.java, line 118:
<comment>A crash during head reversion leaves `history.scan-anchor` pointing past the shortened commit log, causing the next startup to fail before recovery runs. Use the durable truncation-intent protocol for reverts, or atomically update the commit authority and scan anchor.</comment>
<file context>
@@ -0,0 +1,565 @@
+ HistoryCommitMarker head = commits.head();
+ if (head != null && head.getMeta().equals(meta)) {
+ HistoryCommitMarker previous = commits.get(meta.getEpoch() - 1);
+ commits.removeHead(meta);
+ persistHistoryScanAnchor();
+ previous = commits.head();
</file context>
| persistChunk(diffs.subList(start, end)); | ||
| } | ||
| } catch (IOException | RuntimeException e) { | ||
| handleWriteFailure(diffs.get(diffs.size() - 1).getMeta(), e); |
There was a problem hiding this comment.
P1: A commit-log write failure can leave partial marker bytes behind because this failure path rolls back only body and index files. Truncate the uncommitted commit-log suffix before allowing restart or subsequent writes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryWriter.java, line 108:
<comment>A commit-log write failure can leave partial marker bytes behind because this failure path rolls back only body and index files. Truncate the uncommitted commit-log suffix before allowing restart or subsequent writes.</comment>
<file context>
@@ -0,0 +1,565 @@
+ persistChunk(diffs.subList(start, end));
+ }
+ } catch (IOException | RuntimeException e) {
+ handleWriteFailure(diffs.get(diffs.size() - 1).getMeta(), e);
+ }
+ }
</file context>
| this.faultHook = Objects.requireNonNull(faultHook, "faultHook"); | ||
| } | ||
|
|
||
| void store(ArchiveTargetMutationPlan plan) throws IOException { |
There was a problem hiding this comment.
P1: The mutation-plan file is read-only in production: nothing calls store or retire, so coupled target mutations are never durably recorded or cleared and restart recovery cannot handle an in-flight transition. Wire these methods into the mutation materialization and successful-commit lifecycle before relying on mutationPlanPresent() for admission.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/ArchiveTargetMutationPlanFile.java, line 36:
<comment>The mutation-plan file is read-only in production: nothing calls `store` or `retire`, so coupled target mutations are never durably recorded or cleared and restart recovery cannot handle an in-flight transition. Wire these methods into the mutation materialization and successful-commit lifecycle before relying on `mutationPlanPresent()` for admission.</comment>
<file context>
@@ -0,0 +1,97 @@
+ this.faultHook = Objects.requireNonNull(faultHook, "faultHook");
+ }
+
+ void store(ArchiveTargetMutationPlan plan) throws IOException {
+ byte[] encoded = codec.encode(plan);
+ Files.createDirectories(path.getParent());
</file context>
| this.index = new HistoryIndexStore(archiveDirectory, new HistoryIndexCodec(), checkpoint); | ||
| this.commits = new HistoryCommitStore(archiveDirectory, commitCodec, checkpoint); | ||
| this.hook = hook; | ||
| recoverPreparedSuffix(); |
There was a problem hiding this comment.
P2: If recovery or the anchor persist step throws, the already-opened body, index, and commit stores are never closed. Wrap the steps after the store construction so that failures call closeAfterFailedConstruction(failure) and rethrow, as the bootstrap block does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/ArchiveHistoryWriter.java, line 64:
<comment>If recovery or the anchor persist step throws, the already-opened body, index, and commit stores are never closed. Wrap the steps after the store construction so that failures call `closeAfterFailedConstruction(failure)` and rethrow, as the bootstrap block does.</comment>
<file context>
@@ -0,0 +1,565 @@
+ this.index = new HistoryIndexStore(archiveDirectory, new HistoryIndexCodec(), checkpoint);
+ this.commits = new HistoryCommitStore(archiveDirectory, commitCodec, checkpoint);
+ this.hook = hook;
+ recoverPreparedSuffix();
+ persistHistoryScanAnchor();
+ if (commits.head() != null) {
</file context>
| } | ||
|
|
||
| public synchronized HistoryIndexRecord read(HistoryIndexLocation location) throws IOException { | ||
| if (location.endOffset() > channel.size()) { |
There was a problem hiding this comment.
P2: read allocates the persisted record length before validating the offset/length range. A corrupt history location can therefore request an enormous buffer and crash recovery with OutOfMemoryError; validate the range using subtraction before allocating.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/HistoryIndexStore.java, line 74:
<comment>`read` allocates the persisted record length before validating the offset/length range. A corrupt history location can therefore request an enormous buffer and crash recovery with `OutOfMemoryError`; validate the range using subtraction before allocating.</comment>
<file context>
@@ -0,0 +1,286 @@
+ }
+
+ public synchronized HistoryIndexRecord read(HistoryIndexLocation location) throws IOException {
+ if (location.endOffset() > channel.size()) {
+ throw new IllegalArgumentException("History index location is outside file bounds");
+ }
</file context>
Suggested change
| if (location.endOffset() > channel.size()) { | |
| if (location.getOffset() < 0 || location.getRecordLength() <= 0 | |
| || location.getOffset() > channel.size() - location.getRecordLength()) { |
Comment on lines
+79
to
+85
| Files.move(temporary, destination, StandardCopyOption.ATOMIC_MOVE); | ||
| } catch (FileAlreadyExistsException raced) { | ||
| // Another opener established the immutable identity first; validate it below. | ||
| } catch (AtomicMoveNotSupportedException unsupported) { | ||
| throw new IOException("Engine identity filesystem does not support atomic create", | ||
| unsupported); | ||
| } |
There was a problem hiding this comment.
P2: Files.move(..., ATOMIC_MOVE) replaces an existing destination on Unix and Windows, so the FileAlreadyExistsException catch never fires and the create is not no-replace. Immutability currently relies only on the FileLock and the exists check. Use Files.createLink, which fails atomically if the destination exists; the finally already removes the temp file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/common/storage/EngineSourceIdentityFile.java, line 79:
<comment>`Files.move(..., ATOMIC_MOVE)` replaces an existing destination on Unix and Windows, so the `FileAlreadyExistsException` catch never fires and the create is not no-replace. Immutability currently relies only on the `FileLock` and the `exists` check. Use `Files.createLink`, which fails atomically if the destination exists; the `finally` already removes the temp file.</comment>
<file context>
@@ -0,0 +1,182 @@
+ channel.force(true);
+ }
+ try {
+ Files.move(temporary, destination, StandardCopyOption.ATOMIC_MOVE);
+ } catch (FileAlreadyExistsException raced) {
+ // Another opener established the immutable identity first; validate it below.
</file context>
Suggested change
| Files.move(temporary, destination, StandardCopyOption.ATOMIC_MOVE); | |
| } catch (FileAlreadyExistsException raced) { | |
| // Another opener established the immutable identity first; validate it below. | |
| } catch (AtomicMoveNotSupportedException unsupported) { | |
| throw new IOException("Engine identity filesystem does not support atomic create", | |
| unsupported); | |
| } | |
| Files.createLink(destination, temporary); | |
| } catch (FileAlreadyExistsException raced) { | |
| // Another opener established the immutable identity first; validate it below. | |
| } catch (UnsupportedOperationException unsupported) { | |
| throw new IOException("Engine identity filesystem does not support atomic create", | |
| unsupported); | |
| } |
| this.snapshot = snapshot; | ||
| } | ||
|
|
||
| public synchronized ArchiveReadSnapshot getSnapshot() { |
There was a problem hiding this comment.
P2: getSnapshot() exposes the independently closable raw snapshot, allowing callers to bypass lease accounting and leave the gate permanently undrained. Keep the raw snapshot private and have consumers use the lease’s ArchivePointSnapshot delegation instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/query/ArchiveRuntimeQueryGate.java, line 99:
<comment>`getSnapshot()` exposes the independently closable raw snapshot, allowing callers to bypass lease accounting and leave the gate permanently undrained. Keep the raw snapshot private and have consumers use the lease’s `ArchivePointSnapshot` delegation instead.</comment>
<file context>
@@ -0,0 +1,153 @@
+ this.snapshot = snapshot;
+ }
+
+ public synchronized ArchiveReadSnapshot getSnapshot() {
+ if (closed) {
+ throw new IllegalStateException("Archive query lease is closed");
</file context>
❌ Math Usage Detection ResultsFound forbidden usage of Please review if this usage is intended. Caution Note: You should use |
There was a problem hiding this comment.
2 issues found across 37 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="chainbase/src/main/java/org/tron/core/db2/archive/query/snapshot/CheckpointLatestStateFactory.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/query/snapshot/CheckpointLatestStateFactory.java:14">
P3: The default `get` pins the entire latest-state generation on every key read, while `LatestStateGenerationAdapter` overrides it to read a single Store. Any implementation that inherits this default, including lambdas, pays the full-generation pin cost per read without any signal. Document this fallback cost on the method, or override `get` in such implementations.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/PersistentServingKeyIndexCatalog.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/PersistentServingKeyIndexCatalog.java:30">
P2: Implementing `ServingKeyIndexSource` makes `pin(readerVisible)` a query-facing entry point, but a `null` argument silently skips the reader-visibility check. Reject null in the interface-facing overload so historical reads fail closed.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
View guided diff | Turn on auto-fix | Re-trigger cubic
| import org.tron.core.db2.stateroot.PathStateStoreManifest.Engine; | ||
|
|
||
| /** Durable generation catalog with request refcounts and safe retired-generation reaping. */ | ||
| public final class PersistentServingKeyIndexCatalog implements Closeable, ServingKeyIndexSource { |
There was a problem hiding this comment.
P2: Implementing ServingKeyIndexSource makes pin(readerVisible) a query-facing entry point, but a null argument silently skips the reader-visibility check. Reject null in the interface-facing overload so historical reads fail closed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/PersistentServingKeyIndexCatalog.java, line 30:
<comment>Implementing `ServingKeyIndexSource` makes `pin(readerVisible)` a query-facing entry point, but a `null` argument silently skips the reader-visibility check. Reject null in the interface-facing overload so historical reads fail closed.</comment>
<file context>
@@ -23,10 +23,11 @@
/** Durable generation catalog with request refcounts and safe retired-generation reaping. */
-public final class PersistentServingKeyIndexCatalog implements Closeable {
+public final class PersistentServingKeyIndexCatalog implements Closeable, ServingKeyIndexSource {
private static final int MAGIC = 0x534b4943; // SKIC
</file context>
| public interface CheckpointLatestStateFactory { | ||
| PinnedLatestState pin(long blockNumber, byte[] blockHash) throws IOException; | ||
|
|
||
| default OldValue get(long blockNumber, byte[] blockHash, String dbName, |
There was a problem hiding this comment.
P3: The default get pins the entire latest-state generation on every key read, while LatestStateGenerationAdapter overrides it to read a single Store. Any implementation that inherits this default, including lambdas, pays the full-generation pin cost per read without any signal. Document this fallback cost on the method, or override get in such implementations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/query/snapshot/CheckpointLatestStateFactory.java, line 14:
<comment>The default `get` pins the entire latest-state generation on every key read, while `LatestStateGenerationAdapter` overrides it to read a single Store. Any implementation that inherits this default, including lambdas, pays the full-generation pin cost per read without any signal. Document this fallback cost on the method, or override `get` in such implementations.</comment>
<file context>
@@ -0,0 +1,25 @@
+public interface CheckpointLatestStateFactory {
+ PinnedLatestState pin(long blockNumber, byte[] blockHash) throws IOException;
+
+ default OldValue get(long blockNumber, byte[] blockHash, String dbName,
+ byte[] physicalRawKey) throws IOException {
+ try (PinnedLatestState latest = Objects.requireNonNull(
</file context>
Rebuild the state_archive source on develop only. Exclude inherited release implementations and their tests. Preserve archive, path state, checkpoint and functional package work.
❌ Math Usage Detection ResultsFound forbidden usage of Please review if this usage is intended. Caution Note: You should use |
There was a problem hiding this comment.
1 issue found across 47 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="actuator/src/main/java/org/tron/core/actuator/VMActuator.java">
<violation number="1" location="actuator/src/main/java/org/tron/core/actuator/VMActuator.java:340">
P1: This condition stops cleaning up latest constant calls, even though `ConfigLoader.load(..., isConstantCall)` installs their local snapshot and `RuntimeImpl` only cleans up historical calls. Restore cleanup for every constant call so stale VM flags and request tables cannot leak into later executions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| } | ||
| logger.info("runtime result is :{}", result.getException().getMessage()); | ||
| } finally { | ||
| if (isConstantCall && rootRepository != null && rootRepository.isHistorical()) { |
There was a problem hiding this comment.
P1: This condition stops cleaning up latest constant calls, even though ConfigLoader.load(..., isConstantCall) installs their local snapshot and RuntimeImpl only cleans up historical calls. Restore cleanup for every constant call so stale VM flags and request tables cannot leak into later executions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At actuator/src/main/java/org/tron/core/actuator/VMActuator.java, line 340:
<comment>This condition stops cleaning up latest constant calls, even though `ConfigLoader.load(..., isConstantCall)` installs their local snapshot and `RuntimeImpl` only cleans up historical calls. Restore cleanup for every constant call so stale VM flags and request tables cannot leak into later executions.</comment>
<file context>
@@ -337,7 +337,7 @@ public void execute(Object object) throws ContractExeException {
logger.info("runtime result is :{}", result.getException().getMessage());
} finally {
- if (isConstantCall) {
+ if (isConstantCall && rootRepository != null && rootRepository.isHistorical()) {
VMConfig.clearLocalSnapshot();
OperationRegistry.clearRequestTable();
</file context>
Suggested change
| if (isConstantCall && rootRepository != null && rootRepository.isHistorical()) { | |
| if (isConstantCall) { |
bladehan1
force-pushed
the
feature/state_archive
branch
from
October 21, 2026 06:04
1b33ada to
4ce1516
Compare
❌ Math Usage Detection ResultsFound forbidden usage of Please review if this usage is intended. Caution Note: You should use |
There was a problem hiding this comment.
4 issues found across 19 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="platform/src/main/java/x86-high/org/tron/common/storage/RocksDbEnvBudget.java">
<violation number="1" location="platform/src/main/java/x86-high/org/tron/common/storage/RocksDbEnvBudget.java:31">
P3: This swaps `Math.max` for `MathWrapper.max`, which is `@Deprecated` in the x86-high source set and only delegates to `Math.max`. The result is identical, so this adds a deprecated dependency and an unneeded import for no benefit. Revert to `Math.max` (or use the non-deprecated `StrictMathWrapper.max` if a wrapper is wanted), and drop the `MathWrapper` import.</violation>
</file>
<file name="platform/src/main/java/common/org/tron/common/storage/RocksDbEnvHighWater.java">
<violation number="1">
P3: This new code uses `MathWrapper`, which is `@Deprecated` and whose Javadoc says not to use it in new code. Use `StrictMathWrapper` instead. It lives in the shared `common` module, and this file already sits in that source set.</violation>
</file>
<file name="framework/src/test/java/org/tron/common/BaseMethodTest.java">
<violation number="1" location="framework/src/test/java/org/tron/common/BaseMethodTest.java:62">
P2: These resets are not protected by `finally`: an exception from `beforeDestroy()`, `context.close()`, or `Args.clearParam()` leaves the current test thread's VM snapshot and request table installed. Wrap teardown cleanup in `finally` so later tests cannot observe stale historical VM state.</violation>
</file>
<file name="framework/src/test/java/org/tron/core/db2/NativeCheckpointTestSupport.java">
<violation number="1" location="framework/src/test/java/org/tron/core/db2/NativeCheckpointTestSupport.java:24">
P2: This backend switch does not preserve the database location: LevelDB appends the configured `db.directory`, but RocksDB opens directly under `directory`. Passing an output root therefore makes the same test fixture use different paths and prevents it from being reopened through the standard storage layout; append the configured database directory for the RocksDB branch too.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| @@ -57,6 +59,8 @@ protected String configFile() { | |||
|
|
|||
| @Before | |||
| public final void initContext() throws IOException { | |||
| VMConfig.clearLocalSnapshot(); | |||
There was a problem hiding this comment.
P2: These resets are not protected by finally: an exception from beforeDestroy(), context.close(), or Args.clearParam() leaves the current test thread's VM snapshot and request table installed. Wrap teardown cleanup in finally so later tests cannot observe stale historical VM state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At framework/src/test/java/org/tron/common/BaseMethodTest.java, line 62:
<comment>These resets are not protected by `finally`: an exception from `beforeDestroy()`, `context.close()`, or `Args.clearParam()` leaves the current test thread's VM snapshot and request table installed. Wrap teardown cleanup in `finally` so later tests cannot observe stale historical VM state.</comment>
<file context>
@@ -57,6 +59,8 @@ protected String configFile() {
@Before
public final void initContext() throws IOException {
+ VMConfig.clearLocalSnapshot();
+ OperationRegistry.clearRequestTable();
String[] baseArgs = new String[]{
</file context>
|
|
||
| public static DB<byte[], byte[]> database(Path directory, String name) { | ||
| return engine() == Engine.ROCKSDB | ||
| ? new RocksDB(new RocksDbDataSourceImpl(directory.toString(), name)) |
There was a problem hiding this comment.
P2: This backend switch does not preserve the database location: LevelDB appends the configured db.directory, but RocksDB opens directly under directory. Passing an output root therefore makes the same test fixture use different paths and prevents it from being reopened through the standard storage layout; append the configured database directory for the RocksDB branch too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At framework/src/test/java/org/tron/core/db2/NativeCheckpointTestSupport.java, line 24:
<comment>This backend switch does not preserve the database location: LevelDB appends the configured `db.directory`, but RocksDB opens directly under `directory`. Passing an output root therefore makes the same test fixture use different paths and prevents it from being reopened through the standard storage layout; append the configured database directory for the RocksDB branch too.</comment>
<file context>
@@ -0,0 +1,27 @@
+
+ public static DB<byte[], byte[]> database(Path directory, String name) {
+ return engine() == Engine.ROCKSDB
+ ? new RocksDB(new RocksDbDataSourceImpl(directory.toString(), name))
+ : new LevelDB(new LevelDbDataSourceImpl(directory.toString(), name));
+ }
</file context>
Suggested change
| ? new RocksDB(new RocksDbDataSourceImpl(directory.toString(), name)) | |
| ? new RocksDB(new RocksDbDataSourceImpl(directory.resolve( | |
| org.tron.common.parameter.CommonParameter.getInstance().getStorage().getDbDirectory()) | |
| .toString(), name)) |
| public static void raise(int compactionThreads, int flushThreads) { | ||
| RocksDB.loadLibrary(); | ||
| Env env = Env.getDefault(); | ||
| env.incBackgroundThreadsIfNeeded(MathWrapper.max(1, compactionThreads), Priority.LOW); |
There was a problem hiding this comment.
P3: This swaps Math.max for MathWrapper.max, which is @Deprecated in the x86-high source set and only delegates to Math.max. The result is identical, so this adds a deprecated dependency and an unneeded import for no benefit. Revert to Math.max (or use the non-deprecated StrictMathWrapper.max if a wrapper is wanted), and drop the MathWrapper import.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At platform/src/main/java/x86-high/org/tron/common/storage/RocksDbEnvBudget.java, line 31:
<comment>This swaps `Math.max` for `MathWrapper.max`, which is `@Deprecated` in the x86-high source set and only delegates to `Math.max`. The result is identical, so this adds a deprecated dependency and an unneeded import for no benefit. Revert to `Math.max` (or use the non-deprecated `StrictMathWrapper.max` if a wrapper is wanted), and drop the `MathWrapper` import.</comment>
<file context>
@@ -27,8 +28,8 @@ public static boolean canReadPoolSizes() {
Env env = Env.getDefault();
- env.incBackgroundThreadsIfNeeded(Math.max(1, compactionThreads), Priority.LOW);
- env.incBackgroundThreadsIfNeeded(Math.max(1, flushThreads), Priority.HIGH);
+ env.incBackgroundThreadsIfNeeded(MathWrapper.max(1, compactionThreads), Priority.LOW);
+ env.incBackgroundThreadsIfNeeded(MathWrapper.max(1, flushThreads), Priority.HIGH);
}
</file context>
Suggested change
| env.incBackgroundThreadsIfNeeded(MathWrapper.max(1, compactionThreads), Priority.LOW); | |
| env.incBackgroundThreadsIfNeeded(Math.max(1, compactionThreads), Priority.LOW); |
| @@ -0,0 +1,59 @@ | |||
| package org.tron.common.storage; | |||
There was a problem hiding this comment.
P3: This new code uses MathWrapper, which is @Deprecated and whose Javadoc says not to use it in new code. Use StrictMathWrapper instead. It lives in the shared common module, and this file already sits in that source set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At platform/src/main/java/common/org/tron/common/storage/RocksDbEnvHighWater.java, line 44:
<comment>This new code uses `MathWrapper`, which is `@Deprecated` and whose Javadoc says not to use it in new code. Use `StrictMathWrapper` instead. It lives in the shared `common` module, and this file already sits in that source set.</comment>
<file context>
@@ -39,8 +41,9 @@ public interface PoolSetter {
PoolSetter setter) {
- compactionHighWater = Math.max(compactionHighWater, Math.max(1, compactionThreads));
- flushHighWater = Math.max(flushHighWater, Math.max(1, flushThreads));
+ compactionHighWater = MathWrapper.max(compactionHighWater,
+ MathWrapper.max(1, compactionThreads));
+ flushHighWater = MathWrapper.max(flushHighWater, MathWrapper.max(1, flushThreads));
</file context>
move archive implementation and fixtures into owning packages and relocate source readmes to docs. keep storage formats and internal writer ownership unchanged. validation: 508 archive/checkpoint/startup tests and the large-block test pass; standard checks pass with no added style diagnostics.
There was a problem hiding this comment.
23 issues found across 265 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="chainbase/src/main/java/org/tron/core/db2/archive/contract/AccountAssetStorage.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/contract/AccountAssetStorage.java:10">
P2: This file is meant to be the single source of the physical store names, but other classes still declare or inline the same values (e.g., `ArchiveStoreScope.ACCOUNT_DATABASE`, `P66LaneStartup.ACCOUNT_ASSET`, and the `"account-asset"` literals). Reference these constants from `AccountAssetStorage` instead, or drop the duplicates so a rename cannot leave the copies out of sync.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/history/store/ArchiveHistoryBootstrap.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/history/store/ArchiveHistoryBootstrap.java:45">
P2: A single-component relative archive path causes bootstrap to fail because `root.getParent()` is null. Derive the parent from `root.toAbsolutePath()` or normalize the root to an absolute path first.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/history/store/ArchiveHistoryBootstrap.java:78">
P2: The catch only rethrows, so a failed bootstrap leaves its hidden `.<root>.bootstrap-<uuid>` staging directory in the parent. Nothing sweeps these, so each failed startup adds another. Delete the staging tree in this catch, as `ArchiveBootstrapAnchor.store` does for its temporary file. After a successful move the path is gone, and `FileUtil.deleteDir` returns false without throwing.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexTiming.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexTiming.java:12">
P3: Nothing outside `org.tron.core.db2.archive.serving` uses `ServingIndexTiming`, so these `public` modifiers are unnecessary. They expose the thread-local timing scope and its recording hooks to other packages. Drop `public` from the class, the `Stage` enum, the constructor, and all static methods, as the original package-private declarations had them.</violation>
<violation number="2" location="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexTiming.java:130">
P2: `ServingIndexTiming.source(...)` has no caller anywhere in the repo, so it is dead code. Its "Serving source read" log line and the `source` duration/work series (`locked_read`, `decode`, `encoded_bytes`, `frames`) are never emitted. Wire it into the source read path in `StateArchiveServingWorkerV3.readSource` with the decode and frame measurements it needs, or remove it.</violation>
</file>
<file name="docs/archive/root-boundaries.md">
<violation number="1" location="docs/archive/root-boundaries.md:9">
P3: `TASK-039` is an internal reference that no file in the repository defines, so readers cannot follow it. Inline the validation identities and scope, or link to a durable location.</violation>
<violation number="2" location="docs/archive/root-boundaries.md:21">
P3: The responsibility table is presented as complete, but the `recovery` and `recovery.wal` packages are missing. Add a row for them so ownership of commit-log and WAL recovery is discoverable.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexIncrementalPlan.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexIncrementalPlan.java:150">
P2: This overload is now public, and its Javadoc says it is only for test and diagnostic callers, but `StateArchiveServingIndexBuildCoordinatorV3.runFlush` calls it in production. Its legacy step-digest encoding also differs from the frame-digest `stepDigest` used by `ServingIndexService`, and both write the same `serving-index-v3` directory. Either update the Javadoc to match production use, or confirm the two encodings never mix on one index.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/checkpoint/storage/SharedCheckpointDatabase.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/checkpoint/storage/SharedCheckpointDatabase.java:258">
P3: This comment says payload bytes already passed codec checksum validation, but `publish()` passes a caller-built payload that was never decoded. Reword it to say the payload was validated by its constructor.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/history/format/V5HistoryTail.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/history/format/V5HistoryTail.java:42">
P2: `V5HistoryTail.create` has no callers in chainbase or framework, so it is dead code. Remove it. It also takes raw block, hash and digest bytes without a `CommonCheckpointTarget`, bypassing the admission that `forTarget` enforces.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/storage/index/StateArchiveIndexEngineManifest.java">
<violation number="1">
P3: This widens the ENGINE identity writer API to every package, including `openOrCreateLegacy`, which can admit a legacy RocksDB directory. The archive docs say moving a file does not justify making members public. Expose only the needed entry points, for example through `ArchiveIndexStorage`, and keep the writers and the legacy admission package-private.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/history/format/V5Format.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/history/format/V5Format.java:118">
P3: `expectedIndexLength` duplicates `StateArchiveLaneIndexV5.expectedLength`, leaving the persisted index-size rule in two implementations. Make one helper canonical and delegate to it from the other.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/contract/HistoricalQueryException.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/contract/HistoricalQueryException.java:1">
P3: The package move leaves `docs/archive/storage.md` listing `HistoricalQueryException` under `query`, contradicting the new `contract` package and the root-boundaries documentation. Remove it from the query entry-point list or add it to the contract entry points.</violation>
</file>
<file name="docs/archive/storage.md">
<violation number="1" location="docs/archive/storage.md:53">
P3: This guide describes the pre-migration serving layout: the serving implementation classes now live under `serving/`, and only six Java files remain at the archive root. Update these ownership statements and the “69 root files” count so the source guide does not direct readers to nonexistent root-package implementations.</violation>
<violation number="2" location="docs/archive/storage.md:111">
P3: This row lists classes that are not in the root package. Either drop the 'root' framing for this table or move the description to their owning packages, e.g. `query/latest/ArchiveStateBarrier`, `contract/StateArchiveCanonicalBlockMetaSource`.</violation>
<violation number="3" location="docs/archive/storage.md:116">
P2: The count is wrong: the root package holds six files, and `root-boundaries.md` describes six shared root contracts. Replace "69 root files" with the six contracts.</violation>
</file>
<file name="docs/archive/execution.md">
<violation number="1" location="docs/archive/execution.md:19">
P3: The link text `repository/` suggests a source directory, but this target is the `repository.md` guide, and the table above uses the same label for the source directory. Use a label that names the guide, such as `repository guide`.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingFailure.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingFailure.java:50">
P3: This constructor does not need `public`: all `new ServingFailure(...)` calls are inside the `serving` package. Keep it package-private to match the class's "never leave the serving component" contract.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/history/store/ArchiveHistoryAdmission.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/history/store/ArchiveHistoryAdmission.java:10">
P3: The class javadoc promises no store or manifest write access, but `requireFixedPoint` takes the full mutable `ArchiveHistoryWriter`. Either narrow the javadoc to describe what is actually guaranteed, or narrow the parameter type to a read-only view.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexService.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexService.java:175">
P3: The failure path closes `point` without suppressing a `close()` exception, so an IOException from `close()` replaces the original failure. Wrap this in try/catch and call `failure.addSuppressed(closeFailure)`, as the materializer's `pinHistory` already does.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/storage/index/ArchiveIndexStorage.java">
<violation number="1">
P2: `put` accepts a null value and silently turns it into a delete, because the writers test `mutation.value == null` and call `batch.delete`. Reject null in `put` and keep `delete` as the only way to remove a key.</violation>
<violation number="2">
P2: `nextKey()` still loads and copies each value on LevelDB because `LevelCursor` inherits this default. Override `nextKey()` in `LevelCursor` to return the iterator key directly before advancing, matching the RocksDB implementation; otherwise key-only checkpoint scans allocate and copy every payload.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db2/archive/serving/StateArchiveServingIndexBuildCoordinatorV3.java">
<violation number="1">
P2: This new public static `retireLegacyArchiveTail(Path, Engine)` writes to the serving DB without the coordinator's `requireOpen()` or monitor. If a live coordinator on the same archive root is publishing a tail, the unlocked get-then-delete can remove a freshly published `ARCHIVE_TAIL_KEY_V5`. Consider restricting it to package-private or documenting that no live coordinator may exist for that root.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
View guided diff | Turn on auto-fix | Re-trigger cubic
| public final class AccountAssetStorage { | ||
|
|
||
| public static final String PROPERTIES_DATABASE = "properties"; | ||
| public static final String ACCOUNT_DATABASE = "account"; |
There was a problem hiding this comment.
P2: This file is meant to be the single source of the physical store names, but other classes still declare or inline the same values (e.g., ArchiveStoreScope.ACCOUNT_DATABASE, P66LaneStartup.ACCOUNT_ASSET, and the "account-asset" literals). Reference these constants from AccountAssetStorage instead, or drop the duplicates so a rename cannot leave the copies out of sync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/contract/AccountAssetStorage.java, line 10:
<comment>This file is meant to be the single source of the physical store names, but other classes still declare or inline the same values (e.g., `ArchiveStoreScope.ACCOUNT_DATABASE`, `P66LaneStartup.ACCOUNT_ASSET`, and the `"account-asset"` literals). Reference these constants from `AccountAssetStorage` instead, or drop the duplicates so a rename cannot leave the copies out of sync.</comment>
<file context>
@@ -0,0 +1,21 @@
+public final class AccountAssetStorage {
+
+ public static final String PROPERTIES_DATABASE = "properties";
+ public static final String ACCOUNT_DATABASE = "account";
+ public static final String ACCOUNT_ASSET_DATABASE = "account-asset";
+ private static final byte[] PROPOSAL_66_PHYSICAL_KEY =
</file context>
| } | ||
| ArchiveFileDurability.syncDirectory(parent); | ||
| return recovery.recoverPublished(root, maxSegmentSize); | ||
| } catch (IOException | RuntimeException failure) { |
There was a problem hiding this comment.
P2: The catch only rethrows, so a failed bootstrap leaves its hidden .<root>.bootstrap-<uuid> staging directory in the parent. Nothing sweeps these, so each failed startup adds another. Delete the staging tree in this catch, as ArchiveBootstrapAnchor.store does for its temporary file. After a successful move the path is gone, and FileUtil.deleteDir returns false without throwing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/history/store/ArchiveHistoryBootstrap.java, line 78:
<comment>The catch only rethrows, so a failed bootstrap leaves its hidden `.<root>.bootstrap-<uuid>` staging directory in the parent. Nothing sweeps these, so each failed startup adds another. Delete the staging tree in this catch, as `ArchiveBootstrapAnchor.store` does for its temporary file. After a successful move the path is gone, and `FileUtil.deleteDir` returns false without throwing.</comment>
<file context>
@@ -0,0 +1,103 @@
+ }
+ ArchiveFileDurability.syncDirectory(parent);
+ return recovery.recoverPublished(root, maxSegmentSize);
+ } catch (IOException | RuntimeException failure) {
+ throw failure;
+ }
</file context>
| return result; | ||
| } | ||
|
|
||
| public static void source(long from, long through, long totalNanos, long decodeNanos, |
There was a problem hiding this comment.
P2: ServingIndexTiming.source(...) has no caller anywhere in the repo, so it is dead code. Its "Serving source read" log line and the source duration/work series (locked_read, decode, encoded_bytes, frames) are never emitted. Wire it into the source read path in StateArchiveServingWorkerV3.readSource with the decode and frame measurements it needs, or remove it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexTiming.java, line 130:
<comment>`ServingIndexTiming.source(...)` has no caller anywhere in the repo, so it is dead code. Its "Serving source read" log line and the `source` duration/work series (`locked_read`, `decode`, `encoded_bytes`, `frames`) are never emitted. Wire it into the source read path in `StateArchiveServingWorkerV3.readSource` with the decode and frame measurements it needs, or remove it.</comment>
<file context>
@@ -127,7 +127,7 @@ private EnumMap<Stage, Long> microseconds() {
}
- static void source(long from, long through, long totalNanos, long decodeNanos,
+ public static void source(long from, long through, long totalNanos, long decodeNanos,
long bytes, long frames, boolean success) {
LOGGER.info("Serving source read: from={}, through={}, success={}, encodedBytes={}, "
</file context>
| * they only choose batch boundaries (ch04 S3, §3.5.3). Without frame digests the step digest | ||
| * retains the legacy complete-diff encoding (test and diagnostic callers). | ||
| */ | ||
| public static ServingIndexIncrementalPlan planCommittedDiffs(long indexedThrough, byte[] headHash, |
There was a problem hiding this comment.
P2: This overload is now public, and its Javadoc says it is only for test and diagnostic callers, but StateArchiveServingIndexBuildCoordinatorV3.runFlush calls it in production. Its legacy step-digest encoding also differs from the frame-digest stepDigest used by ServingIndexService, and both write the same serving-index-v3 directory. Either update the Javadoc to match production use, or confirm the two encodings never mix on one index.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexIncrementalPlan.java, line 150:
<comment>This overload is now public, and its Javadoc says it is only for test and diagnostic callers, but `StateArchiveServingIndexBuildCoordinatorV3.runFlush` calls it in production. Its legacy step-digest encoding also differs from the frame-digest `stepDigest` used by `ServingIndexService`, and both write the same `serving-index-v3` directory. Either update the Javadoc to match production use, or confirm the two encodings never mix on one index.</comment>
<file context>
@@ -144,7 +147,7 @@ static byte[] sourceSeed(long base, byte[] baseHash, List<String> participants)
* retains the legacy complete-diff encoding (test and diagnostic callers).
*/
- static ServingIndexIncrementalPlan planCommittedDiffs(long indexedThrough, byte[] headHash,
+ public static ServingIndexIncrementalPlan planCommittedDiffs(long indexedThrough, byte[] headHash,
List<BlockReverseDiff> committedDiffs) {
List<BlockReverseDiff> diffs = new ArrayList<>(Objects.requireNonNull(committedDiffs,
</file context>
| return StateArchiveTailV5.forTarget(first, target, digest, TailViews.lanes(lanes)); | ||
| } | ||
|
|
||
| static V5HistoryTail create( |
There was a problem hiding this comment.
P2: V5HistoryTail.create has no callers in chainbase or framework, so it is dead code. Remove it. It also takes raw block, hash and digest bytes without a CommonCheckpointTarget, bypassing the admission that forTarget enforces.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/history/format/V5HistoryTail.java, line 42:
<comment>`V5HistoryTail.create` has no callers in chainbase or framework, so it is dead code. Remove it. It also takes raw block, hash and digest bytes without a `CommonCheckpointTarget`, bypassing the admission that `forTarget` enforces.</comment>
<file context>
@@ -0,0 +1,80 @@
+ return StateArchiveTailV5.forTarget(first, target, digest, TailViews.lanes(lanes));
+ }
+
+ static V5HistoryTail create(
+ long first,
+ long block,
</file context>
|
|
||
| `RepositoryImpl.createHistoricalRoot` remains the public construction boundary. The package-private | ||
| state-source contract, current/historical adapters and property logging now live in | ||
| [repository/](repository.md). `RepositoryStateSources` provides the host read facade while |
There was a problem hiding this comment.
P3: The link text repository/ suggests a source directory, but this target is the repository.md guide, and the table above uses the same label for the source directory. Use a label that names the guide, such as repository guide.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/archive/execution.md, line 19:
<comment>The link text `repository/` suggests a source directory, but this target is the `repository.md` guide, and the table above uses the same label for the source directory. Use a label that names the guide, such as `repository guide`.</comment>
<file context>
@@ -16,7 +16,7 @@ Latest execution continues through its existing path.
`RepositoryImpl.createHistoricalRoot` remains the public construction boundary. The package-private
state-source contract, current/historical adapters and property logging now live in
-[repository/](repository/README.md). `RepositoryStateSources` provides the host read facade while
+[repository/](repository.md). `RepositoryStateSources` provides the host read facade while
preserving request-local caches, child-frame sharing and pinned-session ownership.
</file context>
Suggested change
| [repository/](repository.md). `RepositoryStateSources` provides the host read facade while | |
| [repository guide](repository.md). `RepositoryStateSources` provides the host read facade while |
| private final String message; | ||
| private final long timeMillis; | ||
|
|
||
| public ServingFailure(Reason reason, String message, long timeMillis) { |
There was a problem hiding this comment.
P3: This constructor does not need public: all new ServingFailure(...) calls are inside the serving package. Keep it package-private to match the class's "never leave the serving component" contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingFailure.java, line 50:
<comment>This constructor does not need `public`: all `new ServingFailure(...)` calls are inside the `serving` package. Keep it package-private to match the class's "never leave the serving component" contract.</comment>
<file context>
@@ -47,7 +47,7 @@ public enum Kind {
private final long timeMillis;
- ServingFailure(Reason reason, String message, long timeMillis) {
+ public ServingFailure(Reason reason, String message, long timeMillis) {
this.reason = Objects.requireNonNull(reason, "reason");
this.message = message == null ? "" : message;
</file context>
Suggested change
| public ServingFailure(Reason reason, String message, long timeMillis) { | |
| ServingFailure(Reason reason, String message, long timeMillis) { |
| |---|---| | ||
| | Shared block/value contracts | `BlockChangeView`, `BlockReverseDiff`, `OldValue`, `ArchiveStoreScope`: shared by Snapshot, Archive and PathState | | ||
| | Host lifecycle | `StateArchiveRuntimeOwner`: owns quiescence and release; `runtime/ArchiveRuntimeAttachment` carries borrowed collaborators | | ||
| | Canonical acquisition | `ArchiveStateBarrier`, `StateArchiveCanonicalBlockMetaSource`: bind Store acquisition to canonical state | |
There was a problem hiding this comment.
P3: This row lists classes that are not in the root package. Either drop the 'root' framing for this table or move the description to their owning packages, e.g. query/latest/ArchiveStateBarrier, contract/StateArchiveCanonicalBlockMetaSource.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/archive/storage.md, line 111:
<comment>This row lists classes that are not in the root package. Either drop the 'root' framing for this table or move the description to their owning packages, e.g. `query/latest/ArchiveStateBarrier`, `contract/StateArchiveCanonicalBlockMetaSource`.</comment>
<file context>
@@ -0,0 +1,181 @@
+|---|---|
+| Shared block/value contracts | `BlockChangeView`, `BlockReverseDiff`, `OldValue`, `ArchiveStoreScope`: shared by Snapshot, Archive and PathState |
+| Host lifecycle | `StateArchiveRuntimeOwner`: owns quiescence and release; `runtime/ArchiveRuntimeAttachment` carries borrowed collaborators |
+| Canonical acquisition | `ArchiveStateBarrier`, `StateArchiveCanonicalBlockMetaSource`: bind Store acquisition to canonical state |
+| Private progress-file bridge | `LatestStateGenerationCoordinatorFactory`: delegates registry assembly while retaining private progress-file access |
+| Shared failure | `ArchivePersistenceException`: used across storage and recovery boundaries |
</file context>
| import org.tron.core.db2.archive.BlockSnapshotMeta; | ||
| import org.tron.core.db2.archive.recovery.wal.ArchiveWalBinding; | ||
|
|
||
| /** Read-only base identity and startup validation, without Store or manifest write access. */ |
There was a problem hiding this comment.
P3: The class javadoc promises no store or manifest write access, but requireFixedPoint takes the full mutable ArchiveHistoryWriter. Either narrow the javadoc to describe what is actually guaranteed, or narrow the parameter type to a read-only view.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/history/store/ArchiveHistoryAdmission.java, line 10:
<comment>The class javadoc promises no store or manifest write access, but `requireFixedPoint` takes the full mutable `ArchiveHistoryWriter`. Either narrow the javadoc to describe what is actually guaranteed, or narrow the parameter type to a read-only view.</comment>
<file context>
@@ -0,0 +1,46 @@
+import org.tron.core.db2.archive.BlockSnapshotMeta;
+import org.tron.core.db2.archive.recovery.wal.ArchiveWalBinding;
+
+/** Read-only base identity and startup validation, without Store or manifest write access. */
+public final class ArchiveHistoryAdmission {
+
</file context>
| try { | ||
| return new StateArchiveAppendReadAdapterV3(index, source, point, target, false); | ||
| } catch (IOException | RuntimeException failure) { | ||
| point.close(); |
There was a problem hiding this comment.
P3: The failure path closes point without suppressing a close() exception, so an IOException from close() replaces the original failure. Wrap this in try/catch and call failure.addSuppressed(closeFailure), as the materializer's pinHistory already does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At chainbase/src/main/java/org/tron/core/db2/archive/serving/ServingIndexService.java, line 175:
<comment>The failure path closes `point` without suppressing a `close()` exception, so an IOException from `close()` replaces the original failure. Wrap this in try/catch and call `failure.addSuppressed(closeFailure)`, as the materializer's `pinHistory` already does.</comment>
<file context>
@@ -153,12 +159,24 @@ public void onCommonPublished(CommonCheckpointTarget target) {
+ try {
+ return new StateArchiveAppendReadAdapterV3(index, source, point, target, false);
+ } catch (IOException | RuntimeException failure) {
+ point.close();
+ throw failure;
+ }
</file context>
Suggested change
| point.close(); | |
| try { | |
| point.close(); | |
| } catch (IOException closeFailure) { | |
| failure.addSuppressed(closeFailure); | |
| } |
Group path state, checkpoint and serving responsibilities. Preserve package-owned storage and lifecycle boundaries, move source guides into tracked documentation, and record remaining directory tasks.
Extract serving record/key codecs and physical journal maintenance while preserving persistent bytes, private Store ownership and publication order. Include the preceding physical, metrics, P66, serving and test package moves. Organize source guides and record the ARM two-snapshot, single-runtime policy. Validate golden format vectors, archive regression and checkpoint/reorg paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Introduces the state archive feature: append-file state archive with catalog serving, geth-style archive index, checkpoint-bound recovery, path-state snapshot pipeline, historical query RPC/VM context wiring, and related performance and metrics attribution work. Rebuilt as a fully GPG-signed (Verified) history from feature/archive_block2; content is tree-identical to that branch.
Why are these changes required?
Provides bounded historical state storage and point-query serving for archive data, with recovery semantics aligned to checkpoints.
This PR has been tested by:
Follow up
Extra details
174 commits, all GPG signed. Branch is based on an older develop point and shows divergence from current develop; reconcile (merge/rebase) before merge if required.
Summary by cubic
Adds an opt-in state archive that persists bounded historical state and serves exact point-in-time and range queries, enabling historical RPC and VM execution without changing current chain processing.
HistoricalExecutionGuard, with fail-closed fork gates, deadline-bounded pairing checks, and TRC10 balance resolution at the queried state.rocksdbVersionbuild property for pinned validation candidates, isolated shared VM request configuration, and legacy tail cache retirement.Written for commit aafb0fe. Summary will update on new commits.