bavix / laravel-wallet

It's easy to work with a virtual wallet
https://bavix.github.io/laravel-wallet/
MIT License
1.15k stars 230 forks source link

Wallet balance should be refreshed after lock acquired #124

Closed shavenking closed 5 years ago

shavenking commented 5 years ago

For example, in CommonService#forceWithdraw:

public function forceWithdraw(Wallet $wallet, int $amount, ?array $meta, bool $confirmed = true): Transaction
{
    return app(LockService::class)->lock($this, __FUNCTION__, function () use ($wallet, $amount, $meta, $confirmed) {
        $walletService = app(WalletService::class);
        $walletService->checkAmount($amount);

        /**
         * @var WalletModel $wallet
         */
        $wallet = $walletService->getWallet($wallet);

        // we should refresh balance here or anywhere after lock acquired

        $transactions = $this->multiOperation($wallet, [
            (new Operation())
                ->setType(Transaction::TYPE_WITHDRAW)
                ->setConfirmed($confirmed)
                ->setAmount(-$amount)
                ->setMeta($meta)
        ]);

        return current($transactions);
    });
}

After $wallet = $walletService->getWallet($wallet);, we should refresh the wallet balance, or we could run into an issue caused by race condition, like the following snippet:

$wallet = $user->wallet; // balance now 100

// another thread complete withdraw, real balance now 50

$user->forceWithdraw(100); // balance now 0, which is incorrect, it should be -50

For now, the solution for me is acquiring my own lock, and refresh balance after locked.

rez1dent3 commented 5 years ago

@shavenking Hello. Yes you are right. To combat the race condition, an additional package bavix/laravel-wallet-vacuum was implemented. Balance data is stored in single-threaded redis and the problem disappears.

https://github.com/bavix/laravel-wallet-vacuum/blob/master/src/Store.php#L18 https://github.com/bavix/laravel-wallet-vacuum/blob/master/src/VacuumServiceProvider.php#L28

rez1dent3 commented 5 years ago

You must also enable lock to combat race condition.

https://github.com/bavix/laravel-wallet/blob/master/config/config.php#L39 https://github.com/bavix/laravel-wallet/blob/master/config/config.php#L40

rez1dent3 commented 5 years ago

I close the issue, there are questions - create a new one.