Skip to content

Conversation

@coodos
Copy link
Contributor

@coodos coodos commented Dec 15, 2025

Description of change

add auto reload logic

Issue Number

Type of change

  • Update (a change which updates existing functionality)

How the change has been tested

Change checklist

  • I have ensured that the CI Checks pass locally
  • I have removed any unnecessary logic
  • My code is well documented
  • I have signed my commits
  • My code follows the pattern of the application
  • I have self reviewed my code

Summary by CodeRabbit

  • New Features

    • Ledger entries now include transaction and previous hashes; these are shown in transaction details with copy-to-clipboard.
    • Admins gain a Burn action in the currency UI to decrease supply (with server-side validation and feedback).
  • Improvements

    • Post-account-add now refreshes balances, history, and account details to keep data consistent.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

Warning

Rate limit exceeded

@coodos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 32 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between c4de373 and 7cdf49d.

📒 Files selected for processing (4)
  • platforms/eCurrency-api/src/database/migrations/1765809758435-migration.ts (1 hunks)
  • platforms/eCurrency-api/src/services/LedgerService.ts (4 hunks)
  • platforms/eCurrency/client/src/components/currency/transaction-card.tsx (3 hunks)
  • platforms/eCurrency/client/src/pages/currency-detail.tsx (6 hunks)

Walkthrough

Adds SHA-256 linking to ledger entries (hash & prevHash), exposes hashes in controller responses, introduces a new POST /api/ledger/burn endpoint and burn flow in LedgerService, and updates client UI to display/copy hashes and support burning (modal, button, cache invalidation).

Changes

Cohort / File(s) Summary
Ledger schema & persistence
platforms/eCurrency-api/src/database/entities/Ledger.ts, platforms/eCurrency-api/src/services/LedgerService.ts
Added nullable hash and prevHash columns to Ledger entity. Implemented SHA-256 computeHash() and getPrevHash() helpers. addLedgerEntry() now links entries by prevHash/hash and persists the running hash.
Burn operation (backend)
platforms/eCurrency-api/src/services/LedgerService.ts, platforms/eCurrency-api/src/controllers/LedgerController.ts, platforms/eCurrency-api/src/index.ts
Added burn() method in LedgerService performing validation, treasury/account handling, balance checks, and a single debit ledger entry; controller exposes burn(req,res) with auth and error mapping; route POST /api/ledger/burn registered.
Controller responses extended
platforms/eCurrency-api/src/controllers/LedgerController.ts
History and get-transaction responses augmented to include hash and prevHash fields.
Client: transaction UI & copy
platforms/eCurrency/client/src/components/currency/transaction-detail-modal.tsx
Added display of Transaction Hash and Previous Hash with copy-to-clipboard buttons and brief copied-state feedback.
Client: burn UI & cache invalidation
platforms/eCurrency/client/src/pages/currency-detail.tsx, platforms/eCurrency/client/src/components/currency/add-currency-account-modal.tsx
Added Burn modal, Burn button for admins, API call to /api/ledger/burn, query invalidation on success; broadened cache invalidation in add-currency-account flow to refresh balances, history, and accountDetails.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Controller as LedgerController
    participant Service as LedgerService
    participant DB as Ledger DB

    Client->>Controller: POST /api/ledger/burn {currencyId, amount, ...}
    activate Controller
    Controller->>Service: burn(currencyId, groupId, amount, reason)
    activate Service

    Service->>DB: Query latest ledger entry for currencyId (to get prevHash)
    DB-->>Service: latestEntry (may be null)
    Service->>Service: computeHash(entryPayload + prevHash)
    Service->>DB: Insert ledger entry (initial), then update with prevHash & hash
    DB-->>Service: persisted Ledger with hash & prevHash

    Service-->>Controller: returned Ledger (with hash, prevHash)
    Controller-->>Client: 200 {transaction: {..., hash, prevHash}}
    deactivate Controller
    deactivate Service
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay special attention to:
    • Hash correctness: ensure all relevant fields are included deterministically in computeHash().
    • Race conditions: getPrevHash + insert+update pattern may need transactional guarantees.
    • Error mapping in LedgerController.burn (400 vs 500) and authentication/group-admin checks.
    • DB migrations or schema sync for new nullable columns.
    • Client-side invalidation and modal/error handling for burn flow.

Possibly related PRs

Suggested reviewers

  • sosweetham
  • xPathin

Poem

🐰 Hopping through ledgers, I nibble a byte,

I stitch prevHash to hash with quiet delight,
A burn puff of moonlight, a ledger embraced,
Copies in clipboard, the chain's neatly traced,
I thump — new hashes snug in their place.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete. It mentions 'add auto reload logic' as the main change and states the type as 'Update', but fails to adequately describe the significant new burn feature implementation, hash-linked ledger mechanism, database schema changes, and API endpoint additions. Expand the description to detail the burn endpoint, hash-linked ledger entries, LedgerService modifications, Ledger entity schema updates, and UI components changes. Add testing details and reference any related issues.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title mentions 'burn button' and 'auto reload' which align with the actual changes (burn functionality and cache invalidation), but it is vague and does not clearly convey the scope of the comprehensive API enhancements including hash-linked ledger entries, LedgerService burn operation, and database schema changes. Provide a more descriptive title such as 'feat: add burn transaction endpoint with hash-linked ledger support' to better reflect the core technical changes.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (3)
platforms/eCurrency/client/src/components/currency/transaction-detail-modal.tsx (1)

186-190: Same issue: unhandled Promise and potential timeout leak.

Apply the same fix as above for prevHash. Additionally, consider cleaning up the timeout on unmount to prevent state updates on an unmounted component:

// Alternative: use a ref to track timeout and clear on unmount
useEffect(() => {
  return () => {
    // cleanup timeout on unmount if needed
  };
}, []);
platforms/eCurrency/client/src/pages/currency-detail.tsx (1)

386-395: Consider disabling the Burn button until functionality is implemented.

The empty onClick handler with only a TODO provides no user feedback when clicked. Users might think the button is broken rather than "coming soon."

                   <button
                     onClick={() => {
                       // TODO: wire burn modal/flow when API is ready
                     }}
-                    className="px-6 py-2 border rounded-lg hover:bg-gray-50 flex items-center gap-2 font-medium"
+                    className="px-6 py-2 border rounded-lg hover:bg-gray-50 flex items-center gap-2 font-medium opacity-50 cursor-not-allowed"
                     title="Burn from treasury (coming soon)"
+                    disabled
                   >

Alternatively, hide the button entirely until the burn flow is ready, or show a toast/alert on click indicating the feature is coming soon.

platforms/eCurrency-api/src/services/LedgerService.ts (1)

149-176: Consider: transfer() lacks transaction wrapping with new hash chain.

The transfer method performs two addLedgerEntry calls (debit at line 149, credit at line 165). With the new hash chain feature, if the credit entry fails after the debit succeeds:

  1. The ledger becomes financially inconsistent (debit without matching credit)
  2. The hash chain includes a debit entry that references an orphaned state

While this may be pre-existing, the hash chain feature increases the importance of atomicity here. Consider wrapping the entire transfer in a transaction.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 46f3dd6 and 68440e0.

📒 Files selected for processing (6)
  • platforms/eCurrency-api/src/controllers/LedgerController.ts (2 hunks)
  • platforms/eCurrency-api/src/database/entities/Ledger.ts (1 hunks)
  • platforms/eCurrency-api/src/services/LedgerService.ts (3 hunks)
  • platforms/eCurrency/client/src/components/currency/add-currency-account-modal.tsx (1 hunks)
  • platforms/eCurrency/client/src/components/currency/transaction-detail-modal.tsx (3 hunks)
  • platforms/eCurrency/client/src/pages/currency-detail.tsx (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (7)
platforms/eCurrency/client/src/components/currency/add-currency-account-modal.tsx (1)

74-86: LGTM! Broader cache invalidation ensures data consistency.

The predicate-based invalidation correctly refreshes all related queries (balances, history, accountDetails) after account creation. This is more robust than targeting a single query key.

Note: The balancesQueryKey variable on line 24 is still used by the balances query on line 35, so it's not entirely unused.

platforms/eCurrency/client/src/components/currency/transaction-detail-modal.tsx (1)

156-199: Overall hash display implementation looks good.

The conditional rendering logic and UI structure are clean. The copy-to-clipboard feedback with visual state change provides good UX.

platforms/eCurrency-api/src/controllers/LedgerController.ts (2)

209-210: LGTM! Hash fields correctly exposed in history response.

The hash and prevHash fields are appropriately included in the enriched history response, consistent with the new Ledger entity fields.


277-278: LGTM! Hash fields correctly exposed in transaction detail response.

Consistent with the history endpoint, these fields enable the client to display transaction hashes.

platforms/eCurrency/client/src/pages/currency-detail.tsx (2)

9-9: LGTM! Flame icon import for Burn button.


378-397: Fragment wrapper and Mint button placement are correct.

The use of a React fragment to group the Mint and Burn buttons is appropriate, maintaining proper JSX structure within the conditional rendering.

platforms/eCurrency-api/src/services/LedgerService.ts (1)

19-21: LGTM!

The SHA-256 hashing implementation using Node's built-in crypto module is appropriate for this use case.

Comment on lines +79 to +83
@Column({ type: "text", nullable: true })
hash!: string | null;

@Column({ type: "text", nullable: true })
prevHash!: string | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -path "*/platforms/eCurrency-api/src/database/entities/Ledger.ts"

Repository: MetaState-Prototype-Project/prototype

Length of output: 138


🏁 Script executed:

cd platforms/eCurrency-api/src/database/entities && ls -la | grep -i ledger

Repository: MetaState-Prototype-Project/prototype

Length of output: 139


🏁 Script executed:

# List files in the entities directory if it exists
if [ -d "platforms/eCurrency-api/src/database/entities" ]; then
  ls -la platforms/eCurrency-api/src/database/entities/
else
  echo "Path does not exist, searching for Ledger files..."
  find . -name "*Ledger*" -type f 2>/dev/null
fi

Repository: MetaState-Prototype-Project/prototype

Length of output: 557


🏁 Script executed:

# Search for any references to hash and prevHash in entity files
rg "prevHash" --type ts --type js

Repository: MetaState-Prototype-Project/prototype

Length of output: 1705


🏁 Script executed:

# Check git status and recent changes
git ls-files --modified --deleted --others --exclude-standard 2>/dev/null | head -20

Repository: MetaState-Prototype-Project/prototype

Length of output: 63


🏁 Script executed:

# Get repository structure overview
git ls-files | head -30

Repository: MetaState-Prototype-Project/prototype

Length of output: 794


🏁 Script executed:

# Search all TypeScript files for hash or prevHash
rg "hash|prevHash" --type ts -B 2 -A 2

Repository: MetaState-Prototype-Project/prototype

Length of output: 45571


🏁 Script executed:

git log --oneline -20

Repository: MetaState-Prototype-Project/prototype

Length of output: 114


🏁 Script executed:

# Check if we can find the actual PR/change files
git show --name-only 2>/dev/null | head -30

Repository: MetaState-Prototype-Project/prototype

Length of output: 854


🏁 Script executed:

# Look for the specific path mentioned in the review
find . -path "*platforms*" -name "*.ts" 2>/dev/null | head -10

Repository: MetaState-Prototype-Project/prototype

Length of output: 637


🏁 Script executed:

# Broader search - find all entity files
find . -name "*entities*" -type d 2>/dev/null

Repository: MetaState-Prototype-Project/prototype

Length of output: 603


🏁 Script executed:

# Check if there are any ts files at all, and search for hash/prevHash patterns
rg -t ts "hash|prevHash" --max-count 50

Repository: MetaState-Prototype-Project/prototype

Length of output: 14862


🏁 Script executed:

# Look for migration files in the eCurrency-api directory
find platforms/eCurrency-api/src/database/migrations -type f -name "*.ts" 2>/dev/null | head -10

Repository: MetaState-Prototype-Project/prototype

Length of output: 455


🏁 Script executed:

# Search for migration files that mention ledger, hash, or prevHash
rg -t ts "ledger|Ledger" platforms/eCurrency-api/src/database/migrations/ 2>/dev/null | head -20

Repository: MetaState-Prototype-Project/prototype

Length of output: 4061


🏁 Script executed:

# Check if index is already created on hash or prevHash
rg "@Index" platforms/eCurrency-api/src/database/entities/Ledger.ts

Repository: MetaState-Prototype-Project/prototype

Length of output: 131


🏁 Script executed:

# Look for any files mentioning migration in eCurrency-api
rg -t ts "migration|Migration" platforms/eCurrency-api/ --max-count 5

Repository: MetaState-Prototype-Project/prototype

Length of output: 2134


🏁 Script executed:

# Check the LedgerService implementation for hash logic
cat -n platforms/eCurrency-api/src/services/LedgerService.ts | head -100

Repository: MetaState-Prototype-Project/prototype

Length of output: 4530


🏁 Script executed:

# Look for any reference to index creation on hash field
rg -t ts "@Index|createIndex" platforms/eCurrency-api/src/database/

Repository: MetaState-Prototype-Project/prototype

Length of output: 187


🏁 Script executed:

# Check LedgerService for hash computation implementation
grep -n "computeHash\|getPrevHash" platforms/eCurrency-api/src/services/LedgerService.ts

Repository: MetaState-Prototype-Project/prototype

Length of output: 330


🏁 Script executed:

# Also check if there's an update to any migration or initial migration that includes these fields
git log --oneline -20 -- platforms/eCurrency-api/src/database/

Repository: MetaState-Prototype-Project/prototype

Length of output: 114


🏁 Script executed:

# Check if there's a dedicated migration file or if it's inline
find platforms/eCurrency-api -name "*.ts" -type f | xargs grep -l "ALTER TABLE.*ledger\|CREATE TABLE.*ledger" 2>/dev/null

Repository: MetaState-Prototype-Project/prototype

Length of output: 230


🏁 Script executed:

# Look for migration directory or setup
find platforms/eCurrency-api/src -name "*.ts" | xargs grep -l "MigrationInterface" 2>/dev/null

Repository: MetaState-Prototype-Project/prototype

Length of output: 455


🏁 Script executed:

# Check the service implementation in detail
wc -l platforms/eCurrency-api/src/services/LedgerService.ts && echo "" && head -80 platforms/eCurrency-api/src/services/LedgerService.ts

Repository: MetaState-Prototype-Project/prototype

Length of output: 3102


🏁 Script executed:

# Check if there's a database.json or similar config that manages migrations automatically
find platforms/eCurrency-api -name "*.json" | xargs grep -l "migration" 2>/dev/null

Repository: MetaState-Prototype-Project/prototype

Length of output: 117


🏁 Script executed:

# Get git show for the actual changes
git show 68440e0:platforms/eCurrency-api/src/database/entities/Ledger.ts > /tmp/new_ledger.ts
git show 68440e0~1:platforms/eCurrency-api/src/database/entities/Ledger.ts > /tmp/old_ledger.ts 2>/dev/null
diff -u /tmp/old_ledger.ts /tmp/new_ledger.ts || echo "New file or no previous version"

Repository: MetaState-Prototype-Project/prototype

Length of output: 2151


🏁 Script executed:

# Get full diff of the commit
git show 68440e0 | head -200

Repository: MetaState-Prototype-Project/prototype

Length of output: 5242


🏁 Script executed:

# Also check if there are query patterns that would benefit from hash index
rg "where.*hash|find.*hash|query.*hash" platforms/eCurrency-api/src --type ts -A 2 -B 2

Repository: MetaState-Prototype-Project/prototype

Length of output: 63


🏁 Script executed:

# Check if hash is used in any WHERE clauses or lookups
grep -rn "where.*hash\|find.*hash\|query.*hash" platforms/eCurrency-api/src --include="*.ts"

Repository: MetaState-Prototype-Project/prototype

Length of output: 63


🏁 Script executed:

# Check the SQL queries in the latest migration
grep -n "ALTER TABLE\|ADD" platforms/eCurrency-api/src/database/migrations/1765784749012-migration.ts | head -20

Repository: MetaState-Prototype-Project/prototype

Length of output: 279


Database migration for hash/prevHash columns is missing—application will fail at runtime.

The Ledger entity defines hash and prevHash columns and the LedgerService writes to these fields, but the latest database migration (1765784749012-migration.ts) does NOT include ALTER TABLE statements to add these columns. When the application attempts to save a ledger entry with these new fields, it will crash with a "column does not exist" error.

A migration file must be created immediately to add the nullable text columns to the ledger table before deploying this feature:

ALTER TABLE "ledger" ADD "hash" text;
ALTER TABLE "ledger" ADD "prevHash" text;
🤖 Prompt for AI Agents
In platforms/eCurrency-api/src/database/entities/Ledger.ts around lines 79-83,
the entity added nullable text columns `hash` and `prevHash` but there is no DB
migration adding those columns; create a new TypeORM migration file
(timestamped, e.g., 1765784749013-add-ledger-hash-prevHash.ts) that runs SQL to
ALTER TABLE "ledger" ADD "hash" text; and ALTER TABLE "ledger" ADD "prevHash"
text; make sure the migration export up/down functions add and drop those
columns respectively, place it in the migrations folder, commit, and run the
migration before deploying so the application won’t fail with "column does not
exist".

Comment on lines +23 to +29
private async getPrevHash(currencyId: string): Promise<string | null> {
const prev = await this.ledgerRepository.findOne({
where: { currencyId },
order: { createdAt: "DESC", id: "DESC" },
});
return prev?.hash ?? null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Race condition risk in hash chain creation.

When multiple concurrent addLedgerEntry calls occur for the same currencyId, both can retrieve the same prevHash before either commits, breaking the hash chain integrity. The current implementation has a TOCTOU (time-of-check to time-of-use) vulnerability.

Consider wrapping the entire addLedgerEntry operation in a database transaction with appropriate isolation level (e.g., SERIALIZABLE) or using a row-level lock on the latest entry to ensure sequential hash chain construction.

 private async getPrevHash(currencyId: string): Promise<string | null> {
-    const prev = await this.ledgerRepository.findOne({
+    // NOTE: This should be called within a serializable transaction
+    // to prevent race conditions in hash chain construction
+    const prev = await this.ledgerRepository.findOne({
         where: { currencyId },
         order: { createdAt: "DESC", id: "DESC" },
     });
     return prev?.hash ?? null;
 }

For a proper fix, the addLedgerEntry method should use a transaction:

async addLedgerEntry(...): Promise<Ledger> {
    return await AppDataSource.transaction("SERIALIZABLE", async (manager) => {
        const ledgerRepo = manager.getRepository(Ledger);
        // ... perform all operations within this transaction
    });
}
🤖 Prompt for AI Agents
In platforms/eCurrency-api/src/services/LedgerService.ts around lines 23 to 29,
the getPrevHash + addLedgerEntry flow has a TOCTOU race: concurrent
addLedgerEntry calls can read the same prev hash and break the chain; fix by
running the entire addLedgerEntry logic inside a database transaction with an
isolation level that prevents this (e.g., SERIALIZABLE) or by acquiring a
row-level lock on the latest ledger row for the currency before reading the prev
hash and inserting the new entry; specifically, refactor addLedgerEntry to use
AppDataSource.transaction(...) and use the transaction manager to get the Ledger
repository, read the latest row with a FOR UPDATE/locking mechanism (or rely on
SERIALIZABLE), compute the new hash and save the new ledger record via the
manager so the read+write happen atomically within the transaction.

Comment on lines +86 to +110
const entry = this.ledgerRepository.create(entryData);
const saved = await this.ledgerRepository.save(entry);
return Array.isArray(saved) ? saved[0] : saved;

const hashPayload = {
id: saved.id,
currencyId: saved.currencyId,
accountId: saved.accountId,
accountType: saved.accountType,
amount: saved.amount,
type: saved.type,
description: saved.description,
senderAccountId: saved.senderAccountId,
senderAccountType: saved.senderAccountType,
receiverAccountId: saved.receiverAccountId,
receiverAccountType: saved.receiverAccountType,
balance: saved.balance,
createdAt: saved.createdAt,
prevHash,
};

saved.prevHash = prevHash;
saved.hash = this.computeHash(hashPayload);

const finalized = await this.ledgerRepository.save(saved);
return Array.isArray(finalized) ? finalized[0] : finalized;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Two-phase save lacks atomicity; entry may persist without hash on failure.

The current flow saves the entry first (line 87), then computes and saves the hash (line 109). If the second save fails (e.g., network issue, DB constraint), the ledger entry persists without hash and prevHash, corrupting the hash chain.

Wrap both operations in a single transaction to ensure atomicity:

-    const entry = this.ledgerRepository.create(entryData);
-    const saved = await this.ledgerRepository.save(entry);
-
-    const hashPayload = { ... };
-
-    saved.prevHash = prevHash;
-    saved.hash = this.computeHash(hashPayload);
-
-    const finalized = await this.ledgerRepository.save(saved);
-    return Array.isArray(finalized) ? finalized[0] : finalized;
+    return await AppDataSource.transaction(async (manager) => {
+        const repo = manager.getRepository(Ledger);
+        const entry = repo.create(entryData);
+        const saved = await repo.save(entry);
+
+        const hashPayload = { /* ... fields ... */ };
+        saved.prevHash = prevHash;
+        saved.hash = this.computeHash(hashPayload);
+
+        return await repo.save(saved);
+    });

Additionally, line 110's Array.isArray(finalized) check is unnecessary—TypeORM's save() only returns an array when passed an array, and saved is always a single entity here.

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +167 to +171
onClick={() => {
navigator.clipboard?.writeText(transaction.hash);
setCopiedField("hash");
setTimeout(() => setCopiedField(null), 1200);
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unhandled Promise rejection on clipboard write.

navigator.clipboard.writeText() returns a Promise that could reject (e.g., permission denied, non-secure context). Consider handling the error gracefully.

                        onClick={() => {
-                          navigator.clipboard?.writeText(transaction.hash);
-                          setCopiedField("hash");
-                          setTimeout(() => setCopiedField(null), 1200);
+                          navigator.clipboard?.writeText(transaction.hash)
+                            .then(() => {
+                              setCopiedField("hash");
+                              setTimeout(() => setCopiedField(null), 1200);
+                            })
+                            .catch(() => {
+                              // Clipboard write failed - optionally show error feedback
+                            });
                        }}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
platforms/eCurrency/client/src/components/currency/transaction-detail-modal.tsx
around lines 167-171, the clipboard write call uses
navigator.clipboard.writeText(...) without handling the returned Promise, which
can reject; update the onClick handler to await or attach .then/.catch to
writeText, only call setCopiedField("hash") and the timeout on success, and on
failure handle it gracefully (e.g., log the error, show a user-friendly message
or fallback copy strategy). Also keep the existing optional chaining guard and
ensure any async use is properly handled to avoid unhandled promise rejections.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
platforms/eCurrency-api/src/services/LedgerService.ts (2)

23-29: Race condition in hash chain construction remains unaddressed.

As previously noted, concurrent addLedgerEntry calls for the same currencyId can retrieve the same prevHash before either commits, breaking hash chain integrity.


83-110: Two-phase save without atomicity remains unaddressed.

As previously noted, the entry is saved first without hash (line 87), then updated with hash (line 109). If the second save fails, an entry persists without hash/prevHash, corrupting the chain.

🧹 Nitpick comments (2)
platforms/eCurrency/client/src/pages/currency-detail.tsx (1)

511-570: Consider clearing error state when closing the modal.

When the modal is closed via the Cancel button or overlay click, burnError is not reset. This could cause a stale error message to appear when reopening the modal.

Apply this diff to clear the error on close:

-<div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50" onClick={() => setBurnOpen(false)}>
+<div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50" onClick={() => { setBurnError(null); setBurnOpen(false); }}>

And for the Cancel button:

-<button
-  onClick={() => setBurnOpen(false)}
+<button
+  onClick={() => { setBurnError(null); setBurnOpen(false); }}
platforms/eCurrency-api/src/services/LedgerService.ts (1)

216-226: Balance validation logic is duplicated from transfer method.

Consider extracting the balance boundary check into a shared helper method to reduce duplication and ensure consistent validation across operations.

Example extraction:

private validateDebitBounds(
    currency: Currency,
    currentBalance: number,
    amount: number
): void {
    if (!currency.allowNegative && currentBalance < amount) {
        throw new Error("Insufficient balance. Negative balances are not allowed.");
    }
    if (currency.allowNegative && currency.maxNegativeBalance !== null && currency.maxNegativeBalance !== undefined) {
        const newBalance = currentBalance - amount;
        if (newBalance < Number(currency.maxNegativeBalance)) {
            throw new Error(`Insufficient balance. This currency allows negative balances down to ${currency.maxNegativeBalance}.`);
        }
    }
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68440e0 and c4de373.

📒 Files selected for processing (4)
  • platforms/eCurrency-api/src/controllers/LedgerController.ts (4 hunks)
  • platforms/eCurrency-api/src/index.ts (1 hunks)
  • platforms/eCurrency-api/src/services/LedgerService.ts (4 hunks)
  • platforms/eCurrency/client/src/pages/currency-detail.tsx (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
platforms/eCurrency-api/src/index.ts (1)
platforms/eCurrency-api/src/middleware/auth.ts (1)
  • authGuard (36-41)
platforms/eCurrency/client/src/pages/currency-detail.tsx (1)
platforms/eCurrency/client/src/lib/apiClient.ts (1)
  • apiClient (5-10)
🪛 GitHub Actions: Check Code
platforms/eCurrency/client/src/pages/currency-detail.tsx

[error] 520-520: TS2304: Cannot find name 'X'.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (9)
platforms/eCurrency-api/src/index.ts (1)

121-121: LGTM!

The new burn route follows the established pattern for protected ledger endpoints with proper authGuard middleware.

platforms/eCurrency/client/src/pages/currency-detail.tsx (3)

21-25: LGTM!

State variables for the burn flow are well-structured and follow the existing patterns used for mint and max-negative operations.


242-276: LGTM!

The burn handler properly validates input, makes the API call, invalidates relevant caches, and handles errors consistently with other operations in the file.


419-435: LGTM!

The burn button is properly placed alongside the mint button with consistent styling and appropriate admin-only visibility.

platforms/eCurrency-api/src/controllers/LedgerController.ts (4)

5-5: LGTM!

CurrencyService dependency is properly imported, declared, and initialized following the existing pattern for GroupService.

Also applies to: 10-10, 15-15


212-213: LGTM!

Hash fields are properly exposed in the transaction history response to support the hash chain feature.


280-281: LGTM!

Hash fields are consistently exposed in the transaction detail response.


299-352: LGTM!

The burn endpoint implementation is well-structured with proper authentication, input validation, authorization check (group admin only), and error handling that's consistent with the existing transfer endpoint.

platforms/eCurrency-api/src/services/LedgerService.ts (1)

19-21: LGTM!

The computeHash method correctly uses SHA-256 for deterministic hashing of ledger entry payloads.

@coodos coodos merged commit 5cec93d into main Dec 15, 2025
4 checks passed
@coodos coodos deleted the fix/reload-and-burn-ecurrency branch December 15, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants