8000 pkg/cosmosfaucet: fix race condition during transfer · Issue #1965 · ignite/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

pkg/cosmosfaucet: fix race condition during transfer #1965

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.

Sign up for GitHub

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

Jump to bottom
Closed
ilgooz opened this issue Jan 3, 2022 · 4 comments · Fixed by #1974
Closed

pkg/cosmosfaucet: fix race condition during transfer #1965

ilgooz opened this issue Jan 3, 2022 · 4 comments · Fixed by #1974
Assignees
@lumtis
Labels
report type:error Something isn't working
Milestone
v0.19.2

Comments

@ilgooz
Copy link
Member
ilgooz commented Jan 3, 2022 •
edited
Loading

When sending concurrent requests to the faucet API, it seems that faucet can send more balance to an account, instead of respecting the limitation set inside coin max configuration.

There is a high chance that when sending concurrent requests to the faucet, in the time of querying the balance, for some requests this check might happen at the same time and end up seeing less balance in the account because sending the tokens action has not been finalized for the previous requests.

To avoid this race condition we can add a lock to the Transfer function, to put the transfer requests to faucet into some sort of queue and they won't interfere with each other.
@ilgooz ilgooz added the report label Jan 3, 2022
@ilgooz ilgooz added this to the v0.19.2 milestone Jan 3, 2022
@ilgooz
Copy link
Member Author
ilgooz commented Jan 3, 2022

@ivanovpetr can you please debug to find the cause of this issue and check if adding a mutex to the Transfer function would solve it?

We can add an integration test for this by sending concurrent requests (x50) to the faucet API.

The configuration in the config.yml (faucet part) of the test chain could be:

faucet:
  name: bob
  coins: ["5token", "100000stake"]  
  coins_max: ["11token", "100000stake"]

So, in our integration test, we need to see only 10 tokens as the balance of account that we requested funds for (after sending x50 requests to the faucet API).

@ilgooz ilgooz added the type:error Something isn't working label Jan 3, 2022
@Shashank-In
Copy link
Shashank-In commented Jan 3, 2022 •
edited
Loading

Hi, @ilgooz
Shashank here from the email thread.
Yes, the lock on Transfer should definitely fix this.
Is there a PR for us? I and Aditya would be happy to retest the fix.

@ilgooz
Copy link
Member Author
ilgooz commented Jan 3, 2022 •
edited
Loading

Hey @Shashank-In! There is no a PR yet, we will look into it this week. But if this is urgent for you please free feel create one with or without the integration test (we can add it later). 🙌 For your notice, integration tests go under the integration/ folder in the root.

@lumtis lumtis self-assigned this Jan 5, 2022
@lumtis lumtis linked a pull request Jan 6, 2022 that will close this issue
fix: improve faucet reliability #1974
Merged
@lumtis
Copy link
Contributor
lumtis commented Jan 6, 2022

The maximum amount of coins is actually not overflown because a parallel faucet request fails

@ilgooz ilgooz mentioned this issue Jan 13, 2022
Merged
1 task
@ilgooz ilgooz closed this as completed Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lumtis lumtis

Labels
report type:error Something isn't working
Projects
None yet
Milestone
v0.19.2
Development

Successfully merging a pull request may close this issue.

fix: improve faucet reliability ignite/cli
4 participants
@ilgooz @lumtis @ivanovpetr @Shashank-In
0