PoS2-prover #20159

arvidn · 2025-10-16T19:51:25Z

Purpose:

This PR bumps the chia_rs dependency to the latest version. The first commit is limited to updating the version and updating usage of it accordingly. The second PR updates the V2Prover class to use the new functionality exposed by chia_rs.

It's recommended to review the commits separately.

Current Behavior:

plot strength schedule is used
There's no schedule for v2 plot filter
partial proofs are expected to be bytes
the prover API is assumed to get partial proofs directly from the plot, and then qualities from the partial proof

New Behavior:

plot strength schedule has been removed
There's a new plot filter schedule for v2 plots
partial proofs are list[uint64], and sometimes need to be serialized to form a key for a dictionary
the prover API gets qualities from the plot, and only if they pass the difficulty do we request the remaining bits to form the partial proof
The V2Prover class can now load v2 plots

arvidn · 2025-10-17T10:04:56Z

most of the uncovered lines will be covered once we've completed support for v2 plots, and have the test plots and test chains in place

coveralls-official · 2025-10-17T22:37:02Z

Pull Request Test Coverage Report for Build 18665417613

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

122 of 177 (68.93%) changed or added relevant lines in 18 files are covered.
144 unchanged lines in 11 files lost coverage.
Overall coverage decreased (-0.02%) to 90.826%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
chia/solver/solver_api.py	3	4	75.0%
chia/_tests/harvester/test_harvester_api.py	6	11	54.55%
chia/harvester/harvester_api.py	6	15	40.0%
chia/simulator/block_tools.py	9	21	42.86%
chia/plotting/check_plots.py	0	13	0.0%
chia/plotting/prover.py	25	40	62.5%

Files with Coverage Reduction	New Missed Lines	%
chia/plotting/check_plots.py	1	0.0%
chia/server/node_discovery.py	1	81.22%
chia/_tests/simulation/test_simulation.py	1	96.47%
chia/consensus/blockchain.py	2	94.38%
chia/farmer/farmer_api.py	2	95.03%
chia/plotting/prover.py	2	81.05%
chia/_tests/core/test_farmer_harvester_rpc.py	2	98.08%
chia/full_node/full_node_api.py	3	85.8%
chia/_tests/core/util/test_lockfile.py	3	88.89%
chia/full_node/full_node.py	4	86.85%

Totals
Change from base Build 18601081112:	-0.02%
Covered Lines:	102275
Relevant Lines:	112447

💛 - Coveralls

almogdepaz · 2025-10-19T21:55:39Z

chia/consensus/pot_iterations.py

    constants: ConsensusConstants,
-    quality_string: bytes32,
+    quality_string: bytes,
    size: PlotSize,


i think in the case of v1 we need to make sure its bytes32

I'm not sure what you're suggesting. the quality string is not 32 bytes for v2 plots, but it is for v1 plots. This function supports both, so it has to take bytes.
are you suggesting this function assert len(quality_string) == 32 for the case of v1 plots?

yes thats what im suggesting asserting the correct length for the v1 case

almogdepaz · 2025-10-19T22:10:49Z

chia/solver/solver_api.py


        try:
-            proof = self.solver.solve(request.partial_proof)
+            proof = self.solver.solve(request.partial_proof, request.plot_id, request.strength, request.size)


do we need the plot_id and size for solving ? (i couldn't find any mention of this in the channel)

you can know the size from the partial iirc

i have vague memory of us talking about encoding the strength in the partial did we decide not to ? (i prefer separating it from the partial as well)

yeah, the predictions we made about this API were mostly right. With the concrete implementation, it's just simpler to preserve the partial proof as a list and the strength as a separate variable. This format is not sensitive to size, as opposed to the full proof, which is included in full blocks. That's where we compact the proof and (currently) encode strength as well.

But there's a chance now that we'll lock down the k-size to just a single one, which means we can use the size-field on ProofOfSpace to store the strength for v2 plots, which simplifies the full-proof format.

but here as well size should correspond to the partial length right ?

we definitely need the plot ID. It's used for the ProofParams here: https://github.com/Chia-Network/pos2-chip/blob/main/src/api.cpp#L98
Looking over this again, I'm not sure we can deduce k from partial_proof. I think partial proof is always 64 integers. Each integer is at most k` bits though, but I don't think that's enough. This is the C-function:
https://github.com/Chia-Network/pos2-chip/blob/main/src/api.cpp#L91-L116

This is the rust function:
https://github.com/Chia-Network/pos2-chip/blob/main/crates/chia-pos2/src/lib.rs#L107-L126

almogdepaz · 2025-10-19T22:12:05Z

chia/types/blockchain_format/proof_of_space.py

-        required_plot_strength = calculate_required_plot_strength(constants, height)
-        return validate_proof_v2(plot_id, plot_size.size_v2, required_plot_strength, pos.challenge, bytes(pos.proof))
+        return validate_proof_v2(
+            plot_id, plot_size.size_v2, constants.PLOT_STRENGTH_INITIAL, pos.challenge, bytes(pos.proof)


what are we doing now for required strength ?
its supposed to be dependent on the current state from the chain right ?

this was discussed in a PoS call a few weeks ago. We don't need a pre-determined schedule for increasing strength, since those are soft-forks. We do need a schedule for tightening the plot-filter though, since changing that is a hard fork.

ok so the soft fork would just change the strength we are passing to validate ?

correct. It would make some plots ineligible for making proofs (ones with low stregnth)

almogdepaz · 2025-10-19T22:14:09Z

chia/protocols/solver_protocol.py

+    partial_proof: list[uint64]  # 16 * k bits blob, k (plot size) can be derived from this
+    plot_id: bytes32
+    strength: uint8
+    size: uint8  # k-size


why do we need size if partial proof is k length ?

you're right that we could (probably) calculate k by len(partial_proof)/16. It would make me a little bit uneasy to not specify the k-size explicitly. Would you prefer omitting k? There's a good chance we'll lock down k to be a constant as well, in which case we don't need it

i dont like having extra fields in our context that usually just means extra validation, meaning we would anyway need to assert that the size and the length match so i rather just drop the extra field if thats the case

After having another look at this, I think this comment is left-over and no longer right. I'll change it. The number of items in partial_proof is always 64. We can't (reliably) deduce k from it.

almogdepaz · 2025-10-19T22:24:14Z

chia/harvester/harvester_api.py

                        required_iters: uint64 = calculate_iterations_quality(
                            self.harvester.constants,
-                            quality_str,
+                            quality.get_quality(),


i think i would change the var name to quality_proof or something, i think its a bit strange to have quality.get_quality

I'll change it to get_string(). In the rust API I called it serialize(), but that only applies to the v2 version. The v1 quality is just the quality string, in serialized form.

almogdepaz · 2025-10-19T22:32:29Z

chia/harvester/harvester_api.py

+                    if required_iters >= sp_interval_iters:
+                        continue
+
+                    assert isinstance(plot_info.prover, V2Prover)


this should not be possible we are checking the type before calling
blocking_lookup_v2_partial_proofs

if we want to be defensive then i would put assert isinstance(plot_info.prover, V2Prover) at the start of blocking_lookup_v2_partial_proofs

i think assert isinstance(quality, V2Quality) is redundant especially if you validate the type of the prover

both of these asserts are primarily to appease mypy who can't tell that the prover is guaranteed to be V2Prover. I think all asserts are defensive and not supposed to be possible by their nature though.

mypy can't tell that just because the prover has the type V2Prover that also the qualities object it returned is necessarily V2Quality

…ocol

github-actions · 2025-10-20T22:23:54Z

File	Coverage	Missing Lines
`chia/_tests/harvester/test_harvester_api.py`	54.5%	lines 94-98
`chia/harvester/harvester_api.py`	40.0%	lines 165, 170, 188, 199-200, 202-203, 205-206
`chia/plotting/check_plots.py`	0.0%	lines 10, 12, 18, 180, 208, 212, 214, 217-219, 221-223
`chia/plotting/prover.py`	62.5%	lines 49, 62, 66, 69, 72, 75, 78, 81, 91, 94, 97, 100, 127, 157, 166
`chia/simulator/block_tools.py`	42.9%	lines 1516-1517, 1519-1520, 1524, 1548-1554
`chia/solver/solver_api.py`	75.0%	lines 58

Total	Missing	Coverage
177 lines	55 lines	68%

arvidn requested a review from a team as a code owner October 16, 2025 19:51

arvidn added the Added Required label for PR that categorizes merge commit message as "Added" for changelog label Oct 16, 2025

arvidn requested a review from almogdepaz October 16, 2025 19:59

github-actions bot added the coverage-diff label Oct 16, 2025

arvidn removed the coverage-diff label Oct 17, 2025

github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Oct 17, 2025

arvidn added 3 commits October 17, 2025 23:11

bump chia_rs to 0.31.0

6f810b3

integrate new pos2 API from chia_rs into V2Prover

3bb2434

fixup test_harvester_api mocking of Prover

e2d9c46

arvidn force-pushed the pos2-prover branch from 58b5ed9 to e2d9c46 Compare October 17, 2025 21:12

github-actions bot added coverage-diff and removed merge_conflict Branch has conflicts that prevent merge to main labels Oct 17, 2025

almogdepaz reviewed Oct 19, 2025

View reviewed changes

review-comments: rename get_quality() to get_string() on Quality prot…

c298736

…ocol

arvidn requested a review from almogdepaz October 20, 2025 10:24

reivew comments

3be47ed

PoS2-prover #20159

Are you sure you want to change the base?

PoS2-prover #20159

Conversation

arvidn commented Oct 16, 2025

Purpose:

Current Behavior:

New Behavior:

Uh oh!

arvidn commented Oct 17, 2025

Uh oh!

coveralls-official bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 18665417613

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

almogdepaz Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coveralls-official bot commented Oct 17, 2025 •

edited

Loading

almogdepaz Oct 19, 2025 •

edited

Loading