Skip to content

simplify get_seed_for_proof() #20299

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.

Sign up for GitHub

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

Closed
arvidn wants to merge 1 commit into from
Closed

simplify get_seed_for_proof() #20299

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions chia/full_node/weight_proof.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ async def _create_proof_of_weight(self, tip: bytes32) -> WeightProof | None:
return None

summary_heights = self.blockchain.get_ses_heights()
if len(summary_heights) <= 1:
log.error("failed not enough sub epochs")
Copy link
Contributor

@almogdepaz almogdepaz Nov 27, 2025

Choose a reason for hiding this comment

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

get_seed_for_proof expects at least two sub epochs, it will throw without this.
we cannot get a sees if we dont have 2 finished sub epochs at least

we cannot change how the seed is chosen since it is verified by the verifier so we can change this without being incompatible

Copy link
Contributor Author

@arvidn arvidn Dec 1, 2025

Choose a reason for hiding this comment

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

I'm not sure what you mean by this comment. It is possible to call this function with a height that only yields a single summary height. The existing behavior is to throw a KeyError exception in this case. It took some investigation to understand what that meant. I would think this is an improvement, as a clearer error is printed to the log. I'm pretty confident returning None has essentially the same effect as the exception, we fail to respond to this weight proof request either way.

Copy link
Contributor

@almogdepaz almogdepaz Dec 1, 2025

Choose a reason for hiding this comment

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

i think this error should be thrown from inside get_seed_for_proof

i missed that this is supposed to be equivalent to the current code (i think its less readable but maybe im just familiar with the old) i always prefer not fixing something thats not broken especially critical code, we can add the error there instead of the assert thats fine

Copy link
Contributor Author

@arvidn arvidn Dec 1, 2025

Choose a reason for hiding this comment

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

yeah, it's a good point that it's always risky to change code that works. I think this especially applies to get_seed_for_proof(). I'll propose a less intrusive change to that function.

Regarding the error reporting here. Is this case materially different from the other failure cases here, to warrant an exception instead of returning None?

For example, if self.blockchain.try_block_record(tip) or self._get_recent_chain(tip_rec.height) returns None.

return None
zero_hash = self.blockchain.height_to_hash(uint32(0))
assert zero_hash is not None
prev_ses_block = await self.blockchain.get_block_record_from_db(zero_hash)
Expand All @@ -138,6 +141,7 @@ async def _create_proof_of_weight(self, tip: bytes32) -> WeightProof | None:
ses_blocks = await self.blockchain.get_block_records_at(summary_heights)
if ses_blocks is None:
return None
assert len(ses_blocks) == len(summary_heights)

for sub_epoch_n, ses_height in enumerate(summary_heights):
if ses_height > tip_rec.height:
Expand Down Expand Up @@ -171,17 +175,8 @@ async def _create_proof_of_weight(self, tip: bytes32) -> WeightProof | None:
return WeightProof(sub_epoch_data, sub_epoch_segments, recent_chain)

def get_seed_for_proof(self, summary_heights: list[uint32], tip_height: uint32) -> bytes32:
count = 0
ses = None
for sub_epoch_n, ses_height in enumerate(reversed(summary_heights)):
if ses_height <= tip_height:
count += 1
if count == 2:
ses = self.blockchain.get_ses(ses_height)
break
assert ses is not None
seed = ses.get_hash()
return seed
ses_height = list(filter(lambda h: h <= tip_height, reversed(summary_heights)))[1]
return self.blockchain.get_ses(ses_height).get_hash()

async def _get_recent_chain(self, tip_height: uint32) -> list[HeaderBlock] | None:
recent_chain: list[HeaderBlock] = []
Expand Down
Loading