Skip to content

Add new process pending deposits test#4101

Merged
jtraglia merged 3 commits into
ethereum:devfrom
jtraglia:new-pending-deposits-test
Jan 28, 2025
Merged

Add new process pending deposits test#4101
jtraglia merged 3 commits into
ethereum:devfrom
jtraglia:new-pending-deposits-test

Conversation

@jtraglia

@jtraglia jtraglia commented Jan 27, 2025

Copy link
Copy Markdown
Member

This PR adds a new test which would have caught a bug in Prysm. From @terencechain:

I have a good spec test case that was triggered by devnet5 this morning and caused Prysm to fork off. The test case is relatively simple:

  • There are two pending deposits in the state, both pointing to the same public key
  • The public key does not exist in the beacon state
  • The first pending deposit has an invalid signature, while the second has a valid signature

Pretty simple. This test should do exactly this.

I've also added a third deposit on Terence's advice:

If we want, we could add [third pending deposit] with the same amount or different amount and a make sure balance amount is correct

@jtraglia jtraglia added the testing CI, actions, tests, testing infra label Jan 27, 2025

@mkalinin mkalinin left a comment

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.

LGTM!

@jtraglia jtraglia merged commit 663f2d3 into ethereum:dev Jan 28, 2025
@jtraglia jtraglia deleted the new-pending-deposits-test branch January 28, 2025 14:16
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.

3 participants