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

Add pending callbacks to FeeService #5528

Merged
merged 1 commit into from
May 27, 2021

Conversation

sqrrm
Copy link
Member

@sqrrm sqrrm commented May 25, 2021

This fix guarantees a callback if there is a pending request rather than just
dropping the fee request.

When requestFee() is called the callbacks are added to a list of callBacks.
Once the next fee request completes all callbacks in the list are called
and the list cleared.

This issue popped up while testing atomic trades with the API and could likely be an issue for other usages of the API. If requestFees() is called while there is an pending request no callback is called. Since it's expected that either the resultHandler or the faultHandler is called it's not clear what happens for callers relying on the callbacks. This PR adds a guarantee one callback is called.

Also return early for the frequency check.

This fix guarantees a callback if there is a pending request rather than just
dropping the fee request.

When requestFee() is called the callbacks are added to a list of callBacks.
Once the next fee request completes all callbacks in the list are called
and the list cleared.
@ripcurlx ripcurlx added this to the v1.6.6 milestone May 25, 2021
@ghubstan
Copy link
Contributor

Tested ACK

I used an api test to make hundreds of fee requests @ 1 per 0.5s -- no problems.

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK - based on #5528 (comment)

@ripcurlx ripcurlx merged commit 44af88e into bisq-network:master May 27, 2021
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.

3 participants