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

style: Block::is_valid_internal #293

Open
Sword-Smith opened this issue Dec 19, 2024 · 0 comments
Open

style: Block::is_valid_internal #293

Sword-Smith opened this issue Dec 19, 2024 · 0 comments
Assignees
Labels
mainnet issues that must be resolved before launching mainnet testing

Comments

@Sword-Smith
Copy link
Member

Sword-Smith commented Dec 19, 2024

This function is our central consensus-function and shouldn't be changed light heartedly.

Cf. #215, we want negative test of as many of the modes of failure of this function. So the function should return error codes, not just a boolean as it does now, in order to increase our confidence that the expected code paths are taken.

Also: The function is a bit long. So we could factor out each check with an appropriately named predicate-function or error-code-returning functions. Then we could get rid of the comments in that function.

@Sword-Smith Sword-Smith changed the title style: is_valid_internal style: Block::is_valid_internal Dec 19, 2024
@Sword-Smith Sword-Smith added the mainnet issues that must be resolved before launching mainnet label Jan 13, 2025
Sword-Smith added a commit that referenced this issue Jan 13, 2025
Fix a negative test that verifies that the block validity function fails
if the the `block_mmr_accumulator` value is not set correctly. Prior to
the mutations that invalidate the block, we verify that the block is
valid.

Also: This test only works because we verify the `block_mmr_accumulator`
prior to verifying the block proof, since the block proof is invalidated
by the new block_mmr_accumulator field value (since this modification
changes the block's claim). To address this problem (of not knowing what
you're testing in the negative tests), #293 must be solved sooner rather
than later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mainnet issues that must be resolved before launching mainnet testing
Projects
None yet
Development

No branches or pull requests

2 participants