Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
EL-triggered consolidations #3775
EL-triggered consolidations #3775
Changes from 17 commits
1292bd9
204b39d
74eaf57
8a6ca1c
e030f2c
901a249
c492d61
17c5148
7c4b32a
dc2a2bd
5998e74
c17f22f
6b69368
f0ef76a
1970b56
6a731e9
96db63e
ffebf88
143b9e6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
test case idea: have a validator get partial withdrawal and then process its consolidation at the same block.
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.
would it be more straightforward to use
if request_source_pubkey == request_target_pubkey
here?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.
I don't have a strong opinion here. Whatever we choose, we do use
source_index
andtarget_index
later and it probably makes sense to define themThere 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.
@hwwhww it is great to see that the consolidation of validators with different withdrawal addresses is possible now.
Was there any reason why it was not allowed initially (why ever this constraint has existed?), but became possible by 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.
It was not allowed initially because initially consolidation was authorized by the validator’s pubkey which isn’t secure enough to change the owner (withdrawal creds) of the funds. In order to make this possible, a consolidation request system smart contract has been introduced on EL with the corresponding changes on CL to make it possible to sign requests with withdrawal creds which unlocked moving funds between validators with different creds
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.
Awesome. Appreciate the details 👍