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

Get block spends #16451

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Get block spends #16451

merged 2 commits into from
Oct 10, 2023

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Sep 28, 2023

Purpose:

The get_block_spends RPC will not currently work after the hard fork. This can be demonstrated on testnet10 where the hard fork has already activated.

This patch fixes get_block_spends and also improves the reliability of the get_puzzle_and_solution RPC, which currently does take the hard fork serialization format into account, but no other hard- or soft fork rule changes.

This change incorporates part of a patch from @freddiecoleman .

Current Behavior:

get_block_spends fail after the hard fork.
get_puzzle_and_solution may fail if certain new soft-fork features are used on the block in question.

New Behavior:

get_block_spends have the intended behavior after the hard fork.
get_puzzle_and_solution have the intended behavior regardless of which block it's called on.

Testing Notes:

The test for get_block_spends was extended to be run for each soft-fork version and the hard fork.

@arvidn arvidn added the Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog label Sep 28, 2023
@arvidn arvidn changed the base branch from release/2.1.0 to release/2.1.1 October 5, 2023 23:11
@arvidn arvidn marked this pull request as ready for review October 5, 2023 23:12
@arvidn arvidn requested a review from a team as a code owner October 5, 2023 23:12
@arvidn
Copy link
Contributor Author

arvidn commented Oct 6, 2023

the missing test coverage is in a benchmark, which we don't instrument

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

File Coverage Missing Lines
tests/core/test_cost_calculation.py 66.7% lines 307
Total Missing Coverage
25 lines Unknown 96%

cameroncooper
cameroncooper previously approved these changes Oct 6, 2023
@arvidn arvidn added ready_to_merge Submitter and reviewers think this is ready and removed coverage-diff ready_to_merge Submitter and reviewers think this is ready labels Oct 10, 2023
@arvidn arvidn changed the base branch from release/2.1.1 to release/2.1.2 October 10, 2023 15:06
@arvidn arvidn dismissed cameroncooper’s stale review October 10, 2023 15:06

The base branch was changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants