-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat/snapshot-eip-4881 #377
Conversation
|
If you were trying to commit your work from here, it did not make it into 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.
Looks like a good start. A few comments:
Instead of redefining MerkleTree, Leaf, Node, etc here, please reuse the existing infra from this library. That means using the existing Tree
, LeafNode
, BranchNode
constructs. You should be able to define your custom tree editing / navigation functions either purely functionally, or in a wrapper class.
Eventually, this code / PR should be migrated to the lodestar repo and live there. Whether you want to keep this review here for now is up to you.
Thanks so much for your feedback , I will start resolving the linting
errors and also start to run successfully tests .
Regards ,
Akshat
…On Fri, Jun 14, 2024, 7:29 PM Cayman ***@***.***> wrote:
***@***.**** commented on this pull request.
Looks like a good start. A few comments:
Instead of redefining MerkleTree, Leaf, Node, etc here, please reuse the
existing infra from this library. That means using the existing Tree,
LeafNode, BranchNode constructs. You should be able to define your custom
tree editing / navigation functions either purely functionally, or in a
wrapper class.
Eventually, this code / PR should be migrated to the lodestar repo and
live there. Whether you want to keep this review here for now is up to you.
—
Reply to this email directly, view it on GitHub
<#377 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AXDLB6TCYRZXENTGTDUAJHDZHLZKVAVCNFSM6AAAAABJCDVGMKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMJYGQ3DOOJYGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hi @AkshatGada , do you still plan to keep working on this? |
Yes , I am a little busy with college and my internship , I am running tests locally , please grant me some time to get it done . |
I am planning to do it as a project for the ethereum protocol fellowship. |
Superseded by #400 |
Attempts to solve issue ChainSafe/lodestar#4935
waiting for your review