Skip to content

fix: make fund locking atomic#127

Open
q36zhd46w17o wants to merge 1 commit into
tari-project:mainfrom
q36zhd46w17o:codex/tari-125-atomic-fund-lock
Open

fix: make fund locking atomic#127
q36zhd46w17o wants to merge 1 commit into
tari-project:mainfrom
q36zhd46w17o:codex/tari-125-atomic-fund-lock

Conversation

@q36zhd46w17o

Copy link
Copy Markdown

Summary

Closes #125.

This fixes the UTXO locking race by moving the idempotency lookup, UTXO selection, pending transaction creation, and output locking into one BEGIN IMMEDIATE SQLite transaction. That keeps the critical section enforced by the wallet database instead of a process-global mutex.

It also makes lock_output() fail if the selected output is no longer UNSPENT, so a stale selection cannot silently produce a pending transaction without actually locking the output.

Bounty

This PR is submitted for the open #125 bounty: Tier S, 15,000 XTM, first merged PR wins, payment after merge. Payout details can be provided through the maintainer-approved flow if this PR is selected.

Tests

Using stable Rust 1.96.0 because current Tari dependencies require rustc >= 1.93.0:

  • cargo fmt --all -- --check
  • cargo test -p minotari transactions::fund_locker -- --nocapture
  • cargo test -p minotari db::outputs -- --nocapture
  • cargo check -p minotari
  • cargo test -p minotari --lib --bins
  • cargo test -p minotari --doc
  • cargo clippy -p minotari --all-targets -- -D warnings

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the fund locking mechanism to prevent race conditions and ensure idempotency under concurrent execution. Specifically, it modifies lock_output to verify that exactly one row was updated, returning an error if the output is no longer unspent. It also updates FundLocker::lock to initiate an immediate transaction earlier in the function, ensuring that idempotency checks and UTXO selection occur within the same transaction. Finally, concurrent unit tests are added to verify these behaviors. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@SWvheerden SWvheerden left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I dont like this approach, ideally you should allow multiple lookups without locking all, and you can more grafully handleing the locking

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.

Race condition in utxo locking

2 participants