Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply contest rate limits #8475

Merged
merged 15 commits into from
Jul 18, 2024
Merged

Apply contest rate limits #8475

merged 15 commits into from
Jul 18, 2024

Conversation

rbennettcw
Copy link
Contributor

@rbennettcw rbennettcw commented Jul 16, 2024

Link to Issue

Closes: #8466

Description of Changes

  • Prevents thread creation if all active contests have reached the contest post limit for the user
  • Returns contest data in the GET /topics route
  • Applies limit in ContestWorker where user must have > 0.0005 ETH to post to contest
  • Applies limit in ContestWorker where user can post up to 2 threads per contest round
  • Moves a SQL query to its own query file
  • Include Marcin's UI updates too

Test Plan

Post limit test

  • Create a contest (A)
  • With a wallet > 0 ETH, create 3 posts in a row
    • Ensure that on the 3rd post, the consumer shows warning "user reached post limit for contest"
  • Create a new contest (B)
  • With a wallet > 0 ETH, create 1 post
    • Ensure the consumer shows the limit warning for contest A, but succeeds for contest B

ETH requirement test

  • With a wallet == 0 ETH, create a post during an active contest
    • Ensure that warning is shown: "user ETH balance insufficient"

Other Considerations

  • The only condition in which thread creation is blocked upfront is when the user has reached the post limit for all active contests

@rbennettcw rbennettcw requested review from Rotorsoft and masvelio July 16, 2024 08:22
@masvelio
Copy link
Contributor

masvelio commented Jul 16, 2024

Post limit test

Ok, I got warning from the ContestWorker ThreadCreated: user reached post limit for contest 0x8bCD287DE6F4cF470420966ff53b808D2BEBcFDF (ID 0) but I was able to create a thread anyways. I don't think this is expected. According to the designs and spec, we should show error on the UI and prevent creation of the thread. This means, that I need to get some error from the backend to be able to detect this scenario.

image

ETH requirement test
Same here, got a warning, but I need an error thrown from the backend API to be able to catch it in the FE and display the message + prevent thread creation

EDIT: on top of that, see my comment here

@rbennettcw
Copy link
Contributor Author

@masvelio I've applied fixes:

  • Thread create error is thrown if all active contests have limits reached
  • GET /topics route now returns contest managers + the actions of the current contest, which should be sufficient info for you to render what's needed in the UI

Copy link
Contributor

@masvelio masvelio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works now after changes thanks

rbennettcw and others added 6 commits July 17, 2024 08:44
* 8474 affordance for admin

* disable creating topic

* disable creating topic if user has dust eth value

* revert value

---------

Co-authored-by: Ryan Bennett <ryan@common.xyz>
@dillchen dillchen temporarily deployed to commonwealth-demo July 17, 2024 21:20 Inactive
@dillchen dillchen temporarily deployed to commonwealth-demo July 18, 2024 18:49 Inactive
@rbennettcw rbennettcw merged commit c6eaddc into master Jul 18, 2024
9 checks passed
@rbennettcw rbennettcw deleted the ryan/contest-limits branch July 18, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply Contest rate limits to backend
4 participants