Properly build interaction locks

[SGA-A]

Describe the bug Interaction locks are not implemented properly. Currently, when the user initiates a confirmation prompt, it prevents them from interacting only with other confirmation prompts. This is not intended behaviour and may lead to incorrect data being sent accross the bot.

To Reproduce Steps to reproduce the behavior:

1. Start any confirmation prompt within the Economy system
2. Use any other command that conflicts with what that prompt is asking for
3. See error

Expected behavior As other economy bots have stated: Between any commands that have data being written (coins moving, items gained/lost, even XP earned, etc.) we should lock your account to prevent two conflicting operations from happening. If those conflicting operations happen, it can cause missing items OR the opposite (duplicates!).

[SGA-A]

Another situation encountered, this assumes you have sharing currency confirmations enabled:

1. Share 7,000,000 with someone when you have a wallet containing 10,000,000
2. Deposit all of that money into your bank
3. Operation works successfully, but now your money reaches negatives (-7,000,000)

This is one of the many reasons why we need to implement interaction locks.

[SGA-A]

One solution to this would be to:

• Create a transaction table to indicate all the current transactions taking place. Give a UNIQUE constraint to the userID column.
• Using the interaction_check function enclosed within the Economy cog, acquire a connection and attempt to insert the userID into the column. Note that this should be the only column within the transaction table.
  • If an sqlite3.IntegrityError is raised, return a value of False in the callback
  • Else, if all works successfully, use a context manager to commit and close the connection. Returns True.

I do not wish to do it this way since it requires acquiring a connection from the pool twice if the operation succeeds with no error. Perhaps if there was a way to pass it onto the command the user is interacting with, it would be better. So, we're not sure how to implement this yet. Also worth noting that implementing this may lead to significantly slower response times because we're checking if they are already in a transaction beforehand in the interaction check.

We'll figure out a solution soon when the time comes.

[serephenna]

It might be worth starting a transaction only in certain commands because not all commands can cause a conflicting operation to begin with. Take a look at the balance command, it consists purely of readers to the database, it does not contain any writers (except for it's view, but this nullified by the modal popup displayed that prevents other operations from taking place). It is worth considering, as it may optimize performance when we implement the solution.

This means only reading if a unique constraint was violated in certain commands that are likely to cause a conflicting operation, but not all of them. It also means writing (INSERTing) into the transaction table only within these commands bring rise to potential conflicting operations.

[SGA-A]

Another situation encountered, this assumes you have sharing currency confirmations enabled:

1. Share 7,000,000 with someone when you have a wallet containing 10,000,000
2. Deposit all of that money into your bank
3. Operation works successfully, but now your money reaches negatives (-7,000,000)

This is one of the many reasons why we need to implement interaction locks.

It should be noted that the main reason this occured however was because we were not adding the user to the active session when this confirmation setting is toggled on. Doing this would be the first step into solving the issue along with blocking any form of commands that have a conflicting operation, which was not implemented at the time (and still isn't).

It is planned that we either block only these operations that involve currency and items moving from place to place or block the user from using the entire cog. The former is not ideal, other bots tend to do the latter because it reduces the possibility of missing out certain commands.

So @serephenna's idea is good, but I can't see how this would help improve code maintainability. We're still on the fence as for what happens, so this issue is still on hold for the time being.