Skip to content

Conversation

aqk
Copy link
Contributor

@aqk aqk commented Aug 13, 2024

No description provided.

@aqk aqk changed the title Update pool reference code to be compatible with chia-blockchain 2.4.2 CHIA-992: Update pool reference code to be compatible with chia-blockchain 2.4.2 Aug 13, 2024
@aqk aqk requested review from altendky, Quexington and emlowe August 13, 2024 16:39
Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

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

Remove new empty file tests/test_partial_validation.py?

Quexington
Quexington previously approved these changes Aug 13, 2024
Copy link

@Quexington Quexington left a comment

Choose a reason for hiding this comment

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

This seems fine to me but so many of these changes are so old I think we should consider deprecating this repo if no one noticed this until now.

@aqk
Copy link
Contributor Author

aqk commented Aug 15, 2024

Remove new empty file tests/test_partial_validation.py?

Thanks, ya.

@aqk
Copy link
Contributor Author

aqk commented Aug 15, 2024

This seems fine to me but so many of these changes are so old I think we should consider deprecating this repo if no one noticed this until now.

Earle really wanted to deprecate the repo, but then he reasoned "what if we run into an issue with pooling (which we maintain), and have to fix this before we can even repro the problem?"

Your argument that these changes are very old is a good argument to move the code into the main repo.

@aqk aqk requested a review from altendky August 15, 2024 17:05
@emlowe
Copy link
Contributor

emlowe commented Aug 15, 2024

This seems fine to me but so many of these changes are so old I think we should consider deprecating this repo if no one noticed this until now.

I went back and forth on this myself - but my current rationale is that we do need a way to be able to test the pool features of the wallet from time to time and we can't always rely on pools running testnet instances - and doing the testing on mainnet can be complicated as well. So there is some value in being able to test the wallet features with a nominally working testnet pool server.

There have been some occasional comments in discord that this repo wasn't up to date, so it was noticed by that small group of users who run pools.

Copy link
Contributor

@emlowe emlowe left a comment

Choose a reason for hiding this comment

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

Remove empty test_partial_validation.py

@aqk aqk requested a review from emlowe August 15, 2024 18:14
@altendky altendky dismissed their stale review August 15, 2024 18:15

concerns were addressed

@Quexington
Copy link

Quexington commented Aug 15, 2024

we do need a way to be able to test the pool features of the wallet from time to time

If this is the whole rationale then we should at least maybe put effort into stripping this down to the simplest possible pooling client. Like it doesn't even need to run a loop scanning for rewards, the DB can be in memory, etc. We can then cut down on complexity and hopefully dependencies and exposure to the chia-blockchain source "library" code.

@emlowe emlowe merged commit 0ee875e into main Aug 16, 2024
6 checks passed
@emlowe emlowe deleted the ak.update-to-chia-blockchain-2.4.2 branch August 16, 2024 17:23
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.

4 participants