Is this SQL safe enough to handle user balance transaction?

Question

I am building a system with nodeJS, sequelize, Mysql. It has a modules handles user purchasing game items. The logic is:

1. User initiate a request to buy an item

2. Server read user balance -

   2.1 if balance is enough, issue user the game item and deduct the price of the item from user balance.

   2.2 if balance is NOT enough, abort this request/transaction

The code I wrote to implement is:

return await Models.sequelize.transaction(async (t) => {
  const wallet = await Models.Wallets.findOne({where: {user: userId}, transasction: t})
  if (wallet.balance > item.price) {
    await assignItemToUser(userId, t) // this is another sql with transaction
    await wallet.decrement(
      "balance",
      { by: item.price },
      { transaction: t }
    );
   } else {
     throw new Error("cannot let user buy item")
   }
})

Let's say user has balance $100, each item worth $90. I am wondering if this logic is secure enough to make sure I don't have a negative balance in a race condition where user triggers two buy action and both transaction read the balance $100 then resulting in user bought two items and with a negative balance (100 - 90 -90 = - 80) dollar?

Answer 1

As the comments state this is a job for the database. Each database has a concept of transaction isolation levels, you can read more about those in MySQL here.

You could start a transaction in Sequelize as mentioned using managed transactions or you can do this in the database itself using a stored procedure (START TRANSACTION). The main issue is that you've got a number of different locks/reads taking place.

1: First you need to read the balance. This will be a read, (probably) only takes a small amount of time, and isn't related in any way to the other actions you're going to take later on.

2: Then you're going to insert the sale to the user's records

3: Finally, you're going to update the balance.

The time difference between step 1 and step 3 is where you're going to introduce a problem, as you say with a race condition. Your test will be logging in to one account, opening 5 tabs with different purchases and pushing them as quickly as possible, one after the other.

What you want to do is to make sure you lock the balance until after each transaction has committed. You can do this in MySql with a locking read, you can read more about that here.

1: Start a transaction

2: Read the balance with FOR UPDATE. This locks this row, so any other reads will need to wait until your transaction from step 1 commits or rolls back. This won't affect any other accounts doing purchases as you're only locking one row for one user.

3: You insert the sale record

4: Update the balance

5: Commit the transaction

What you should now see in your test set up where you're firing 5 sales from the same account, is that the last sale will wait for all the others to complete before continuing.