Skip to content

Bump chia rs #19615

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Bump chia rs #19615

wants to merge 2 commits into from

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented May 19, 2025

Purpose:

update chia_rs to get an updated ConsensusConstatnts that includes the hard fork related constants. as well as an update ProofOfSpace that defines the protocol for how to identify v2 plots.

The blockchain still only allows v1 plots and the hard for is currently set to 0xffffffff as a placeholder.

@arvidn arvidn requested a review from almogdepaz May 19, 2025 12:52
@arvidn arvidn added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label May 19, 2025
32,
)
await _validate_and_add_block(empty_blockchain, block_bad, expected_error=Err.INVALID_POSPACE)
block_bad = recursive_replace(
blocks[-1],
"reward_chain_block.proof_of_space.proof",
bytes([1] * int(blocks[-1].reward_chain_block.proof_of_space.size * 64 / 8)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test looks confused. the size field is the k-value for the plot. It does not indicate how many bytes the proof is.

Copy link
Contributor

File Coverage Missing Lines
chia/consensus/block_header_validation.py 66.7% lines 509
chia/types/blockchain_format/proof_of_space.py 63.3% lines 26, 54-58, 60, 82-85
Total Missing Coverage
56 lines 12 lines 78%

@@ -375,7 +375,7 @@ def prepare_sp_and_pos_for_fee_test(
pool_public_key=None,
pool_contract_puzzle_hash=None,
plot_public_key=pubkey,
size=uint8(len(proof)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this test also was confused about the size field

@@ -25,7 +25,7 @@
"pool_public_key": "0xa04c6b5ac7dfb935f6feecfdd72348ccf1d4be4fe7e26acf271ea3b7d308da61e0a308f7a62495328a81f5147b66634c",
"pool_contract_puzzle_hash": None,
"plot_public_key": "0xb6449c2c68df97c19e884427e42ee7350982d4020571ead08732615ff39bd216bfd630b6460784982bec98b49fea79d0",
"size": 204,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll work on restoring this JSON serialization, to stay backwards compatible

@almogdepaz almogdepaz mentioned this pull request May 19, 2025
@@ -2866,7 +2866,7 @@ async def test_invalid_coin_spend_coin(
new_bundle = recursive_replace(spend_bundle, "coin_spends", [coin_spend_0, *spend_bundle.coin_spends[1:]])
assert spend_bundle is not None
res = await full_node_1.full_node.add_transaction(new_bundle, new_bundle.name(), test=True)
assert res == (MempoolInclusionStatus.FAILED, Err.INVALID_SPEND_BUNDLE)
assert res == (MempoolInclusionStatus.FAILED, Err.WRONG_PUZZLE_HASH)
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems suspicious did the test change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the mempool, currently, validates the puzzle reveal against the puzzle hash really late, here: https://github.com/Chia-Network/chia-blockchain/blob/main/chia/full_node/mempool_manager.py#L635-L637

With the new chia_rs we check it early, before running the puzzle. And we have a more precise error code.

PLOT_FILTER_128_HEIGHT=uint32(10542000),
PLOT_FILTER_64_HEIGHT=uint32(15592000),
PLOT_FILTER_32_HEIGHT=uint32(20643000),
PLOT_DIFFICULTY_4_HEIGHT=uint32(0xFFFFFFFF),
PLOT_DIFFICULTY_5_HEIGHT=uint32(0xFFFFFFFF),
PLOT_DIFFICULTY_6_HEIGHT=uint32(0xFFFFFFFF),
Copy link
Contributor

Choose a reason for hiding this comment

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

what are these for ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new concept for proof-of-space v2. My understanding is that it's a little bit like the plot filter, but perhaps more granular. Over time, we increase the difficulty, to account for hardware improving efficiency to grind. So, this is a schedule of block heights of when it's increased.

# the puzzle. This test probably needs to be updated
# assert result == (MempoolInclusionStatus.FAILED, Err.ASSERT_ANNOUNCE_CONSUMED_FAILED)

assert result == (MempoolInclusionStatus.FAILED, Err.WRONG_PUZZLE_HASH)
Copy link
Contributor

Choose a reason for hiding this comment

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

same question as the other test return value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I think this test is trying to test something else, and used to rely on the puzzle reveal being validated last

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.

2 participants