Voice Gate: define atomic `_delete_ping` function

The code for ping deletion was duplicated in two places. In this commit,
we move it into a helper function, and apply a lock to make each
transaction atomic.

This means that if two coroutines try to call the function, the first
has to finish before the second can begin. This avoids the following:

Coro1: Message in cache? Yes.
Coro1: Send delete request. Yield control (await).
Coro2: Message in cache? Yes.

Now Coro2 has to wait for Coro1 to finish. Therefore it will always
find the `NO_MSG` signal, and not attempt the deletion.

Co-authored-by: MarkKoz <[email protected]>
Co-authored-by: Sebastiaan Zeeff <[email protected]>
Co-authored-by: Daniel Brown <[email protected]>

1 files changed, 19 insertions, 11 deletions

diff --git a/bot/exts/moderation/voice_gate.py b/bot/exts/moderation/voice_gate.py
index 97b588e72..d53d08efe 100644
--- a/bot/exts/moderation/voice_gate.py
+++ b/bot/exts/moderation/voice_gate.py
@@ -54,6 +54,23 @@ class VoiceGate(Cog):
         """Get the currently loaded ModLog cog instance."""
         return self.bot.get_cog("ModLog")
 
+    @redis_cache.atomic_transaction  # Fully process each call until starting the next
+    async def _delete_ping(self, member_id: int) -> None:
+        """
+        If `redis_cache` holds a message ID for `member_id`, delete the message.
+
+        If the message was deleted, the value under the `member_id` key is then set to `NO_MSG`.
+        When `member_id` is not in the cache, or has a value of `NO_MSG` already, this function
+        does nothing.
+        """
+        if message_id := await self.redis_cache.get(member_id):
+            log.trace(f"Removing voice gate reminder message for user: {member_id}")
+            with suppress(discord.NotFound):
+                await self.bot.http.delete_message(Channels.voice_gate, message_id)
+            await self.redis_cache.set(member_id, NO_MSG)
+        else:
+            log.trace(f"Voice gate reminder message for user {member_id} was already removed")
+
     @command(aliases=('voiceverify',))
     @has_no_roles(Roles.voice_verified)
     @in_whitelist(channels=(Channels.voice_gate,), redirect=None)
@@ -67,12 +84,7 @@ class VoiceGate(Cog):
         - You must not be actively banned from using our voice channels
         - You must have been active for over a certain number of 10-minute blocks
         """
-        # If user has received a ping in voice_verification, delete the message
-        if message_id := await self.redis_cache.get(ctx.author.id, NO_MSG):
-            log.trace(f"Removing voice gate reminder message for user: {ctx.author.id}")
-            with suppress(discord.NotFound):
-                await self.bot.http.delete_message(Channels.voice_gate, message_id)
-            await self.redis_cache.set(ctx.author.id, NO_MSG)
+        await self._delete_ping(ctx.author.id)  # If user has received a ping in voice_verification, delete the message
 
         try:
             data = await self.bot.api_client.get(f"bot/users/{ctx.author.id}/metricity_data")
@@ -217,11 +229,7 @@ class VoiceGate(Cog):
 
         await asyncio.sleep(GateConf.voice_ping_delete_delay)
 
-        if message_id := await self.redis_cache.get(member.id):
-            log.trace(f"Removing voice gate reminder message for user: {member.id}")
-            with suppress(discord.NotFound):
-                await self.bot.http.delete_message(Channels.voice_gate, message_id)
-            await self.redis_cache.set(member.id, NO_MSG)
+        await self._delete_ping(member.id)
 
     async def cog_command_error(self, ctx: Context, error: Exception) -> None:
         """Check for & ignore any InWhitelistCheckFailure."""