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

[CHIA-1773] Add better reuse_puzhash checking to WalletTestFramework #18897

Merged
merged 8 commits into from
Nov 22, 2024

Conversation

Quexington
Copy link
Contributor

@Quexington Quexington commented Nov 19, 2024

(reviewing with whitespace changes hidden will be beneficial)

I learned recently that the WalletTestFramework testing for the respect of the reuse_puzhash parameter in the WalletActionScope was subpar in that it would only check for new puzzle hashes being generated as the result of farming a block. While this is a useful check, it ignored most of where the puzzle hashes get generated which is during spend construction.

This PR is still not perfect, because the only thing it adds is that no puzzle hashes are generated between action scope opening/closing. If the wrong value is fed to an action scope, it can't check for that still, but hopefully as our TX endpoints become more and more standardized, that responsibility can mostly fall to mypy. Still, this is a massive improvement that identified many bugs that the framework had previously missed.

@Quexington Quexington added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Nov 19, 2024
@Quexington Quexington changed the title Add better reuse_puzhash checking to WalletTestFramework [CHIA-1773] Add better reuse_puzhash checking to WalletTestFramework Nov 20, 2024
@Quexington Quexington marked this pull request as ready for review November 21, 2024 15:07
@Quexington Quexington requested a review from a team as a code owner November 21, 2024 15:07
Copy link
Contributor

File Coverage Missing Lines
chia/wallet/cat_wallet/cat_wallet.py 50.0% lines 147, 150, 429
chia/wallet/did_wallet/did_wallet.py 83.3% lines 145, 1106
chia/wallet/wallet_puzzle_store.py 90.9% lines 360
chia/wallet/wallet_state_manager.py 85.7% lines 1546
Total Missing Coverage
110 lines 7 lines 93%

@markelrod
Copy link
Contributor

code diff ok

@Starttoaster Starttoaster merged commit 9755c46 into main Nov 22, 2024
361 of 362 checks passed
@Starttoaster Starttoaster deleted the quex.coin_rpc_reuse_puzhash branch November 22, 2024 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog coverage-diff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants