Skip to content

fix: resolve race condition in update_from_tx (PM-19905)#767

Merged
m2ux merged 11 commits into
mainfrom
fix/PM-19905-race-condition-update-from-tx
Feb 28, 2026
Merged

fix: resolve race condition in update_from_tx (PM-19905)#767
m2ux merged 11 commits into
mainfrom
fix/PM-19905-race-condition-update-from-tx

Conversation

@m2ux

@m2ux m2ux commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix the read-modify-write race condition in LedgerContext::update_from_tx by holding the ledger_state mutex for the full critical section, preventing lost updates under concurrent access.

🎫 Ticket 📐 Engineering 🧪 Test Plan


Motivation

Audit report Issue AJ (Least Authority, October 2025) identified a structural race condition in LedgerContext::update_from_tx. The method clones the ledger state under the mutex, releases it, performs transaction validation and application, then re-acquires the mutex to write back. Two concurrent callers can clone the same baseline, compute divergent results, and the second writer silently overwrites the first — causing lost updates.

While the current toolkit code calls update_from_tx sequentially (proving is parallelized, not state mutation), the structural vulnerability would silently cause data loss if concurrent calls were introduced in the future.


Changes

  • Lock scope — Acquire ledger_state mutex at method entry; hold through validation, application, and write-back (previously released after clone, re-acquired for write-back)
  • Inline context — Replace self.tx_context() (clone-and-release) with inline TransactionContext construction from the held guard
  • Write-back — Write through the held guard instead of re-acquiring the mutex
  • Lock ordering — First simultaneous L+W hold; wallets acquired nested within ledger_state and released first (safe, no circular dependency)
  • Preserve APItx_context() public method left unchanged for backward compatibility
  • Tests — Lock-level concurrency test (8 threads x 100 iterations) demonstrating serialization across all 3 ledger versions

Submission Checklist

  • Changes are backward-compatible (or flagged if breaking)
  • Pull request description explains why the change is needed
  • Self-reviewed the diff
  • I have included a change file, or skipped for this reason: audit fix, no feature change
  • If the changes introduce a new feature, I have bumped the node minor version
  • Update documentation (if relevant)
  • No new todos introduced

Fork Strategy

  • Node Runtime Update
  • Node Client Update
  • Other
  • N/A

TODO before merging

  • Ready for review

Race condition fix in update_from_tx (Least Authority Issue AJ)

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

kics-logo

KICS version: v2.1.16

Category Results
CRITICAL CRITICAL 0
HIGH HIGH 0
MEDIUM MEDIUM 99
LOW LOW 12
INFO INFO 83
TRACE TRACE 0
TOTAL TOTAL 194
Metric Values
Files scanned placeholder 31
Files parsed placeholder 31
Files failed to scan placeholder 0
Total executed queries placeholder 73
Queries failed to execute placeholder 0
Execution time placeholder 9

@m2ux m2ux self-assigned this Feb 26, 2026
m2ux and others added 7 commits February 27, 2026 08:32
Race condition fix in update_from_tx (Least Authority Issue AJ)

Co-authored-by: Cursor <cursoragent@cursor.com>
Closes the read-modify-write race window (Audit Issue AJ) by acquiring
the ledger_state lock at method entry and holding it through the
write-back, instead of clone-and-release via tx_context().

Lock ordering (ledger_state → wallets) is preserved and consistent
with update_from_block.

Refs: PM-19905
Exercises the same ledger_state mutex that update_from_tx now holds
for its full RMW cycle. Spawns 8 threads doing non-atomic
read-yield-write under with_ledger_state, verifying no lost updates.

Runs across all three ledger versions via compile-time versioning.

Covers: PR767-TC-02 (no lost updates), PR767-TC-03 (no deadlock)
Refs: PM-19905
@gilescope gilescope requested a review from ozgb February 27, 2026 12:05
@m2ux m2ux marked this pull request as ready for review February 28, 2026 11:01
@m2ux m2ux requested a review from a team as a code owner February 28, 2026 11:01
@m2ux m2ux added this pull request to the merge queue Feb 28, 2026
Merged via the queue into main with commit dd49935 Feb 28, 2026
42 checks passed
@m2ux m2ux deleted the fix/PM-19905-race-condition-update-from-tx branch February 28, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants