feat(toolkit): exclude historical balance DBs from lite snapshot#121
Open
halibobo1205 wants to merge 9 commits intodevelopfrom
Open
feat(toolkit): exclude historical balance DBs from lite snapshot#121halibobo1205 wants to merge 9 commits intodevelopfrom
halibobo1205 wants to merge 9 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. |
fix(docs): contributing.md
opt(common): GenesisBlock timestamp valid message error
**Exclude historical balance data from lite snapshots** ### What Changed - Lite history cleanup now removes balance trace data and the related account trace entries for trimmed blocks. - When rebuilding a lite database, account trace data is copied from the start, so historical balance lookups still work after the split. - Snapshot tests now enable historical balance lookup to cover this behavior. - Added `.claude` to `.gitignore`. ### Impact `✅ Smaller lite snapshots` `✅ Complete balance history after trimming` `✅ Fewer missing trace records in rebuilt databases`
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. |
Owner
Author
|
@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.
Suggested change
| 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.
Comment on lines
136
to
+158
| @@ -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.
Suggested change
| 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.
Comment on lines
+531
to
543
| 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.
Suggested change
| 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.
|
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 · |
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
5 participants
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.
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 historical balance lookups working in lite snapshots
What Changed
Impact
✅ Complete balance history after lite snapshot trimming✅ Fewer missing trace records after rebuilds✅ Clearer invalid timestamp errors💡 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.