-
Notifications
You must be signed in to change notification settings - Fork 2k
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 some checks when trying to join the same pool already joined #18822
base: main
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 11716290246Details
💛 - Coveralls |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Conflicts have been resolved. A maintainer will review the pull request shortly. |
await time_out_assert(45, status_is_farming_to_pool, True, wallet_id) | ||
|
||
# Test joining the same pool via the RPC client | ||
with pytest.raises(ValueError, match="Already farming to pool"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs an import as well.
with pytest.raises(ValueError, match="Already farming to pool"): | |
with pytest.raises(ResponseFailureError, match="Already farming to pool"): |
@@ -1020,3 +1022,48 @@ async def status_is_leaving_no_blocks() -> bool: | |||
|
|||
# Eventually, leaves pool | |||
assert await status_is_farming_to_pool() | |||
|
|||
@pytest.mark.anyio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason this isn't WalletTestFramework
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing in this file was using WalletTestFramework at the time, so I followed the conventions here. But I think your question is a good one and I'll look to convert it to use the newer concepts
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties. |
Currently, if you are farming to a pool, and you join that same pool again, those transactions are merrily carried out on the chain even though this changes nothing for your farming.
Previous version would throw a cryptic error which was also a problem
This PR checks before creating any spends if the join pool URL is the same as the current pool URL
Fixes #7292