Skip to content

Add validator index with Withdrawal pb #11563

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.

Sign up for GitHub

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

Merged
prylabs-bulldozer merged 5 commits into from
Oct 22, 2022
Merged

Add validator index with Withdrawal pb #11563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9e3b12
Add validator index with Withdrawal pb
terencechain Oct 20, 2022
2946647
Update BUILD.bazel
terencechain Oct 20, 2022
8feb24d
Fix test
terencechain Oct 21, 2022
5c0895a
Better tests
terencechain Oct 21, 2022
3fb6bb3
Merge branch 'develop' into add-validator-index
terencechain Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions beacon-chain/state/state-native/setters_withdrawal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,27 @@ func TestSetWithdrawalQueue(t *testing.T) {
WithdrawalIndex: 0,
ExecutionAddress: []byte("address1"),
Amount: 1,
ValidatorIndex: 2,
},
{
WithdrawalIndex: 1,
ExecutionAddress: []byte("address2"),
Amount: 2,
ValidatorIndex: 3,
},
}
newQ := []*enginev1.Withdrawal{
{
WithdrawalIndex: 2,
ExecutionAddress: []byte("address3"),
Amount: 3,
ValidatorIndex: 4,
},
{
WithdrawalIndex: 3,
ExecutionAddress: []byte("address4"),
Amount: 4,
ValidatorIndex: 5,
},
}
s := BeaconState{
Expand All @@ -61,11 +65,13 @@ func TestAppendWithdrawal(t *testing.T) {
WithdrawalIndex: 0,
ExecutionAddress: []byte("address1"),
Amount: 1,
ValidatorIndex: 2,
}
oldWithdrawal2 := &enginev1.Withdrawal{
WithdrawalIndex: 1,
ExecutionAddress: []byte("address2"),
Amount: 2,
ValidatorIndex: 3,
}
q := []*enginev1.Withdrawal{oldWithdrawal1, oldWithdrawal2}
s := BeaconState{
Expand All @@ -80,6 +86,7 @@ func TestAppendWithdrawal(t *testing.T) {
WithdrawalIndex: 2,
ExecutionAddress: []byte("address3"),
Amount: 3,
ValidatorIndex: 4,
}
err := s.AppendWithdrawal(newWithdrawal)
require.NoError(t, err)
Expand Down
3 changes: 3 additions & 0 deletions proto/engine/v1/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ proto_library(
ssz_gen_marshal(
name = "ssz_generated_files",
go_proto = ":go_proto",
includes = [
"//consensus-types/primitives:go_default_library",
],
objs = [
"ExecutionPayload",
"ExecutionPayloadCapella",
Expand Down
58 changes: 37 additions & 21 deletions proto/engine/v1/execution_engine.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions proto/engine/v1/execution_engine.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,13 @@ message Withdrawal {
// Withdrawal index for accounting purposes
uint64 withdrawal_index = 1;

// Validator index for the withdrawal
uint64 validator_index = 2 [(ethereum.eth.ext.cast_type) = "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives.ValidatorIndex"];

// The execution address receiving the funds
bytes execution_address = 2 [(ethereum.eth.ext.ssz_size) = "20"];
bytes execution_address = 3 [(ethereum.eth.ext.ssz_size) = "20"];

// The withdrawn amount in Gwei
uint64 amount = 3;
uint64 amount = 4;
}
Comment on lines +135 to 143
Copy link
Member

@prestonvanloon prestonvanloon Oct 21, 2022

Choose a reason for hiding this comment

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

Do not change IDs. This is not going to be backwards compatible with the previous version.

Copy link
Contributor

@potuz potuz Oct 21, 2022

Choose a reason for hiding this comment

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

what do you mean? there's no previous version, the spec is in flux

Copy link
Member

@prestonvanloon prestonvanloon Oct 21, 2022

Choose a reason for hiding this comment

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

What I mean is that we should not be changing proto IDs. In this case, it seems like we can call it out as something that has not be implemented yet so it is safe to change, but it has to be explicit so that we don't develop the habit of changing proto IDs like this.


42 changes: 26 additions & 16 deletions proto/engine/v1/generated.ssz.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions proto/prysm/v1alpha1/cloners.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ func CopyWithdrawal(withdrawal *enginev1.Withdrawal) *enginev1.Withdrawal {

return &enginev1.Withdrawal{
WithdrawalIndex: withdrawal.WithdrawalIndex,
ValidatorIndex: withdrawal.ValidatorIndex,
ExecutionAddress: bytesutil.SafeCopyBytes(withdrawal.ExecutionAddress),
Amount: withdrawal.Amount,
}
Expand Down
1 change: 1 addition & 0 deletions proto/prysm/v1alpha1/cloners_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ func genWithdrawals(num int) []*enginev1.Withdrawal {
func genWithdrawal() *enginev1.Withdrawal {
return &enginev1.Withdrawal{
WithdrawalIndex: 123456,
ValidatorIndex: 654321,
ExecutionAddress: bytes(20),
Amount: 55555,
}
Expand Down
10 changes: 5 additions & 5 deletions proto/prysm/v1alpha1/generated.ssz.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.