Skip to content

Add attestation simulator#4880

Merged
paulhauner merged 48 commits intosigp:unstablefrom
v4lproik:attestation-simulator-performance-metric
Dec 14, 2023
Merged

Add attestation simulator#4880
paulhauner merged 48 commits intosigp:unstablefrom
v4lproik:attestation-simulator-performance-metric

Conversation

@v4lproik
Copy link
Copy Markdown
Contributor

@v4lproik v4lproik commented Oct 25, 2023 •
edited
Loading

@v4lproik v4lproik changed the title basic scaffold wip: add attestation simulator Oct 25, 2023
@chong-he chong-he added the work-in-progress PR is a work-in-progress label Oct 26, 2023
paulhauner
paulhauner reviewed Nov 2, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Heyo, just providing some initial feedback in response to #4526 (comment) ☺️

Looking good so far, I don't think there's that much left to do!

@v4lproik v4lproik marked this pull request as ready for review November 10, 2023 01:53
@v4lproik
Copy link
Copy Markdown
Contributor Author

v4lproik commented Dec 12, 2023 •
edited
Loading

fmt, lint and test-beacon-chain are green on my side! 🚀 🙏🏼

@paulhauner paulhauner added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Dec 12, 2023
@paulhauner paulhauner changed the base branch from stable to unstable December 13, 2023 00:20
paulhauner
paulhauner requested changes Dec 13, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

I'm running this on our Holesky nodes and it's working great! I have another suggestion please 🙏

Also, I noticed it was targeting stable when it should target unstable (I wish we could make unstable the default PR target, but alas, we cannot 😔). There are some minor conflicts, but I fixed them here during my testing.

Also, it's not an issue but I wanted to point out that ff9b6b5 does not dereference. |var| *var and |&var| var are equivalent, you can read more about them here. It's the .cloned() you used that does the dereferencing.

v4lproik and others added 2 commits December 13, 2023 07:26
@paulhauner
Copy link
Copy Markdown
Member

paulhauner commented Dec 13, 2023

Looking good! I'll let the tests run before I slap an approval on it.

I've been running this on a Holesky node overnight, it's working well. Here's the last 24hrs:

Screenshot 2023-12-14 at 9 22 18 am

paulhauner
paulhauner approved these changes Dec 14, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

All tests passed 🎉 Let's get this merged!

@paulhauner paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Dec 14, 2023
@paulhauner paulhauner merged commit 189430a into sigp:unstable Dec 14, 2023
This was referenced Jan 9, 2024
Closed
Closed
@chenrui333 chenrui333 mentioned this pull request Jan 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulhauner paulhauner paulhauner approved these changes

Assignees

@paulhauner paulhauner

Labels

ready-for-merge This PR is ready to merge.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@v4lproik @paulhauner @chong-he