Skip to content

Fix bouncing attack tests#2301

Merged
djrtwo merged 7 commits intodevfrom
fix-bouncing-attack-tests
Apr 27, 2021
Merged

Fix bouncing attack tests#2301
djrtwo merged 7 commits intodevfrom
fix-bouncing-attack-tests

Conversation

@adiasg
Copy link
Copy Markdown
Contributor

@adiasg adiasg commented Apr 4, 2021

Fixes #2298

@hwwhww hwwhww added the testing CI, actions, tests, testing infra label Apr 5, 2021
Copy link
Copy Markdown
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

one minor question

# Mock fictitious justified checkpoint in store
store.justified_checkpoint = spec.Checkpoint(
epoch=spec.compute_epoch_at_slot(last_signed_block.message.slot),
root=spec.Root("0x4a55535449464945440000000000000000000000000000000000000000000000")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not use htr of last_block_root for justified_checkpoint.root?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We want to trigger this piece of code from should_update_justified_checkpoint:

if not get_ancestor(store, new_justified_checkpoint.root, justified_slot) == store.justified_checkpoint.root:
        return False

If we use last_block_root & spec.compute_epoch_at_slot(last_signed_block.message.slot) to construct store.justified_checkpoint, then the newly constructed blocks will not trigger this, and should_update_justified_checkpoint returns True because the new justified checkpoint is a descendant of the current one in the store.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Filling in a fictitious root ensures that this get_ancestor check fails and should_update_justified_checkpoint returns False, creating the test scenario we want

@djrtwo djrtwo merged commit 6031417 into dev Apr 27, 2021
@djrtwo djrtwo deleted the fix-bouncing-attack-tests branch April 27, 2021 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some block processing unit tests force processing of inconsistent blocks

3 participants