fix: prevent double withdrawal of community earnings (#835)

grunch · claude · web-flow · commit 23a3d2f29fb4 · 2026-06-13T18:52:35.000+02:00
* fix: prevent double withdrawal of community earnings Atomically claim community earnings before scheduling a payout by zeroing them with a conditional compare-and-set. Concurrent withdrawal attempts now race on this update so only one wins, preventing the node from paying out the same earnings more than once. If the payout later fails permanently (invoice expired or payment attempts exhausted), the pending-payments job restores the earnings so the community creator can withdraw again without losing funds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address review feedback on earnings claim accounting - Roll back the atomic earnings claim if creating the PendingPayment fails (user lookup or save throws). Without a persisted pending payment the retry job can never restore the claimed earnings, so they would be stranded. - Restore earnings before any notification can abort the run: wrap the expired-invoice notification in try/catch so a failed sendMessage no longer skips the restore and leaves funds locked. - Stop re-zeroing community.earnings on a successful payout; the earnings were already claimed at scheduling time, and resetting them again would wipe out earnings accrued in the meantime. - Use an atomic $inc to restore earnings on permanent failure instead of a stale read-modify-write, so the update applies against the current DB value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: sync package-lock.json version to 0.15.2 package.json was bumped to 0.15.2 (commit 056284a) but package-lock.json still declared 0.15.1. The CI 'Run prettier' step runs 'npm install' followed by 'git diff --exit-code', and npm rewrites the lockfile version to match package.json, producing an uncommitted diff that fails the check. Sync the lockfile version so the working tree stays clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/bot/modules/community/scenes.ts b/bot/modules/community/scenes.ts
@@ -1004,7 +1004,9 @@ export const addEarningsInvoiceWizard = new Scenes.WizardScene(
       const res = await isValidInvoice(ctx, lnInvoice);
       if (!res.success) return;
 
-      if (!!res.invoice.tokens && res.invoice.tokens !== community.earnings)
+      const amountToWithdraw = community.earnings;
+
+      if (!!res.invoice.tokens && res.invoice.tokens !== amountToWithdraw)
         return await ctx.reply(ctx.i18n.t('invoice_with_incorrect_amount'));
 
       const isScheduled = await PendingPayment.findOne({
@@ -1019,18 +1021,42 @@ export const addEarningsInvoiceWizard = new Scenes.WizardScene(
       if (!!isScheduled || !!isPending)
         return await ctx.reply(ctx.i18n.t('invoice_already_being_paid'));
 
-      const user = await User.findById(community.creator_id);
-      if (user === null) throw new Error('user was not found');
-      logger.debug(`Creating pending payment for community ${community.id}`);
-      const pp = new PendingPayment({
-        amount: community.earnings,
-        payment_request: lnInvoice,
-        user_id: user.id,
-        community_id: community._id,
-        description: `Retiro por admin @${user.username}`,
-        hash: res.invoice.hash,
-      });
-      await pp.save();
+      // SECURITY: atomically claim the earnings before scheduling the payout.
+      // We zero the community's earnings only if they still equal the amount
+      // we snapshotted. Two concurrent withdrawals race on this compare-and-set
+      // and only one wins; the loser gets null and aborts, which prevents the
+      // node from paying the same earnings twice. If the payout later fails
+      // permanently the job restores the earnings (see attemptCommunities-
+      // PendingPayments) so the creator can withdraw again.
+      const claimed = await Community.findOneAndUpdate(
+        { _id: community._id, earnings: amountToWithdraw },
+        { earnings: 0 },
+      );
+      if (claimed === null)
+        return await ctx.reply(ctx.i18n.t('invoice_already_being_paid'));
+
+      // The earnings were already claimed (zeroed) above. If creating the
+      // pending payment fails there is nothing for the retry job to restore
+      // from, so roll the claim back here to avoid stranding the earnings.
+      try {
+        const user = await User.findById(community.creator_id);
+        if (user === null) throw new Error('user was not found');
+        logger.debug(`Creating pending payment for community ${community.id}`);
+        const pp = new PendingPayment({
+          amount: amountToWithdraw,
+          payment_request: lnInvoice,
+          user_id: user.id,
+          community_id: community._id,
+          description: `Retiro por admin @${user.username}`,
+          hash: res.invoice.hash,
+        });
+        await pp.save();
+      } catch (error) {
+        await Community.findByIdAndUpdate(community._id, {
+          $inc: { earnings: amountToWithdraw },
+        });
+        throw error;
+      }
       await ctx.reply(ctx.i18n.t('invoice_updated_and_will_be_paid'));
 
       return ctx.scene.leave();
diff --git a/jobs/pending_payments.ts b/jobs/pending_payments.ts
@@ -217,22 +217,32 @@ export const attemptCommunitiesPendingPayments = async (
       // If the buyer's invoice is expired we let it know and don't try to pay again
       if (!!payment && payment.is_expired) {
         pending.is_invoice_expired = true;
-        await bot.telegram.sendMessage(
-          user.tg_id,
-          i18nCtx.t('invoice_expired_earnings'),
-          {
-            reply_markup: {
-              inline_keyboard: [
-                [
-                  {
-                    text: i18nCtx.t('withdraw_earnings'),
-                    callback_data: `withdrawEarnings_${pending.community_id}`,
-                  },
+        // Don't let a notification failure abort the run before the earnings
+        // are restored further down; restoring the balance is the critical
+        // step and the pending payment is excluded from future runs once it is
+        // flagged as expired.
+        try {
+          await bot.telegram.sendMessage(
+            user.tg_id,
+            i18nCtx.t('invoice_expired_earnings'),
+            {
+              reply_markup: {
+                inline_keyboard: [
+                  [
+                    {
+                      text: i18nCtx.t('withdraw_earnings'),
+                      callback_data: `withdrawEarnings_${pending.community_id}`,
+                    },
+                  ],
                 ],
-              ],
+              },
             },
-          },
-        );
+          );
+        } catch (error) {
+          logger.error(
+            `Failed to notify user ${user.tg_id} about expired invoice: ${error}`,
+          );
+        }
       }
 
       const community = await Community.findById(pending.community_id);
@@ -241,8 +251,10 @@ export const attemptCommunitiesPendingPayments = async (
         pending.paid = true;
         pending.paid_at = new Date();
 
-        // Reset the community's values
-        community.earnings = 0;
+        // The earnings were already atomically zeroed when this withdrawal was
+        // claimed at scheduling time, so don't reset them again here: doing so
+        // would wipe out any earnings accrued between the claim and this
+        // successful payment.
         community.orders_to_redeem = 0;
         await community.save();
         logger.info(
@@ -270,10 +282,25 @@ export const attemptCommunitiesPendingPayments = async (
           );
         }
 
-        if (
+        const attemptsExhausted =
           process.env.PAYMENT_ATTEMPTS !== undefined &&
-          pending.attempts >= parseInt(process.env.PAYMENT_ATTEMPTS)
-        ) {
+          pending.attempts >= parseInt(process.env.PAYMENT_ATTEMPTS);
+
+        // SECURITY/accounting: the earnings were atomically claimed (zeroed)
+        // when this withdrawal was scheduled. If the payout has now failed
+        // permanently (invoice expired or no attempts left) we restore them so
+        // the community can withdraw again without losing funds. Because the
+        // pending payment is then excluded from future runs (is_invoice_expired
+        // or attempts >= PAYMENT_ATTEMPTS), this restore happens exactly once.
+        if (pending.is_invoice_expired || attemptsExhausted) {
+          // Atomic increment against the current DB value so we don't clobber
+          // earnings accrued since this community document was read above.
+          await Community.findByIdAndUpdate(community._id, {
+            $inc: { earnings: pending.amount },
+          });
+        }
+
+        if (attemptsExhausted) {
           await bot.telegram.sendMessage(
             user.tg_id,
             i18nCtx.t('pending_payment_failed_earnings', {
diff --git a/package-lock.json b/package-lock.json
