-
Notifications
You must be signed in to change notification settings - Fork 32
Add wrong-slot STF test with filler support #161
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
Add wrong-slot STF test with filler support #161
Conversation
| This is a private attribute not part of the model schema. | ||
| Tests cannot set this. The framework populates it during make_fixture(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is no change here, can we revert this to minimize git diff? I guess this is the same at other places in this PR :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about this I got a Ruffβs line-length lint so had to split to a new line
| Builds blocks from BlockSpec if needed, then processes them through | ||
| state_transition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about minimizing git diff
| TODO: If the spec implements a State.produce_block() method in the | ||
| future, we should use that instead of manually computing fields here. | ||
| Until then, this manual approach is necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same message about minimizing git diff
| if getattr(block_spec, "skip_slot_processing", False): | ||
| state = state.process_block(block) | ||
| else: | ||
| state = state.state_transition( | ||
| block=block, | ||
| valid_signatures=True, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why we need all this here and why we need this new skip_slot_processing, looks like confusing and I guess we want to test the classical state_transition processing here because the block processing is the first step of it
leanSpec/src/lean_spec/subspecs/containers/state/state.py
Lines 521 to 522 in 715276e
| # First, process any intermediate slots. | |
| state = self.process_slots(block.slot) |
so that we should see the exact same error that if we do state.process_block(block) because the only additional step which happens before this in the state transition processing is to check that valid_signatures is true but this is the case here in this call so there should be no problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reason the skip_slot_processing flag exists is to make the new βwrong slotβ vector possible. state_transition() always advances through process_slots(block.slot) before running process_block, so by the time the header check runs the state slot already equals the block slot and assert block.slot == self.slot can never trip. The flag simply lets an individual test opt out of that automatic advancement so we can feed the block to process_block while the state is still at the earlier slot. Everything else continues through the normal state_transition path; no existing tests are touched. If youβd prefer an alternative hook (e.g., a dedicated helper to run process_block directly), Iβm happy to rework it the key requirement is just some way to bypass that initial process_slots call for this negative test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes true I get it, can you solve conflicts?
|
Hey @bomanaps I think that we have some conflicts with recent updates and then looks good, we should just try to minimize git diff here especially with the comments if possible. |
b572062 to
12810de
Compare
ποΈ Description
Add skip slot flag to state transition filler and implement consensus test rejecting blocks with mismatched slots.
π Related Issues or PRs
#120
β Checklist
toxchecks to avoid unnecessary CI fails:uvx tox