fix(consensus): use Locale.ROOT for case-insensitive#127
fix(consensus): use Locale.ROOT for case-insensitive#127halibobo1205 wants to merge 4 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 · |
codeant-ai
Bot
added
the
size:L
halibobo1205
force-pushed
the
fix/locale-sensitive-case-folding
branch
from
1f83a8f to
05bfab2
Compare
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
![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: 1f83a8fd7d
ℹ️ 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".
|
CodeAnt AI finished reviewing your PR. |
halibobo1205
force-pushed
the
fix/locale-sensitive-case-folding
branch
8 times, most recently
from
2179fbb to
bedec50
Compare
|
@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 · |
|
@codex: review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
codeant-ai
Bot
added
size:XL
Sequence DiagramThis PR changes account ID index key generation to use locale-independent lowercase and adds a Turkish-like locale fallback so nodes both avoid future consensus splits and can still read and reject duplicates for legacy data. sequenceDiagram
participant Node
participant IndexStore
participant Database
Node->>IndexStore: Store account by accountId
IndexStore->>IndexStore: Compute newKey = accountId lowercased with ROOT locale
IndexStore->>Database: Save newKey mapped to account address
Node->>IndexStore: Lookup or check duplicate by accountId
IndexStore->>IndexStore: Compute newKey = accountId lowercased with ROOT locale
IndexStore->>Database: Read by newKey
alt No value and system locale is Turkish like
IndexStore->>IndexStore: Compute legacyKey using Turkish locale rules
IndexStore->>Database: Read by legacyKey
end
IndexStore-->>Node: Return address if found so Node can read or reject duplicate
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 · |
halibobo1205
force-pushed
the
fix/locale-sensitive-case-folding
branch
from
bedec50 to
3dbe19e
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
halibobo1205
force-pushed
the
fix/locale-sensitive-case-folding
branch
from
3dbe19e to
bf2c75c
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex: review |
String.toLowerCase()/toUpperCase() without an explicit Locale uses Locale.getDefault(), which on Turkish (tr) or Azerbaijani (az) systems folds 'I' to dotless-ı (U+0131) instead of 'i' (U+0069). This caused different index keys on tr/az nodes → potential consensus split. Changes: - Fix all toLowerCase()/toUpperCase() calls to use Locale.ROOT - Enable the ErrorProne StringCaseLocaleUsage checker at ERROR level to prevent future regressions at compile time - Add dual Turkish legacy fallback in AccountIdIndexStore for backward compatibility with keys written under tr/az locale: 1. Direct probe: toLowerCase(TURKISH) for same-case queries 2. Normalized probe: replace 'i' with 'ı' for cross-case queries - Add one-time data migration (MigrateTurkishKeyHelper) to normalize all Turkish legacy keys (ı → i) at startup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: codeant-ai[bot] <151821869+codeant-ai[bot]@users.noreply.github.com> Co-Authored-By: codex <codex@openai.com>
halibobo1205
force-pushed
the
fix/locale-sensitive-case-folding
branch
from
e00d67d to
979beae
Compare
|
@codex: review |
|
@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 adds a startup migration that normalizes legacy Turkish account ID index keys to a locale‑independent form and ensures all future account ID lookups use Locale.ROOT, preventing consensus splits across nodes with different default locales. sequenceDiagram
participant Node
participant Manager
participant DynamicPropertiesStore
participant MigrateTurkishKeyHelper
participant AccountIdIndexStore
Node->>Manager: Start node (init)
Manager->>DynamicPropertiesStore: getTurkishKeyMigrationDone
alt migration not done
Manager->>MigrateTurkishKeyHelper: doWork
MigrateTurkishKeyHelper->>AccountIdIndexStore: Scan keys for Turkish dotless i
MigrateTurkishKeyHelper->>AccountIdIndexStore: Rewrite keys with standard i and delete old keys
MigrateTurkishKeyHelper->>DynamicPropertiesStore: saveTurkishKeyMigrationDone(1)
end
Node->>AccountIdIndexStore: Put or query account by accountId
AccountIdIndexStore->>AccountIdIndexStore: Normalize accountId with Locale.ROOT and read/write index entry
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 · |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 979beae895
ℹ️ 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".
The dual Turkish fallback (direct + normalized probes) in AccountIdIndexStore is no longer needed because: 1. MigrateTurkishKeyHelper normalizes all legacy keys at startup before any queries are served 2. New writes always use Locale.ROOT, so no new Turkish keys are created 3. If migration is interrupted, the DynamicPropertiesStore flag ensures it reruns on next startup This simplifies get()/has() to single-lookup operations and removes ~100 lines of fallback logic (DOTLESS_I, TURKISH locale, direct/ normalized key methods, lookupWithFallback). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace remaining @SuppressWarnings("StringCaseLocaleUsage") with explicit Locale.ROOT in 6 test files. The suppress annotations were hiding real locale-sensitivity bugs: - BytecodeCompiler: toUpperCase() on opcode names like "jumpi" would produce "JUMPİ" on Turkish locale, causing opcode lookup failure - ShieldedReceiveTest: toUpperCase() on ztron address containing 'i' would produce 'İ' instead of 'I', breaking address decoding - CronExpressionTest, OperationsTest, DataWord, SM2KeyTest: hex formatting with toUpperCase() (low risk but inconsistent) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
halibobo1205
force-pushed
the
fix/locale-sensitive-case-folding
branch
from
979beae to
aa5489a
Compare
|
@codex: review |
|
@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 adds a one-time migration that normalizes Turkish legacy account ID index keys and updates all case handling to use Locale.ROOT so that account ID lookups behave consistently across nodes and prevent consensus splits. sequenceDiagram
participant Node
participant Manager
participant DynamicPropertiesStore
participant TurkishKeyMigration
participant AccountIdIndexStore
participant Client
Node->>Manager: Startup init
Manager->>DynamicPropertiesStore: Check Turkish key migration flag
alt Migration not done
Manager->>TurkishKeyMigration: Run Turkish key migration
TurkishKeyMigration->>AccountIdIndexStore: Scan and rewrite Turkish keys to ROOT form
TurkishKeyMigration->>DynamicPropertiesStore: Mark migration done
end
Client->>AccountIdIndexStore: Put or query accountId
AccountIdIndexStore->>AccountIdIndexStore: Normalize accountId with Locale.ROOT
AccountIdIndexStore-->>Client: Store or lookup result using normalized key
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 · |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa5489a55b
ℹ️ 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".
Simulate Turkish locale writes for all 13 mainnet accountId keys (excluding the empty key), verify ROOT-based queries miss for keys containing uppercase 'I', run MigrateTurkishKeyHelper, then verify all queries (original case, lowercase, uppercase) succeed. Keys with uppercase 'I' (BitTorrent, InfStonesSSRWallet, ISSRWallet, JustDoIt, RtytIturtet) produce different Turkish keys (I→ı) and require migration to restore ROOT-based lookups. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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
| // Original accountIds as they might have been submitted (mixed case) | ||
| // The lowercase versions match the 14 mainnet keys observed in production | ||
| String[] mainnetAccountIds = { | ||
| "12345678", // no letters |
There was a problem hiding this comment.
Suggestion: The new "mainnet 14 keys" migration test does not actually include the empty-string key ("") that is explicitly listed as part of the production dataset, so this test misses one real key path and can allow regressions on that key to slip through. Add the missing empty accountId entry so the test truly validates all advertised mainnet keys. [logic error]
Severity Level: Major ⚠️
- ⚠️ Turkish migration untested for mainnet empty accountId key.
- ⚠️ Consensus behavior for empty accountId may regress unnoticed.
- ⚠️ Scenario #11 comment and assertions mismatch actual data set.| "12345678", // no letters | |
| "", // empty key observed in production |
Steps of Reproduction ✅
1. Open `framework/src/test/java/org/tron/core/db/AccountIdIndexStoreTest.java` and locate
the Javadoc for Scenario #11 above the `testMainnetKeysMigration` method (around lines
378–385 in the PR hunk). The comment enumerates the mainnet keys as: `\"\", \"12345678\",
\"543838383\", \"bittorrent\", \"converse\", \"helloworld\", \"infstonessrwallet\",
\"issrwallet\", \"justdoit\", \"justinsun\", \"justinsuntron\", \"rtytiturtet\",
\"tronbetfestival\", \"vena_family\"`, explicitly including the empty-string key `""`.
2. In the same file, inspect the `testMainnetKeysMigration` method body and find the
`mainnetAccountIds` array declaration at lines 402–416: `String[] mainnetAccountIds = {
"12345678", "543838383", "BitTorrent", ..., "vena_family" };`. Observe that this array
does not contain the empty-string entry `""` that is listed in the Javadoc comment; the
first element is `"12345678"`.
3. Still within `testMainnetKeysMigration`, follow the logic using this array: for each
`mainnetAccountIds[i]`, the test (a) generates an address and a Turkish-cased key, (b)
writes it via `accountIdIndexStore.put(...)`, (c) performs pre-migration lookups, then (d)
runs `new MigrateTurkishKeyHelper(chainBaseManager).doWork();` and finally (e) asserts
post-migration lookups using `accountIdIndexStore.get(...)` and
`accountIdIndexStore.has(...)` (see lines ~421–469). Because `mainnetAccountIds` omits
`""`, the migration path and lookups are never exercised for the empty-string key.
4. Run the test `testMainnetKeysMigration` (e.g., `mvn test
-Dtest=AccountIdIndexStoreTest#testMainnetKeysMigration`). It passes, demonstrating that
the scenario for the empty mainnet accountId key described in the comment is not actually
validated by this test. Any future regression in how `AccountIdIndexStore` /
`MigrateTurkishKeyHelper` handle the empty accountId would not be caught by this "mainnet
keys" migration test unless the missing `""` entry is added to `mainnetAccountIds`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** framework/src/test/java/org/tron/core/db/AccountIdIndexStoreTest.java
**Line:** 403:403
**Comment:**
*Logic Error: The new "mainnet 14 keys" migration test does not actually include the empty-string key (`""`) that is explicitly listed as part of the production dataset, so this test misses one real key path and can allow regressions on that key to slip through. Add the missing empty accountId entry so the test truly validates all advertised mainnet keys.
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 Incremental review completed. |
|
close #128 |
1 participant
User description
String.toLowerCase()/toUpperCase() without an explicit Locale uses Locale.getDefault(), which on Turkish (tr) or Azerbaijani (az) systems folds 'I' to dotless-ı (U+0131) instead of 'i' (U+0069). This caused AccountIdIndexStore to generate different index keys on tr/az nodes, enabling a consensus split where tr/az nodes accept duplicate accountId transactions that other nodes reject.
Fix AccountIdIndexStore.getLowerCaseAccountId to use Locale.ROOT, and add a legacy fallback in get()/has() so that tr/az nodes can still read old data stored with the locale-sensitive key.
Also fix all other toLowerCase()/toUpperCase() calls across the codebase and enable the built-in ErrorProne StringCaseLocaleUsage checker at ERROR level to prevent future regressions at compile time.
What does this PR do?
Why are these changes required?
This PR has been tested by:
Follow up
Extra details
CodeAnt-AI Description
Use locale-independent case handling across nodes and migrate Turkish legacy account IDs
What Changed
Impact
✅ Fewer consensus splits on Turkish and Azerbaijani nodes✅ Existing account IDs stay readable after upgrade✅ Consistent API blocking and command handling across locales💡 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.