fix wp error on failed sync #18883
fix wp error on failed sync #18883
Conversation
almogdepaz
added
full_node
sync
Fixed
| @@ -1686,10 +1690,6 @@ async def _finish_sync(self) -> None: | |||
| ) | |||
| await self.peak_post_processing_2(peak_fb, None, state_change_summary, ppp_result) | |||
|
|
|||
| if peak is not None and self.weight_proof_handler is not None: | |||
| await self.weight_proof_handler.get_proof_of_weight(peak.header_hash) | |||
| self._state_changed("block") | |||
There was a problem hiding this comment.
is it safe to remove this _state_changed() call?
There was a problem hiding this comment.
from what i can tell (and its not very easy because of how the state change code is written) this is for metrics that we are collecting
chia-blockchain/chia/rpc/full_node_rpc_api.py
Line 155 in f7897a6
and actually i dont understand why we are collecting this, we have metrics collection for new_peak and a change in sync_mode and both happen when we finish sync
There was a problem hiding this comment.
my understanding is that _state_changed is used to notify the GUI of things it need to refresh
| peak = self.blockchain.get_peak() | ||
| assert peak is not None | ||
| if self.weight_proof_handler is not None: | ||
| await self.weight_proof_handler.get_proof_of_weight(peak.header_hash) |
There was a problem hiding this comment.
it looks a bit odd to call a function called get_... and then throw away the return value. I imagine it, perhaps, updates some cache. I think it would mak sense to explain that in a comment (or whatever the reason is).
I also think it would be good to comment on why we need this call here, and not in _finish_sync(). To avoid someone moving it back in the future. Especially since we don't have a test that would fail if someone did.
There was a problem hiding this comment.
i dont undestand the finish_sync() on failure at all actually i think we can do that better, anyway now that i have a better understanding of the DB corruption issue i dont think we need this change
|
closed since the underlining issue was due to a problem with the data in the db not us creating the WP on the current peak after a faild sync |
Purpose:
code clean up, don't attempt weight proof creation at the end of failed syncs
Current Behavior:
even if we throw during sync we will try and create a weight proof for the peak
New Behavior:
only create the weight proof if the sync ends successfully
Testing Notes: