feat(toolkit): exclude historical balance DBs from lite snapshot#121
feat(toolkit): exclude historical balance DBs from lite snapshot#121halibobo1205 wants to merge 2 commits intodevelopfrom
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
codeant-ai
Bot
added
the
size:M
|
CodeAnt AI finished reviewing your PR. |
halibobo1205
force-pushed
the
feat/toolkit_db_lite_opt
branch
from
0f1467c to
eaba17c
Compare
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
|
CodeAnt AI Incremental review completed. |
|
@CodeAnt-AI: review |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis PR updates the DbLite toolkit so that lite snapshot history trimming also removes balance and account trace data for trimmed blocks, while merges from backup restore account trace history from the start height to keep historical balance lookups working. sequenceDiagram
participant Operator
participant DbLite toolkit
participant Block and transaction DBs
participant Balance trace DB
participant Account trace DB
Operator->>DbLite toolkit: Run lite split and trim history after snapshot
DbLite toolkit->>Block and transaction DBs: Identify and remove blocks beyond snapshot range
DbLite toolkit->>Balance trace DB: Load balance traces for trimmed blocks
DbLite toolkit->>Account trace DB: Delete account trace entries for affected addresses
DbLite toolkit->>Balance trace DB: Delete balance trace entries for trimmed blocks
Operator->>DbLite toolkit: Merge backup history into lite database
DbLite toolkit->>Block and transaction DBs: Copy archived block and transaction data
DbLite toolkit->>Account trace DB: Copy account trace entries from history start
Generated by CodeAnt AI |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
|
||
| @Test | ||
| public void testToolsWithTrimHistory() throws InterruptedException, IOException { | ||
| testTools("ROCKSDB", 1, true, true); |
There was a problem hiding this comment.
Suggestion: This test is named to validate trim-history behavior, but passing true for the third argument drives the merge-back path (snapshotMaxNum > historyMaxNum) instead of the trim path (trimExtraHistory). As written, it does not cover the intended scenario and can miss regressions in history trimming with balance lookup enabled. Pass false so the test actually exercises trim-history logic. [logic error]
Severity Level: Major ⚠️
- ⚠️ DbLite CLI history-trim with balance lookup untested.
- ⚠️ Lite snapshot rebuild regressions may bypass CI detection.| testTools("ROCKSDB", 1, true, true); | |
| testTools("ROCKSDB", 1, false, true); |
Steps of Reproduction ✅
1. Open `plugins/src/test/java/org/tron/plugins/rocksdb/DbLiteWithHistoryRocksDbTest.java`
and see `testToolsWithTrimHistory()` at lines 9–12 calling `testTools("ROCKSDB", 1, true,
true);` (Read output confirms this exact call).
2. Open `plugins/src/test/java/org/tron/plugins/DbLiteTest.java` and inspect the Javadoc
for the 4‑arg overload at lines 96–101 (Grep output): it states `advanceSnapshot=true`
makes `snapshotMaxNum > historyMaxNum` and "exercises mergeBak2Database", while
`advanceSnapshot=false` makes `historyMaxNum > snapshotMaxNum` and "exercises
trimExtraHistory".
3. In the same file, locate the 4‑arg method `testTools(String dbType, int
checkpointVersion, boolean advanceSnapshot, boolean historyBalanceLookup)` at line 102
(Grep) and its log statement at line 104; when
`DbLiteWithHistoryRocksDbTest.testToolsWithTrimHistory()` runs, it always invokes this
overload with `advanceSnapshot=true, historyBalanceLookup=true`, so the executed scenario
is the merge path, not the trimExtraHistory path described in the Javadoc.
4. Check other callers via Grep: `DbLiteRocksDbTest` and the LevelDB tests only call the
2‑arg `testTools(dbType, checkpointVersion)` (lines 90–93), which hard‑codes
`advanceSnapshot=false, historyBalanceLookup=false`; thus no test ever invokes `testTools`
with `advanceSnapshot=false` and `historyBalanceLookup=true`. Therefore, despite its name
(`testToolsWithTrimHistory`), the new RocksDB test never exercises the "trim history with
balance lookup enabled" scenario, and history‑trimming logic under
`historyBalanceLookup=true` remains untested.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** plugins/src/test/java/org/tron/plugins/rocksdb/DbLiteWithHistoryRocksDbTest.java
**Line:** 11:11
**Comment:**
*Logic Error: This test is named to validate trim-history behavior, but passing `true` for the third argument drives the merge-back path (`snapshotMaxNum > historyMaxNum`) instead of the trim path (`trimExtraHistory`). As written, it does not cover the intended scenario and can miss regressions in history trimming with balance lookup enabled. Pass `false` so the test actually exercises trim-history logic.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Valid point. advanceSnapshot=true exercises the merge path, not trim — the test name is misleading. Changed to testTools("ROCKSDB", 1, false, true) so it actually covers the v1 trim + historyBalance scenario. Fixed in the next commit.
| @@ -137,11 +152,13 @@ public void testTools(String dbType, int checkpointVersion) | |||
| String.format("rename snapshot to %s failed", | |||
| Paths.get(dbPath, databaseDir))); | |||
| } | |||
| // start and validate the snapshot | |||
| startApp(); | |||
| generateSomeTransactions(checkpointVersion == 1 ? 18 : 6); | |||
| // stop the node | |||
| shutdown(); | |||
| if (advanceSnapshot) { | |||
| // start and validate the snapshot, producing blocks beyond history | |||
| startApp(); | |||
| generateSomeTransactions(checkpointVersion == 1 ? 18 : 6); | |||
There was a problem hiding this comment.
Suggestion: The block-generation durations for advanceSnapshot=true are inverted for checkpoint version 2, which makes history grow more than snapshot and violates the method's own contract (snapshotMaxNum > historyMaxNum). This causes the test to exercise the wrong merge path and can miss regressions in the intended branch. Use the same "history shorter, snapshot longer" timing strategy regardless of checkpoint version. [logic error]
Severity Level: Major ⚠️
- ⚠️ RocksDB checkpoint v2 mergeBak2Database path never exercised.
- ⚠️ trimExtraHistory branch redundantly tested for checkpoint version two.
- ⚠️ Checkpoint v2 merge regressions may pass unnoticed in tests.| generateSomeTransactions(advanceSnapshot ? 6 : 18); | |
| generateSomeTransactions(18); |
Steps of Reproduction ✅
1. Run the unit test `testToolsWithRocksDB` in
`plugins/src/test/java/org/tron/plugins/rocksdb/DbLiteRocksDbV2Test.java:9-12`, which
calls `testTools("ROCKSDB", 2, true, false)` on `DbLiteTest`.
2. Inside `DbLiteTest.testTools`
(`plugins/src/test/java/org/tron/plugins/DbLiteTest.java:102-169`), after the initial
18-second block generation and snapshot split, the line at 136 executes
`generateSomeTransactions(advanceSnapshot ? (checkpointVersion == 1 ? 6 : 18) : 18);`
which, for `advanceSnapshot=true` and `checkpointVersion=2`, generates 18 seconds of
additional blocks before the history split (making `historyMaxNum` significantly larger).
3. Later in the same method, after renaming `snapshot` to the live database directory, the
`if (advanceSnapshot)` block at lines 36–41 (diff line 158 in the hunk) runs
`generateSomeTransactions(checkpointVersion == 1 ? 18 : 6);`, which for
`checkpointVersion=2` generates only 6 seconds of blocks on the snapshot, so at merge time
`snapshotMaxNum < historyMaxNum` contrary to the method's Javadoc contract for
`advanceSnapshot=true`.
4. When `cli.execute(argsForMerge)` is called in `DbLiteTest.testTools`
(`plugins/src/test/java/common/org/tron/plugins/DbLite.java:460-519`),
`checkAndGetBlockNumInfo` computes a `BlockNumInfo` with `snapshotMaxNum < historyMaxNum`,
causing `trimExtraHistory` (`DbLite.java:55-52, lines 520-599`) to execute and
`mergeBak2Database` (`DbLite.java:54-64, lines 573-599`) to early-return with "Ignore
merging the bak data"; thus `DbLiteRocksDbV2Test` unintentionally exercises the trim path
again instead of the intended `mergeBak2Database` path for checkpoint version 2.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** plugins/src/test/java/org/tron/plugins/DbLiteTest.java
**Line:** 136:158
**Comment:**
*Logic Error: The block-generation durations for `advanceSnapshot=true` are inverted for checkpoint version 2, which makes history grow more than snapshot and violates the method's own contract (`snapshotMaxNum > historyMaxNum`). This causes the test to exercise the wrong merge path and can miss regressions in the intended branch. Use the same "history shorter, snapshot longer" timing strategy regardless of checkpoint version.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
This is a pre-existing behavior, not introduced by this PR. The original code already had checkpointVersion == 1 ? 6 : 18 / checkpointVersion == 1 ? 18 : 6 timing. This PR only wrapped it in the advanceSnapshot conditional without changing the logic. Can be addressed separately if needed.
| DBInterface balanceTraceDb = DbTool.getDB(liteDir, BALANCE_TRACE_DB_NAME); | ||
| DBInterface accountTraceDb = DbTool.getDB(liteDir, ACCOUNT_TRACE_DB_NAME); | ||
|
|
||
| ProgressBar.wrap(LongStream.rangeClosed(start, end) | ||
| .boxed() | ||
| .sorted((a, b) -> Long.compare(b, a)), "trimHistory").forEach(n -> { | ||
| .sorted((a, b) -> Long.compare(b, a)), "trimHistory") | ||
| .map(ByteArray::fromLong).forEach(n -> { | ||
| try { | ||
| byte[] blockIdHash = blockIndexDb.get(ByteArray.fromLong(n)); | ||
| byte[] blockIdHash = blockIndexDb.get(n); | ||
| Protocol.Block block = Protocol.Block.parseFrom(blockDb.get(blockIdHash)); | ||
| // delete transactions | ||
| for (Protocol.Transaction e : block.getTransactionsList()) { | ||
| transDb.delete(DBUtils.getTransactionId(e).getBytes()); |
There was a problem hiding this comment.
Suggestion: During history trimming, transaction IDs are removed from trans and transactionRetStore, but transactionHistoryStore entries for the same trimmed blocks are left behind. This leaves stale transaction info queryable by transaction ID after the corresponding blocks are deleted, so delete transaction-history entries for each removed block transaction as well. [logic error]
Severity Level: Major ⚠️
- ❌ getTransactionInfoById returns info for trimmed-block transactions.
- ⚠️ Lite node exposes inconsistent transaction and block history.
- ⚠️ History-based tooling may misinterpret node's canonical chain.| DBInterface balanceTraceDb = DbTool.getDB(liteDir, BALANCE_TRACE_DB_NAME); | |
| DBInterface accountTraceDb = DbTool.getDB(liteDir, ACCOUNT_TRACE_DB_NAME); | |
| ProgressBar.wrap(LongStream.rangeClosed(start, end) | |
| .boxed() | |
| .sorted((a, b) -> Long.compare(b, a)), "trimHistory").forEach(n -> { | |
| .sorted((a, b) -> Long.compare(b, a)), "trimHistory") | |
| .map(ByteArray::fromLong).forEach(n -> { | |
| try { | |
| byte[] blockIdHash = blockIndexDb.get(ByteArray.fromLong(n)); | |
| byte[] blockIdHash = blockIndexDb.get(n); | |
| Protocol.Block block = Protocol.Block.parseFrom(blockDb.get(blockIdHash)); | |
| // delete transactions | |
| for (Protocol.Transaction e : block.getTransactionsList()) { | |
| transDb.delete(DBUtils.getTransactionId(e).getBytes()); | |
| DBInterface transactionHistoryDb = DbTool.getDB(liteDir, TRANSACTION_HISTORY_DB_NAME); | |
| DBInterface balanceTraceDb = DbTool.getDB(liteDir, BALANCE_TRACE_DB_NAME); | |
| DBInterface accountTraceDb = DbTool.getDB(liteDir, ACCOUNT_TRACE_DB_NAME); | |
| ProgressBar.wrap(LongStream.rangeClosed(start, end) | |
| .boxed() | |
| .sorted((a, b) -> Long.compare(b, a)), "trimHistory") | |
| .map(ByteArray::fromLong).forEach(n -> { | |
| try { | |
| byte[] blockIdHash = blockIndexDb.get(n); | |
| Protocol.Block block = Protocol.Block.parseFrom(blockDb.get(blockIdHash)); | |
| // delete transactions | |
| for (Protocol.Transaction e : block.getTransactionsList()) { | |
| byte[] txId = DBUtils.getTransactionId(e).getBytes(); | |
| transDb.delete(txId); | |
| transactionHistoryDb.delete(txId); |
Steps of Reproduction ✅
1. Enable transaction history persistence so `TransactionHistoryStore` is used (its `put`
method checks `CommonParameter.getInstance().getStorage().getTransactionHistorySwitch()`
in `chainbase/src/main/java/org/tron/core/store/TransactionHistoryStore.java:27-32`). Run
a full node so that transactions and their history are written into
`transactionHistoryStore`.
2. Use the DbLite toolkit to create a snapshot and then, after producing additional
blocks, create a history dataset so that `historyMaxNum > snapshotMaxNum`. This pattern is
exercised by `testTools(..., advanceSnapshot=false, historyBalanceLookup=false)` in
`plugins/src/test/java/org/tron/plugins/DbLiteTest.java:23-60`, which generates blocks,
creates a snapshot, generates more blocks, then creates history.
3. Merge history into the lite database by invoking the DbLite CLI with `-o merge` (wired
to `DbLite.call()` at `plugins/src/main/java/common/org/tron/plugins/DbLite.java:119-149`,
which dispatches to `completeHistoryData()` at `DbLite.java:225-260`). In
`completeHistoryData`, archive DBs including `"transactionHistoryStore"` are backed up and
then copied from `historyDir` into `liteDir` (`archiveDbs` list at `DbLite.java:65-72`,
`backupArchiveDbs` at `DbLite.java:497-505`, `copyHistory2Database` at
`DbLite.java:508-512`).
4. When `historyMaxNum > snapshotMaxNum`, `completeHistoryData` calls
`trimExtraHistory(liteDir, blockNumInfo)` at `DbLite.java:244`. Inside `trimExtraHistory`
(`DbLite.java:514-571`), for each block in `[start, end]` it deletes transactions from
`transDb` and per-block results from `TRANSACTION_RET_DB_NAME` via `tranRetDb.delete(n)`
at `DbLite.java:541-546` and removes the corresponding blocks and indices, but it never
touches `TRANSACTION_HISTORY_DB_NAME`. After this, `mergeBak2Database`
(`DbLite.java:573-609`) merges backed-up DBs; for `TRANSACTION_HISTORY_DB_NAME` it copies
all entries from the backup without filtering by block (see `archiveDbs.stream()` and
conditional seek at `DbLite.java:592-603`). As a result, `transactionHistoryStore` retains
entries for transactions whose blocks were trimmed. A client calling
`Wallet.getTransactionInfoById(transactionId)` in
`framework/src/main/java/org/tron/core/Wallet.java:1888-1894` will find no result in
`TransactionRetStore` (entry deleted) but a non-null `TransactionInfoCapsule` from
`TransactionHistoryStore.get(transactionId.toByteArray())`, even though the corresponding
block and trans mapping were removed, leaving transaction-by-id queries returning
information for blocks that no longer exist in the trimmed lite history.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** plugins/src/main/java/common/org/tron/plugins/DbLite.java
**Line:** 531:543
**Comment:**
*Logic Error: During history trimming, transaction IDs are removed from `trans` and `transactionRetStore`, but `transactionHistoryStore` entries for the same trimmed blocks are left behind. This leaves stale transaction info queryable by transaction ID after the corresponding blocks are deleted, so delete transaction-history entries for each removed block transaction as well.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Not applicable. transactionHistoryStore is deprecated and no longer written to, so there is no stale data to clean up.
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
@codex: review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaba17c211
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (TRANS_DB_NAME.equals(dbName) || TRANSACTION_HISTORY_DB_NAME.equals(dbName) | ||
| || ACCOUNT_TRACE_DB_NAME.equals(dbName)) { | ||
| iterator.seekToFirst(); |
There was a problem hiding this comment.
mergeBak2Database computes head = historyMaxNum + 1 to restore only backup data newer than the imported history, but this branch forces account-trace to seekToFirst() and copies the entire DB. When snapshotMaxNum > historyMaxNum (the merge-back path) and history balance lookup is enabled, this reintroduces snapshot-side account-trace entries for overlapping heights, so historical balance queries can read forked/stale values for affected addresses. Account-trace rows need height filtering (e.g., decode the block number suffix and skip <= historyMaxNum) instead of unconditional full replay.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is not an issue. The merge path runs when snapshotMaxNum > historyMaxNum. The backup (snapshot) and liteDir (history) are from the same chain, so for blocks <= historyMaxNum, account-trace entries are identical — put is idempotent. For blocks > historyMaxNum, only the snapshot has data, which is correctly merged back. There is no fork scenario in normal DbLite usage.
halibobo1205
force-pushed
the
feat/toolkit_db_lite_opt
branch
from
eaba17c to
3885da2
Compare
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:M
|
CodeAnt AI Incremental review completed. |
…#6631) * feat(protocol): add protoLint script for enum validation * feat(protocol): optimize protoLint performance and caching * fix(proto): resolve gradle implicit task dependency warning * docs(protocol): clarify enum discriminator in protoLint * build(protocol): harden proto lint buf config * docs: update comment to reflect dynamic include path derivation
halibobo1205
force-pushed
the
feat/toolkit_db_lite_opt
branch
from
3885da2 to
a03adc7
Compare
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:L
|
CodeAnt AI Incremental review completed. |
What Changed Exclude balance-trace and account-trace from lite snapshots. Since historical balance lookup is off by default, most nodes see no difference in snapshot size; only nodes with the feature enabled benefit from smaller snapshots. Impact ✅ Smaller lite snapshots when historical balance lookup is enabled ✅ No impact for default-configured nodes close tronprotocol#6597
halibobo1205
force-pushed
the
feat/toolkit_db_lite_opt
branch
from
a03adc7 to
4f4f8dd
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f4f8dd10e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| DBInterface balanceTraceDb = DbTool.getDB(liteDir, BALANCE_TRACE_DB_NAME); | ||
| DBInterface accountTraceDb = DbTool.getDB(liteDir, ACCOUNT_TRACE_DB_NAME); |
There was a problem hiding this comment.
When merging with a history dataset that does not contain balance-trace/account-trace (e.g., datasets generated before this change), trimExtraHistory opens these DBs via DbTool.getDB(liteDir, ...), which defaults to LevelDB if engine.properties is absent. In a RocksDB node this creates new trace DB directories tagged as LEVELDB, and subsequent node startup fails engine validation (Cannot open LEVELDB database with ROCKSDB engine). This is a regression introduced by eagerly opening these optional stores without inheriting the existing node engine.
Useful? React with 👍 / 👎.
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:M
Sequence DiagramThis PR updates the lite snapshot tooling so that trimming history also removes matching balance and account trace records, and merging history back into a lite database always restores full account trace history to keep historical balance lookups working. sequenceDiagram
participant Operator
participant DbLite
participant BlockStore
participant BalanceTraceStore
participant AccountTraceStore
participant HistoryBackup
Operator->>DbLite: Run trim history on lite database
DbLite->>BlockStore: Scan blocks selected for trimming
DbLite->>BalanceTraceStore: Load balance trace for each trimmed block
DbLite->>AccountTraceStore: Delete account trace entries for affected addresses
DbLite->>BalanceTraceStore: Delete balance trace entries for trimmed blocks
DbLite->>BlockStore: Delete block and transaction data for trimmed blocks
Operator->>DbLite: Run merge history into lite database
DbLite->>HistoryBackup: Read account trace history from start
DbLite->>AccountTraceStore: Write full account trace history into lite database
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:M
Sequence DiagramThis PR updates the lite toolkit so snapshots exclude balance and account trace databases while history splits still include them, and merges now trim extra blocks along with their trace data to keep historical balance lookups reliable. New tests cover the snapshot, history, and merge flow when historical balance lookup is enabled. sequenceDiagram
participant Admin
participant DbLiteToolkit as DbLite toolkit
participant NodeDB as Node database
participant SnapshotStore as Snapshot store
participant HistoryStore as History store
Admin->>NodeDB: Run full node with historical balance lookup and produce blocks
Admin->>DbLiteToolkit: Split snapshot from node database
DbLiteToolkit->>SnapshotStore: Store snapshot without balance and account trace dbs
Admin->>DbLiteToolkit: Split history from node database
DbLiteToolkit->>HistoryStore: Store history with archive dbs including balance and account traces
Admin->>DbLiteToolkit: Merge history into lite node database built from snapshot
DbLiteToolkit->>NodeDB: Copy history, trim extra blocks and related balance and account traces, merge recent data
NodeDB-->>Admin: Lite node serves historical balance lookups with smaller snapshot
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:M
Sequence DiagramThis diagram shows how the DbLite tool now excludes historical balance databases from lite snapshots while keeping them in history, and how merging history into a lite node trims matching balance and account trace records for old blocks. sequenceDiagram
participant Operator
participant DbLite
participant Snapshot
participant History
participant LiteDB
Operator->>DbLite: Split snapshot from full database
DbLite->>Snapshot: Copy non archive tables excluding balance and account trace from full DB
Operator->>DbLite: Split history from full database
DbLite->>History: Copy archive tables including balance and account trace from full DB
Operator->>DbLite: Merge history into lite database
DbLite->>LiteDB: Trim old blocks, transactions, balance and account trace entries beyond snapshot range
DbLite->>LiteDB: Merge remaining history tables including account trace from history dataset
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:M
Sequence DiagramThis PR updates the lite history trimming flow so that when old blocks are removed, their associated balance and account trace records are also deleted, keeping historical balance data consistent and snapshots smaller. sequenceDiagram
participant Operator
participant DbLite
participant BlockStore as Block and transaction store
participant BalanceTrace as Balance trace store
participant AccountTrace as Account trace store
Operator->>DbLite: Run trim history
DbLite->>BlockStore: Find blocks beyond snapshot range
loop Each old block
DbLite->>BlockStore: Delete block and transaction data
DbLite->>BalanceTrace: Fetch balance trace for block
DbLite->>AccountTrace: Delete account traces for addresses in trace
DbLite->>BalanceTrace: Delete balance trace entry
end
Generated by CodeAnt AI |
| TRANSACTION_HISTORY_DB_NAME, | ||
| BALANCE_TRACE_DB_NAME, | ||
| ACCOUNT_TRACE_DB_NAME); |
There was a problem hiding this comment.
Suggestion: The trace databases are added to archiveDbs unconditionally, but getSnapshotDbs() excludes everything in archiveDbs, so balance-trace and account-trace are always removed from snapshots even when historical balance lookup is enabled. Make inclusion/exclusion of these two DBs conditional on the history-balance-lookup setting so snapshots created for that mode keep required trace data. [logic error]
Severity Level: Critical 🚨
- ❌ Lite snapshots lack trace DBs for history lookups.
- ⚠️ Historical balance lookup unusable on snapshot-only lite nodes.
- ⚠️ Operators must ship history data to preserve lookups.Steps of Reproduction ✅
1. In `plugins/src/test/java/org/tron/plugins/DbLiteTest.java:16-35`, call
`testTools(dbType, checkpointVersion, false, true)` so that `historyBalanceLookup` is
`true` (line 34 sets `Args.getInstance().setHistoryBalanceLookup(historyBalanceLookup)`),
then `startApp()` boots a full node that writes balance and account traces via
`AccountStore` and `BalanceTraceStore`
(`chainbase/src/main/java/org/tron/core/store/AccountStore.java:45-63`,
`BalanceTraceStore.java:53-71`).
2. After shutting down the node in `DbLiteTest.shutdown()` (lines 64-69) and deleting
`trans-cache` (line 43), execute the snapshot split via `cli.execute(argsForSnapshot)`
(DbLiteTest.java:45), which runs `DbLite.generateSnapshot(...)` (`DbLite.java:119-53`).
3. Inside `generateSnapshot`, `getSnapshotDbs(sourceDir)` is called at `DbLite.java:165`;
`getSnapshotDbs` (`DbLite.java:43-50`) enumerates all subdirectories under `sourceDir` and
filters with `.filter(dir -> !archiveDbs.contains(dir.getName()))`, where `archiveDbs` at
`DbLite.java:26-33` includes `BALANCE_TRACE_DB_NAME` and `ACCOUNT_TRACE_DB_NAME` (lines
62-72). This causes the `balance-trace` and `account-trace` directories present in the
fullnode data path to be excluded from `snapshotDbs`.
4. `split(sourceDir, snapshotDir, snapshotDbs)` (`DbLite.java:62-77`) copies only the
filtered DB list into the `snapshot` directory, and no later method copies balance or
account traces into the snapshot (see `fillSnapshotBlockAndTransDb` at
`DbLite.java:170-212`, which only creates `block`, `block-index`, and `trans`). As a
result, a lite node started on the snapshot-only database with `historyBalanceLookup`
still enabled has no `balance-trace` or `account-trace` stores, so historical balance
lookup features (which rely on `BalanceTraceStore` and `AccountTraceStore` in
`chainbase/src/main/java/org/tron/core/store`) cannot function from snapshot data alone.
The exclusion is unconditional; DbLite never reads the history-balance-lookup setting.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** plugins/src/main/java/common/org/tron/plugins/DbLite.java
**Line:** 70:72
**Comment:**
*Logic Error: The trace databases are added to `archiveDbs` unconditionally, but `getSnapshotDbs()` excludes everything in `archiveDbs`, so `balance-trace` and `account-trace` are always removed from snapshots even when historical balance lookup is enabled. Make inclusion/exclusion of these two DBs conditional on the history-balance-lookup setting so snapshots created for that mode keep required trace data.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
2 participants
User description
What does this PR do?
Why are these changes required?
This PR has been tested by:
Follow up
Extra details
CodeAnt-AI Description
Keep lite snapshots smaller without breaking historical balance lookups
What Changed
Impact
✅ Smaller lite snapshots for default setups✅ Reliable historical balance lookups after trimming✅ Fewer broken balance queries after restoring a lite database🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.