search

Joystream / discord-bot

Joystream Discord Bots
GNU General Public License v3.0
0 stars 4 forks source link

Initial Commit #4

Closed chrlschwb closed 1 year ago

chrlschwb commented 1 year ago

Adding two discord bots

bwhm commented 1 year ago

Hey @chrlschwb,

For the Discord-Tip-Bot, I have a few suggestions:

  1. Replace balances.transfer with balances.transferKeepAlive, so we avoid the account (and balances) being reaped.
  2. Use bn. Although probably unlikely the account, much less a user, exceeds this balance, it's still better.
  3. It doesn't look like fees are considered. If correct, that will cause trouble down the road...

Now, on the more serious side:

  1. It looks like the only way to deposit is to provide a seed to the bot? If that is correct, please find another approach to this. For example:
    • Require a deposit when setting the wallet, and/or request a signature, or add a random, small amount of HAPI on top of the amount the user wants to deposit. Then, have the user tell the bot when "ready", and listen for balances.Transfer events matching amount/address for 5 blocks or so...
  2. Maintain some "maximum" balance, so we avoid the bot holding more funds than we want some relatively insecure server to control. If exceeded, the bot automatically withdraws some amount back to the user.

Going to assign @DzhideX for the "full" review, as the DB parts would take me longer than it should :D

For the "old" discord bot: should we assume that this is the same version as PR #1? If so, I don't really want to merge it, unless it's clearly listed as deprecated or smth.

bedeho commented 1 year ago

This PR is too big, and we are not fixing old problems, and feature set is not appropriate, here is a better way forward in my opinion

https://github.com/Joystream/discord-bot/issues/7