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

Cleanup & Fix Simulator Sub Epoch Logic #15698

Merged
merged 3 commits into from
Jul 15, 2023
Merged

Cleanup & Fix Simulator Sub Epoch Logic #15698

merged 3 commits into from
Jul 15, 2023

Conversation

jack60612
Copy link
Contributor

@jack60612 jack60612 commented Jul 6, 2023

Purpose:

This fixes an issue where a sub epoch might not be included if the simulator decides to overflow blocks, as it checks if there is a difficulty change, and therefore a new epoch instead of a sub epoch

Current Behavior:

No Breaking Change

New Behavior:

N/A

Testing Notes:

Testing is borderline impossible, because it requires making hundreds of thousands of blocks, however @nirajpathak13 said the issue was fixed.

Crash log before fix:

(venv) niraj@summer:~/development/chia/chia-blockchain$ Traceback (most recent call last):
  File "/home/niraj/development/chia/chia-blockchain/venv/bin/chia_full_node", line 8, in <module>
    sys.exit(main())
  File "/home/niraj/development/chia/chia-blockchain/chia/server/start_full_node.py", line 117, in main
    return async_run(coro=async_main(service_config), connection_limit=target_peer_count)
  File "/home/niraj/development/chia/chia-blockchain/chia/server/start_service.py", line 319, in async_run
    return asyncio.run(coro)
  File "/usr/lib/python3.8/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/home/niraj/development/chia/chia-blockchain/chia/server/start_full_node.py", line 100, in async_main
    await service.run()
  File "/home/niraj/development/chia/chia-blockchain/chia/server/start_service.py", line 224, in run
    await self.start()
  File "/home/niraj/development/chia/chia-blockchain/chia/server/start_service.py", line 183, in start
    await self._node._start()
  File "/home/niraj/development/chia/chia-blockchain/chia/full_node/full_node.py", line 401, in _start
    await self.peak_post_processing_2(full_peak, None, state_change_summary, ppp_result)
  File "/home/niraj/development/chia/chia-blockchain/chia/full_node/full_node.py", line 1522, in peak_post_processing_2
    await self.send_peak_to_timelords(block)
  File "/home/niraj/development/chia/chia-blockchain/chia/full_node/full_node.py", line 731, in send_peak_to_timelords
    ses: Optional[SubEpochSummary] = next_sub_epoch_summary(
  File "/home/niraj/development/chia/chia-blockchain/chia/consensus/make_sub_epoch_summary.py", line 198, in next_sub_epoch_summary
    return make_sub_epoch_summary(
  File "/home/niraj/development/chia/chia-blockchain/chia/consensus/make_sub_epoch_summary.py", line 68, in make_sub_epoch_summary
    uint8(curr.height % constants.SUB_EPOCH_BLOCKS),
  File "/home/niraj/development/chia/chia-blockchain/chia/util/struct_stream.py", line 73, in __init__
    raise ValueError(f"Value {self} does not fit into {type(self).__name__}")
ValueError: Value 335 does not fit into uint8

@jack60612 jack60612 added simulator Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog labels Jul 6, 2023
@jack60612 jack60612 marked this pull request as ready for review July 6, 2023 19:38
@jack60612 jack60612 requested a review from a team as a code owner July 6, 2023 19:38
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

it looks correct, but I'm not very familiar with this code. I think leaving some comments explaining why the previous code was wrong (perhaps by explaining why the new code is correct) would be helpful

chia/simulator/block_tools.py Show resolved Hide resolved
chia/simulator/block_tools.py Show resolved Hide resolved
@jack60612 jack60612 requested a review from arvidn July 7, 2023 21:34
@wallentx wallentx merged commit 5d1d328 into main Jul 15, 2023
@wallentx wallentx deleted the jn.simulator-sub-epoch branch July 15, 2023 23:18
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 simulator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants