Basic Discord Bot #53

sveitser · 2023-06-20T15:01:47Z

Limitations

There is no persistence so during downtime requests will be lost. However the discord users would see that the bot does not respond to messages like it usually does.
Funding logic only runs at the start of the faucet. The faucet will always use the available account with the highest available balance for transfers, so I think refunding is unlikely to be necessary.
There is currently no functionality to rebalance funds when decreasing the number of clients. It shouldn't take more than 15 minutes to write a client for that though.
The CI currently can't run successfully because we need the faucet docker image that doesn't exist on main. The command to pull docker images fails as a result and the test that uses the zkevm-node can't work.

TODO

Docker container.
Add to compose env.
Documentation for faucet setup.
Web server with health check.

Close EspressoSystems/espresso-sequencer#373

Not really working at the moment beyond a few transfers. Suspect there's some kind of deadlock and may have to change the design a bit.

Popping from two Vecs is a bad idea.

The zkevm websocket server runs on a different port than the http server. Other than that there are no surprises.

sveitser · 2023-06-29T14:35:58Z

faucet/src/lib.rs

+    inflight_clients: HashMap<Address, Arc<Middleware>>,
+    // Funding wallets has priority, these transfer requests must be pushed to
+    // the front.
+    transfer_queue: VecDeque<Transfer>,


It may be somewhat simpler to use a channel for this transfer queue as well or just get rid of it completely. Currently for internal funding transfers the faucet will add to this queue directly and the external faucet transfer get added to this queue via the faucet_receiver.

faucet/src/lib.rs

faucet/src/main.rs

- Fix docker compose demo. - Include change of zkevm-node to panic on startup. - Make it possible to run without discord faucet for local testing.

sveitser · 2023-06-30T16:01:34Z

The CI currently fails because it needs the container that is only available from main.

This reverts commit 17d38ac. This doesn't work: The workflow is not valid. .github/workflows/build.yml (Line: 19, Col: 3): The workflow must contain at least one job with no dependencies.

faucet/src/discord.rs

sveitser · 2023-07-03T19:02:23Z

For the CI I suggest we just merge it and then if necessary make new PRs to fix it. It's quite involved to change the CI workflows so that it can use the docker containers built from this repo directly so I don't think it's worth it at this stage.

sveitser · 2023-07-03T19:07:47Z

faucet/src/faucet.rs

+        let required = match transfer {
+            // Double the faucet amount to be on the safe side regarding gas.
+            TransferRequest::Faucet { amount, .. } => amount * 2,
+            TransferRequest::Funding { .. } => self.total_balance / self.clients.len(),


This is wrong. The balance here should be looking at the total balance and not the balance that is currently in the pool of available clients. Maybe it's better to keep all the clients in the pool but mark them as available or not, instead of moving the in and out of the pool.

Fix in 8d82f76

.env

faucet/src/discord.rs

jbearer · 2023-07-03T19:24:30Z

faucet/src/discord.rs

+        let intents = GatewayIntents::GUILD_MESSAGES
+            | GatewayIntents::DIRECT_MESSAGES
+            | GatewayIntents::MESSAGE_CONTENT;


I think to avoid spam we should restrict the bot to listen only to messages in a specific channel (and maybe DMs). We don't want it responding to every message in any channel that happens to include a valid address (e.g. someone asks for help in a support channel and happens to include their address). Not sure how or where to do that.

I thought we can tell discord to only have the bot get messages from the faucet channel but it turns out that is way more involved than I thought and one has to remove the bot from each channel that it shouldn't respond to. Some discussion here: https://support.discord.com/hc/en-us/community/posts/360045778711-Restrict-bots-to-certain-channels-

The easiest for us would be to filter by channel ID in rust but this is a bit tedious to configure: we need to add an env var with the channel ID.

I think a more idiomatic discord way of doing it may be to

add a command to the bot, so users would have to write for example /faucet 0x123... instead of just posting the address

only allow that command in the faucet channel.

Unfortunately after half an hour I'm still not sure how to create these commands with serenity.

Another solution that might be okay, but still tedious. We can remove the bots permissions to read messages for all channel categories except for a channel category that only contains the faucet channel.

Got this slash command thing to work after all. It also includes some docs for the users and AFAICT does not require any extra config in the code.

The discord UI can be used to restrict usage of the bot to a channel

This change is in 4d54dd9

faucet/src/faucet.rs

Require a wallet to have more than the average initiial balance of all the faucet wallets. - Log an error when we retry for a transaction receipt of a transactions that's already in a block.

Avoid holding guards longer than necessary.

jbearer

LGTM

sveitser · 2023-07-05T06:52:10Z

Will merge to get docker image built. Then fix what's necessary.

Base automatically changed from ma/eth-client-to-generate-load to main June 22, 2023 18:40

sveitser force-pushed the ma/zkevm-faucet branch from 90088fb to 475f1d2 Compare June 26, 2023 16:36

sveitser added 6 commits June 28, 2023 19:13

Basic Discord Bot

f154147

WIP: faucet funding logic

45121ee

Not really working at the moment beyond a few transfers. Suspect there's some kind of deadlock and may have to change the design a bit.

WIP: figuring out where transfers get lost

6264888

WIP: prototype works now

c856e36

Popping from two Vecs is a bad idea.

Remove unnecessary variable

943efa1

Add smoke test, faucet works with zkevm-node

b676ba5

The zkevm websocket server runs on a different port than the http server. Other than that there are no surprises.

sveitser force-pushed the ma/zkevm-faucet branch from 475f1d2 to b676ba5 Compare June 28, 2023 18:00

sveitser added 9 commits June 29, 2023 10:55

Move receipt handling to separate method

c2151ca

Fund clients with below average balance on startup

25dbfb3

Remove unused unfunded_clients attribute

8eb4e42

Add client priority pool

6596412

Simplify code a bit, handle some error scenarios

376743f

Remove some unused code

2bd1480

Discord and faucet integration

bd260c1

Cleanup, clippy

36ee65c

Remove unused method

ffd14a6

sveitser commented Jun 29, 2023

View reviewed changes

GitHub Action: add foundry (to use anvil)

ed1516b

sveitser commented Jun 29, 2023

View reviewed changes

faucet/src/lib.rs Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into ma/zkevm-faucet

4a8447c

sveitser commented Jun 29, 2023

View reviewed changes

faucet/src/main.rs Outdated Show resolved Hide resolved

sveitser added 5 commits June 30, 2023 06:39

Add faucet docker image

122a31e

Add webserver for faucet and healthcheck

7481689

- Fix docker compose demo. - Include change of zkevm-node to panic on startup. - Make it possible to run without discord faucet for local testing.

Split code into a few files

f7c0e4a

Deduplicate test code

3f8dfa1

Clean up and fix zkevm-node faucet tests

22d36f7

Temporarily disable test that uses docker images

9b47ecb

sveitser added 5 commits July 3, 2023 14:11

Update zkevm-node

4a32b7b

CI: try running with docker images from PR branch

17d38ac

Revert "CI: try running with docker images from PR branch"

fdf241b

This reverts commit 17d38ac. This doesn't work: The workflow is not valid. .github/workflows/build.yml (Line: 19, Col: 3): The workflow must contain at least one job with no dependencies.

Simplify execute_transfers loop

5d712ef

Periodically clean up timed out transactions

fe10cfe

sveitser marked this pull request as ready for review July 3, 2023 18:05

sveitser requested review from nomaxg, philippecamacho and jbearer as code owners July 3, 2023 18:05

Cleanup

60ab915

sveitser commented Jul 3, 2023

View reviewed changes

faucet/src/discord.rs Outdated Show resolved Hide resolved

sveitser added 2 commits July 3, 2023 20:24

Add missing env var to docker compose file

e579c25

Add explicit env var for enabling discord bot

b41578c

sveitser commented Jul 3, 2023

View reviewed changes

jbearer reviewed Jul 3, 2023

View reviewed changes

sveitser added 5 commits July 4, 2023 11:04

Fix computation of funds for funding transfer

8d82f76

Require a wallet to have more than the average initiial balance of all the faucet wallets. - Log an error when we retry for a transaction receipt of a transactions that's already in a block.

Remove unnecessary nesting

acffce3

Use slash command instead of reading messages

4d54dd9

Refactor transfer execution

8815d73

Avoid holding guards longer than necessary.

Remove env var to enable discord

b8e5e01

sveitser requested a review from jbearer July 4, 2023 17:28

jbearer approved these changes Jul 4, 2023

View reviewed changes

Execute more transfers in anvil faucet test

9cfc40b

sveitser merged commit 8acbe40 into main Jul 5, 2023
0 of 3 checks passed

sveitser deleted the ma/zkevm-faucet branch July 5, 2023 06:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic Discord Bot #53

Basic Discord Bot #53

sveitser commented Jun 20, 2023 •

edited

sveitser Jun 29, 2023

sveitser commented Jun 30, 2023

sveitser commented Jul 3, 2023

sveitser Jul 3, 2023

sveitser Jul 4, 2023 •

edited

jbearer Jul 3, 2023

sveitser Jul 4, 2023 •

edited

sveitser Jul 4, 2023 •

edited

sveitser Jul 4, 2023

jbearer left a comment

sveitser commented Jul 5, 2023

Basic Discord Bot #53

Basic Discord Bot #53

Conversation

sveitser commented Jun 20, 2023 • edited

Limitations

TODO

sveitser Jun 29, 2023

Choose a reason for hiding this comment

sveitser commented Jun 30, 2023

sveitser commented Jul 3, 2023

sveitser Jul 3, 2023

Choose a reason for hiding this comment

sveitser Jul 4, 2023 • edited

Choose a reason for hiding this comment

jbearer Jul 3, 2023

Choose a reason for hiding this comment

sveitser Jul 4, 2023 • edited

Choose a reason for hiding this comment

sveitser Jul 4, 2023 • edited

Choose a reason for hiding this comment

sveitser Jul 4, 2023

Choose a reason for hiding this comment

jbearer left a comment

Choose a reason for hiding this comment

sveitser commented Jul 5, 2023

sveitser commented Jun 20, 2023 •

edited

sveitser Jul 4, 2023 •

edited

sveitser Jul 4, 2023 •

edited

sveitser Jul 4, 2023 •

edited