From 1ada0439c9739f534f042e484dc76141e25b196a Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 7 Nov 2022 11:18:00 -0300 Subject: [PATCH 1/8] implement process_withdrawals --- beacon-chain/core/blocks/withdrawals.go | 40 ++ beacon-chain/core/blocks/withdrawals_test.go | 387 +++++++++++++++++++ beacon-chain/state/interfaces.go | 1 + encoding/bytesutil/bytes.go | 9 + encoding/bytesutil/bytes_test.go | 8 + 5 files changed, 445 insertions(+) diff --git a/beacon-chain/core/blocks/withdrawals.go b/beacon-chain/core/blocks/withdrawals.go index 92c0f89ac730..02700c367259 100644 --- a/beacon-chain/core/blocks/withdrawals.go +++ b/beacon-chain/core/blocks/withdrawals.go @@ -3,11 +3,14 @@ package blocks import ( "bytes" + "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/signing" "github.com/prysmaticlabs/prysm/v3/beacon-chain/state" "github.com/prysmaticlabs/prysm/v3/config/params" "github.com/prysmaticlabs/prysm/v3/crypto/hash/htr" "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" + enginev1 "github.com/prysmaticlabs/prysm/v3/proto/engine/v1" ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v3/time/slots" ) @@ -76,3 +79,40 @@ func ProcessBLSToExecutionChange(st state.BeaconState, signed *ethpb.SignedBLSTo err = st.UpdateValidatorAtIndex(message.ValidatorIndex, val) return st, err } + +func ProcessWithdrawals(st state.BeaconState, withdrawals []*enginev1.Withdrawal) (state.BeaconState, error) { + expected, err := st.ExpectedWithdrawals() + if err != nil { + return nil, errors.Wrap(err, "could not get expected withdrawals") + } + if len(expected) != len(withdrawals) { + return nil, errors.New("invalid number of withdrawals") + } + for i, withdrawal := range withdrawals { + if withdrawal.WithdrawalIndex != expected[i].WithdrawalIndex { + return nil, errors.New("invalid withdrawal index") + } + if withdrawal.ValidatorIndex != expected[i].ValidatorIndex { + return nil, errors.New("invalid validator index") + } + if bytesutil.ToBytes20(withdrawal.ExecutionAddress) != bytesutil.ToBytes20(expected[i].ExecutionAddress) { + return nil, errors.New("invalid execution address") + } + if withdrawal.Amount != expected[i].Amount { + return nil, errors.New("invalid withdrawal amount") + } + err := helpers.DecreaseBalance(st, withdrawal.ValidatorIndex, withdrawal.Amount) + if err != nil { + return nil, errors.Wrap(err, "could not decrease balance") + } + } + if len(withdrawals) > 0 { + if err := st.SetNextWithdrawalIndex(withdrawals[len(withdrawals)-1].WithdrawalIndex + 1); err != nil { + return nil, errors.Wrap(err, "could not set withdrawal index") + } + if err := st.SetLastWithdrawalValidatorIndex(withdrawals[len(withdrawals)-1].ValidatorIndex); err != nil { + return nil, errors.Wrap(err, "could not set latest withdrawal validator index") + } + } + return st, nil +} diff --git a/beacon-chain/core/blocks/withdrawals_test.go b/beacon-chain/core/blocks/withdrawals_test.go index 30a83c1283d9..648a28d20fce 100644 --- a/beacon-chain/core/blocks/withdrawals_test.go +++ b/beacon-chain/core/blocks/withdrawals_test.go @@ -1,18 +1,23 @@ package blocks_test import ( + "math/rand" "testing" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/blocks" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/signing" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/time" + "github.com/prysmaticlabs/prysm/v3/beacon-chain/state" state_native "github.com/prysmaticlabs/prysm/v3/beacon-chain/state/state-native" "github.com/prysmaticlabs/prysm/v3/config/params" + types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v3/crypto/bls" "github.com/prysmaticlabs/prysm/v3/crypto/hash/htr" "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" + enginev1 "github.com/prysmaticlabs/prysm/v3/proto/engine/v1" ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v3/testing/require" + "github.com/prysmaticlabs/prysm/v3/time/slots" ) func TestProcessBLSToExecutionChange(t *testing.T) { @@ -191,3 +196,385 @@ func TestProcessBLSToExecutionChange(t *testing.T) { }) } + +func TestProcessWithdrawals(t *testing.T) { + const ( + currentEpoch = types.Epoch(10) + epochInFuture = types.Epoch(12) + epochInPast = types.Epoch(8) + numValidators = 128 + notWithdrawableIndex = 127 + notPartiallyWithdrawable = 126 + ) + maxEffectiveBalance := params.BeaconConfig().MaxEffectiveBalance + + type args struct { + Name string + LastWithdrawalValidatorIndex types.ValidatorIndex + NextWithdrawalIndex uint64 + FullWithdrawalIndices []types.ValidatorIndex + PartialWithdrawalIndices []types.ValidatorIndex + Withdrawals []*enginev1.Withdrawal + } + type control struct { + LastWithdrawalValidatorIndex types.ValidatorIndex + NextWithdrawalIndex uint64 + ExpectedError bool + Balances map[uint64]uint64 + } + type Test struct { + Args args + Control control + } + executionAddress := func(i types.ValidatorIndex) []byte { + wc := make([]byte, 20) + wc[19] = byte(i) + return wc + } + withdrawalAmount := func(i types.ValidatorIndex) uint64 { + return maxEffectiveBalance + uint64(i)*100000 + } + fullWithdrawal := func(i types.ValidatorIndex, idx uint64) *enginev1.Withdrawal { + return &enginev1.Withdrawal{ + WithdrawalIndex: idx, + ValidatorIndex: i, + ExecutionAddress: executionAddress(i), + Amount: withdrawalAmount(i), + } + } + partialWithdrawal := func(i types.ValidatorIndex, idx uint64) *enginev1.Withdrawal { + return &enginev1.Withdrawal{ + WithdrawalIndex: idx, + ValidatorIndex: i, + ExecutionAddress: executionAddress(i), + Amount: withdrawalAmount(i) - maxEffectiveBalance, + } + } + tests := []Test{ + { + Args: args{ + Name: "success no withdrawals", + LastWithdrawalValidatorIndex: 10, + NextWithdrawalIndex: 3, + }, + Control: control{ + LastWithdrawalValidatorIndex: 10, + NextWithdrawalIndex: 3, + }, + }, + { + Args: args{ + Name: "success one full withdrawal", + NextWithdrawalIndex: 3, + LastWithdrawalValidatorIndex: 5, + FullWithdrawalIndices: []types.ValidatorIndex{1}, + Withdrawals: []*enginev1.Withdrawal{ + fullWithdrawal(1, 3), + }, + }, + Control: control{ + LastWithdrawalValidatorIndex: 1, + NextWithdrawalIndex: 4, + Balances: map[uint64]uint64{1: 0}, + }, + }, + { + Args: args{ + Name: "success one partial withdrawal", + NextWithdrawalIndex: 21, + LastWithdrawalValidatorIndex: 37, + PartialWithdrawalIndices: []types.ValidatorIndex{7}, + Withdrawals: []*enginev1.Withdrawal{ + partialWithdrawal(7, 21), + }, + }, + Control: control{ + LastWithdrawalValidatorIndex: 7, + NextWithdrawalIndex: 22, + Balances: map[uint64]uint64{7: maxEffectiveBalance}, + }, + }, + { + Args: args{ + Name: "success many full withdrawals", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 4, + FullWithdrawalIndices: []types.ValidatorIndex{7, 19, 28, 1}, + Withdrawals: []*enginev1.Withdrawal{ + fullWithdrawal(7, 22), fullWithdrawal(19, 23), fullWithdrawal(28, 24), + fullWithdrawal(1, 25), + }, + }, + Control: control{ + LastWithdrawalValidatorIndex: 1, + NextWithdrawalIndex: 26, + Balances: map[uint64]uint64{7: 0, 19: 0, 28: 0, 1: 0}, + }, + }, + { + Args: args{ + Name: "success many partial withdrawals", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 4, + PartialWithdrawalIndices: []types.ValidatorIndex{7, 19, 28, 1}, + Withdrawals: []*enginev1.Withdrawal{ + partialWithdrawal(7, 22), partialWithdrawal(19, 23), partialWithdrawal(28, 24), + partialWithdrawal(1, 25), + }, + }, + Control: control{ + LastWithdrawalValidatorIndex: 1, + NextWithdrawalIndex: 26, + Balances: map[uint64]uint64{ + 7: maxEffectiveBalance, + 19: maxEffectiveBalance, + 28: maxEffectiveBalance, + 1: maxEffectiveBalance, + }, + }, + }, + { + Args: args{ + Name: "success many withdrawals", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 12, + FullWithdrawalIndices: []types.ValidatorIndex{7, 19, 28}, + PartialWithdrawalIndices: []types.ValidatorIndex{2, 1, 89, 15}, + Withdrawals: []*enginev1.Withdrawal{ + partialWithdrawal(15, 22), fullWithdrawal(19, 23), fullWithdrawal(28, 24), + partialWithdrawal(89, 25), partialWithdrawal(1, 26), partialWithdrawal(2, 27), + fullWithdrawal(7, 28), + }, + }, + Control: control{ + LastWithdrawalValidatorIndex: 7, + NextWithdrawalIndex: 29, + Balances: map[uint64]uint64{ + 7: 0, 19: 0, 28: 0, + 2: maxEffectiveBalance, 1: maxEffectiveBalance, 89: maxEffectiveBalance, + 15: maxEffectiveBalance, + }, + }, + }, + { + Args: args{ + Name: "success more than max fully withdrawals", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 0, + FullWithdrawalIndices: []types.ValidatorIndex{1, 2, 3, 4, 5, 6, 7, 8, 9, 21, 22, 23, 24, 25, 26, 27, 29, 35, 89}, + Withdrawals: []*enginev1.Withdrawal{ + fullWithdrawal(1, 22), fullWithdrawal(2, 23), fullWithdrawal(3, 24), + fullWithdrawal(4, 25), fullWithdrawal(5, 26), fullWithdrawal(6, 27), + fullWithdrawal(7, 28), fullWithdrawal(8, 29), fullWithdrawal(9, 30), + fullWithdrawal(21, 31), fullWithdrawal(22, 32), fullWithdrawal(23, 33), + fullWithdrawal(24, 34), fullWithdrawal(25, 35), fullWithdrawal(26, 36), + fullWithdrawal(27, 37), + }, + }, + Control: control{ + LastWithdrawalValidatorIndex: 27, + NextWithdrawalIndex: 38, + Balances: map[uint64]uint64{ + 1: 0, 2: 0, 3: 0, 4: 0, 5: 0, 6: 0, 7: 0, 8: 0, 9: 0, + 21: 0, 22: 0, 23: 0, 24: 0, 25: 0, 26: 0, 27: 0, + }, + }, + }, + { + Args: args{ + Name: "success more than max partially withdrawals", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 0, + PartialWithdrawalIndices: []types.ValidatorIndex{1, 2, 3, 4, 5, 6, 7, 8, 9, 21, 22, 23, 24, 25, 26, 27, 29, 35, 89}, + Withdrawals: []*enginev1.Withdrawal{ + partialWithdrawal(1, 22), partialWithdrawal(2, 23), partialWithdrawal(3, 24), + partialWithdrawal(4, 25), partialWithdrawal(5, 26), partialWithdrawal(6, 27), + partialWithdrawal(7, 28), partialWithdrawal(8, 29), partialWithdrawal(9, 30), + partialWithdrawal(21, 31), partialWithdrawal(22, 32), partialWithdrawal(23, 33), + partialWithdrawal(24, 34), partialWithdrawal(25, 35), partialWithdrawal(26, 36), + partialWithdrawal(27, 37), + }, + }, + Control: control{ + LastWithdrawalValidatorIndex: 27, + NextWithdrawalIndex: 38, + Balances: map[uint64]uint64{ + 1: maxEffectiveBalance, + 2: maxEffectiveBalance, + 3: maxEffectiveBalance, + 4: maxEffectiveBalance, + 5: maxEffectiveBalance, + 6: maxEffectiveBalance, + 7: maxEffectiveBalance, + 8: maxEffectiveBalance, + 9: maxEffectiveBalance, + 21: maxEffectiveBalance, + 22: maxEffectiveBalance, + 23: maxEffectiveBalance, + 24: maxEffectiveBalance, + 25: maxEffectiveBalance, + 26: maxEffectiveBalance, + 27: maxEffectiveBalance, + }, + }, + }, + { + Args: args{ + Name: "failure wrong number of partial withdrawal", + NextWithdrawalIndex: 21, + LastWithdrawalValidatorIndex: 37, + PartialWithdrawalIndices: []types.ValidatorIndex{7}, + Withdrawals: []*enginev1.Withdrawal{ + partialWithdrawal(7, 21), partialWithdrawal(9, 22), + }, + }, + Control: control{ + ExpectedError: true, + }, + }, + { + Args: args{ + Name: "failure invalid withdrawal index", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 4, + FullWithdrawalIndices: []types.ValidatorIndex{7, 19, 28, 1}, + Withdrawals: []*enginev1.Withdrawal{ + fullWithdrawal(7, 22), fullWithdrawal(19, 23), fullWithdrawal(28, 25), + fullWithdrawal(1, 25), + }, + }, + Control: control{ + ExpectedError: true, + }, + }, + { + Args: args{ + Name: "failure invalid validator index", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 4, + FullWithdrawalIndices: []types.ValidatorIndex{7, 19, 28, 1}, + Withdrawals: []*enginev1.Withdrawal{ + fullWithdrawal(7, 22), fullWithdrawal(19, 23), fullWithdrawal(27, 24), + fullWithdrawal(1, 25), + }, + }, + Control: control{ + ExpectedError: true, + }, + }, + { + Args: args{ + Name: "failure invalid withdrawal amount", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 4, + FullWithdrawalIndices: []types.ValidatorIndex{7, 19, 28, 1}, + Withdrawals: []*enginev1.Withdrawal{ + fullWithdrawal(7, 22), fullWithdrawal(19, 23), partialWithdrawal(27, 24), + fullWithdrawal(1, 25), + }, + }, + Control: control{ + ExpectedError: true, + }, + }, + { + Args: args{ + Name: "failure validator not fully withdrawable", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 4, + FullWithdrawalIndices: []types.ValidatorIndex{notWithdrawableIndex}, + Withdrawals: []*enginev1.Withdrawal{ + fullWithdrawal(notWithdrawableIndex, 22), + }, + }, + Control: control{ + ExpectedError: true, + }, + }, + { + Args: args{ + Name: "failure validator not withdrawable", + NextWithdrawalIndex: 22, + LastWithdrawalValidatorIndex: 4, + PartialWithdrawalIndices: []types.ValidatorIndex{notPartiallyWithdrawable}, + Withdrawals: []*enginev1.Withdrawal{ + fullWithdrawal(notPartiallyWithdrawable, 22), + }, + }, + Control: control{ + ExpectedError: true, + }, + }, + } + + checkPostState := func(t *testing.T, expected control, st state.BeaconState) { + l, err := st.LastWithdrawalValidatorIndex() + require.NoError(t, err) + require.Equal(t, expected.LastWithdrawalValidatorIndex, l) + + n, err := st.NextWithdrawalIndex() + require.NoError(t, err) + require.Equal(t, expected.NextWithdrawalIndex, n) + balances := st.Balances() + for idx, bal := range expected.Balances { + require.Equal(t, bal, balances[idx]) + } + } + + prepareValidators := func(st *ethpb.BeaconStateCapella, arguments args) (state.BeaconState, error) { + validators := make([]*ethpb.Validator, numValidators) + st.Balances = make([]uint64, numValidators) + for i := range validators { + v := ðpb.Validator{} + v.EffectiveBalance = maxEffectiveBalance + v.WithdrawableEpoch = epochInFuture + v.WithdrawalCredentials = make([]byte, 32) + v.WithdrawalCredentials[31] = byte(i) + st.Balances[i] = v.EffectiveBalance - uint64(rand.Intn(1000)) + validators[i] = v + } + for _, idx := range arguments.FullWithdrawalIndices { + if idx != notWithdrawableIndex { + validators[idx].WithdrawableEpoch = epochInPast + } + st.Balances[idx] = withdrawalAmount(idx) + validators[idx].WithdrawalCredentials[0] = params.BeaconConfig().ETH1AddressWithdrawalPrefixByte + } + for _, idx := range arguments.PartialWithdrawalIndices { + validators[idx].WithdrawalCredentials[0] = params.BeaconConfig().ETH1AddressWithdrawalPrefixByte + st.Balances[idx] = withdrawalAmount(idx) + } + st.Validators = validators + return state_native.InitializeFromProtoCapella(st) + } + + for _, test := range tests { + t.Run(test.Args.Name, func(t *testing.T) { + if test.Args.Withdrawals == nil { + test.Args.Withdrawals = make([]*enginev1.Withdrawal, 0) + } + if test.Args.FullWithdrawalIndices == nil { + test.Args.FullWithdrawalIndices = make([]types.ValidatorIndex, 0) + } + if test.Args.PartialWithdrawalIndices == nil { + test.Args.PartialWithdrawalIndices = make([]types.ValidatorIndex, 0) + } + slot, err := slots.EpochStart(currentEpoch) + require.NoError(t, err) + spb := ðpb.BeaconStateCapella{ + Slot: slot, + LastWithdrawalValidatorIndex: test.Args.LastWithdrawalValidatorIndex, + NextWithdrawalIndex: test.Args.NextWithdrawalIndex, + } + st, err := prepareValidators(spb, test.Args) + require.NoError(t, err) + post, err := blocks.ProcessWithdrawals(st, test.Args.Withdrawals) + if test.Control.ExpectedError { + require.NotNil(t, err) + } else { + require.NoError(t, err) + checkPostState(t, test.Control, post) + } + }) + } +} diff --git a/beacon-chain/state/interfaces.go b/beacon-chain/state/interfaces.go index 67d5877d84bf..db410ca313c6 100644 --- a/beacon-chain/state/interfaces.go +++ b/beacon-chain/state/interfaces.go @@ -65,6 +65,7 @@ type ReadOnlyBeaconState interface { LatestExecutionPayloadHeader() (interfaces.ExecutionData, error) LastWithdrawalValidatorIndex() (types.ValidatorIndex, error) ExpectedWithdrawals() ([]*enginev1.Withdrawal, error) + NextWithdrawalIndex() (uint64, error) } // WriteOnlyBeaconState defines a struct which only has write access to beacon state methods. diff --git a/encoding/bytesutil/bytes.go b/encoding/bytesutil/bytes.go index 869be8aa17ec..97e8d554db35 100644 --- a/encoding/bytesutil/bytes.go +++ b/encoding/bytesutil/bytes.go @@ -99,6 +99,15 @@ func ToBytes4(x []byte) [4]byte { return y } +// ToBytes20 is a convenience method for converting a byte slice to a fix +// sized 20 byte array. This method will truncate the input if it is larger +// than 20 bytes. +func ToBytes20(x []byte) [20]byte { + var y [20]byte + copy(y[:], x) + return y +} + // ToBytes32 is a convenience method for converting a byte slice to a fix // sized 32 byte array. This method will truncate the input if it is larger // than 32 bytes. diff --git a/encoding/bytesutil/bytes_test.go b/encoding/bytesutil/bytes_test.go index 389dc7775e83..c4e9fd570a6d 100644 --- a/encoding/bytesutil/bytes_test.go +++ b/encoding/bytesutil/bytes_test.go @@ -54,6 +54,14 @@ func TestToBytes(t *testing.T) { } } +func TestToBytes20(t *testing.T) { + arg := []byte{3, 4} + expected := [20]byte{} + expected[0] = 3 + expected[1] = 4 + require.DeepEqual(t, expected, bytesutil.ToBytes20(arg)) +} + func TestBytes1(t *testing.T) { tests := []struct { a uint64 From 33cd9acb4b540577224be45af9fe9dda32b40e9e Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 7 Nov 2022 17:20:16 -0300 Subject: [PATCH 2/8] change errors to error.go --- beacon-chain/core/blocks/error.go | 5 +++++ beacon-chain/core/blocks/withdrawals.go | 10 +++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/beacon-chain/core/blocks/error.go b/beacon-chain/core/blocks/error.go index 7d968e7d5ee8..7455ba17f31a 100644 --- a/beacon-chain/core/blocks/error.go +++ b/beacon-chain/core/blocks/error.go @@ -6,3 +6,8 @@ var errNilSignedWithdrawalMessage = errors.New("nil SignedBLSToExecutionChange m var errNilWithdrawalMessage = errors.New("nil BLSToExecutionChange message") var errInvalidBLSPrefix = errors.New("withdrawal credential prefix is not a BLS prefix") var errInvalidWithdrawalCredentials = errors.New("withdrawal credentials do not match") +var errInvalidWithdrawalIndex = errors.New("invalid withdrawal index") +var errInvalidValidatorIndex = errors.New("invalid validator index") +var errInvalidWithdrawalAmount = errors.New("invalid withdrawal amount") +var errInvalidExecutionAddress = errors.New("invalid execution address") +var errInvalidWithdrawalNumber = errors.New("invalid number of withdrawals") diff --git a/beacon-chain/core/blocks/withdrawals.go b/beacon-chain/core/blocks/withdrawals.go index 02700c367259..f7d95ddc248b 100644 --- a/beacon-chain/core/blocks/withdrawals.go +++ b/beacon-chain/core/blocks/withdrawals.go @@ -86,20 +86,20 @@ func ProcessWithdrawals(st state.BeaconState, withdrawals []*enginev1.Withdrawal return nil, errors.Wrap(err, "could not get expected withdrawals") } if len(expected) != len(withdrawals) { - return nil, errors.New("invalid number of withdrawals") + return nil, errInvalidWithdrawalNumber } for i, withdrawal := range withdrawals { if withdrawal.WithdrawalIndex != expected[i].WithdrawalIndex { - return nil, errors.New("invalid withdrawal index") + return nil, errInvalidWithdrawalIndex } if withdrawal.ValidatorIndex != expected[i].ValidatorIndex { - return nil, errors.New("invalid validator index") + return nil, errInvalidValidatorIndex } if bytesutil.ToBytes20(withdrawal.ExecutionAddress) != bytesutil.ToBytes20(expected[i].ExecutionAddress) { - return nil, errors.New("invalid execution address") + return nil, errInvalidExecutionAddress } if withdrawal.Amount != expected[i].Amount { - return nil, errors.New("invalid withdrawal amount") + return nil, errInvalidWithdrawalAmount } err := helpers.DecreaseBalance(st, withdrawal.ValidatorIndex, withdrawal.Amount) if err != nil { From 93bf5a25649c2b9d69a5e534857f19c217a1e851 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 7 Nov 2022 18:19:14 -0300 Subject: [PATCH 3/8] gazelle --- beacon-chain/core/blocks/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon-chain/core/blocks/BUILD.bazel b/beacon-chain/core/blocks/BUILD.bazel index 62ceba68dc27..fb1dc1de47ff 100644 --- a/beacon-chain/core/blocks/BUILD.bazel +++ b/beacon-chain/core/blocks/BUILD.bazel @@ -45,6 +45,7 @@ go_library( "//encoding/bytesutil:go_default_library", "//math:go_default_library", "//network/forks:go_default_library", + "//proto/engine/v1:go_default_library", "//proto/prysm/v1alpha1:go_default_library", "//proto/prysm/v1alpha1/attestation:go_default_library", "//proto/prysm/v1alpha1/slashings:go_default_library", From feb68eee6f2e50d9ac28e42a77e6b0a583253147 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 7 Nov 2022 20:55:31 -0300 Subject: [PATCH 4/8] James' review --- beacon-chain/core/blocks/withdrawals.go | 2 +- encoding/bytesutil/bytes.go | 9 --------- encoding/bytesutil/bytes_test.go | 8 -------- 3 files changed, 1 insertion(+), 18 deletions(-) diff --git a/beacon-chain/core/blocks/withdrawals.go b/beacon-chain/core/blocks/withdrawals.go index f7d95ddc248b..53a3a5118b48 100644 --- a/beacon-chain/core/blocks/withdrawals.go +++ b/beacon-chain/core/blocks/withdrawals.go @@ -95,7 +95,7 @@ func ProcessWithdrawals(st state.BeaconState, withdrawals []*enginev1.Withdrawal if withdrawal.ValidatorIndex != expected[i].ValidatorIndex { return nil, errInvalidValidatorIndex } - if bytesutil.ToBytes20(withdrawal.ExecutionAddress) != bytesutil.ToBytes20(expected[i].ExecutionAddress) { + if bytes.Compare(withdrawal.ExecutionAddress, expected[i].ExecutionAddress) != 0 { return nil, errInvalidExecutionAddress } if withdrawal.Amount != expected[i].Amount { diff --git a/encoding/bytesutil/bytes.go b/encoding/bytesutil/bytes.go index 97e8d554db35..869be8aa17ec 100644 --- a/encoding/bytesutil/bytes.go +++ b/encoding/bytesutil/bytes.go @@ -99,15 +99,6 @@ func ToBytes4(x []byte) [4]byte { return y } -// ToBytes20 is a convenience method for converting a byte slice to a fix -// sized 20 byte array. This method will truncate the input if it is larger -// than 20 bytes. -func ToBytes20(x []byte) [20]byte { - var y [20]byte - copy(y[:], x) - return y -} - // ToBytes32 is a convenience method for converting a byte slice to a fix // sized 32 byte array. This method will truncate the input if it is larger // than 32 bytes. diff --git a/encoding/bytesutil/bytes_test.go b/encoding/bytesutil/bytes_test.go index c4e9fd570a6d..389dc7775e83 100644 --- a/encoding/bytesutil/bytes_test.go +++ b/encoding/bytesutil/bytes_test.go @@ -54,14 +54,6 @@ func TestToBytes(t *testing.T) { } } -func TestToBytes20(t *testing.T) { - arg := []byte{3, 4} - expected := [20]byte{} - expected[0] = 3 - expected[1] = 4 - require.DeepEqual(t, expected, bytesutil.ToBytes20(arg)) -} - func TestBytes1(t *testing.T) { tests := []struct { a uint64 From 3e5cb1a97c741b5b82675358ca413c86333d67f8 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 7 Nov 2022 21:11:39 -0300 Subject: [PATCH 5/8] use bytes.Equal instead --- beacon-chain/core/blocks/withdrawals.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/core/blocks/withdrawals.go b/beacon-chain/core/blocks/withdrawals.go index 53a3a5118b48..c95abd73df7f 100644 --- a/beacon-chain/core/blocks/withdrawals.go +++ b/beacon-chain/core/blocks/withdrawals.go @@ -95,7 +95,7 @@ func ProcessWithdrawals(st state.BeaconState, withdrawals []*enginev1.Withdrawal if withdrawal.ValidatorIndex != expected[i].ValidatorIndex { return nil, errInvalidValidatorIndex } - if bytes.Compare(withdrawal.ExecutionAddress, expected[i].ExecutionAddress) != 0 { + if !bytes.Equal(withdrawal.ExecutionAddress, expected[i].ExecutionAddress) { return nil, errInvalidExecutionAddress } if withdrawal.Amount != expected[i].Amount { From 26228b02e417cea7e479746833192b8ad271e60e Mon Sep 17 00:00:00 2001 From: Potuz Date: Tue, 8 Nov 2022 09:13:41 -0300 Subject: [PATCH 6/8] Radek's review --- beacon-chain/core/blocks/withdrawals_test.go | 2 +- beacon-chain/state/interfaces.go | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/beacon-chain/core/blocks/withdrawals_test.go b/beacon-chain/core/blocks/withdrawals_test.go index 648a28d20fce..0ce0537180dd 100644 --- a/beacon-chain/core/blocks/withdrawals_test.go +++ b/beacon-chain/core/blocks/withdrawals_test.go @@ -493,7 +493,7 @@ func TestProcessWithdrawals(t *testing.T) { }, { Args: args{ - Name: "failure validator not withdrawable", + Name: "failure validator not partially withdrawable", NextWithdrawalIndex: 22, LastWithdrawalValidatorIndex: 4, PartialWithdrawalIndices: []types.ValidatorIndex{notPartiallyWithdrawable}, diff --git a/beacon-chain/state/interfaces.go b/beacon-chain/state/interfaces.go index db410ca313c6..70f2c1819761 100644 --- a/beacon-chain/state/interfaces.go +++ b/beacon-chain/state/interfaces.go @@ -63,9 +63,6 @@ type ReadOnlyBeaconState interface { IsNil() bool Version() int LatestExecutionPayloadHeader() (interfaces.ExecutionData, error) - LastWithdrawalValidatorIndex() (types.ValidatorIndex, error) - ExpectedWithdrawals() ([]*enginev1.Withdrawal, error) - NextWithdrawalIndex() (uint64, error) } // WriteOnlyBeaconState defines a struct which only has write access to beacon state methods. @@ -169,6 +166,13 @@ type ReadOnlyAttestations interface { CurrentEpochAttestations() ([]*ethpb.PendingAttestation, error) } +// ReadOnlyWithdrawals defines a struct which only has read access to withdrawal methods. +type ReadOnlyWithdrawals interface { + ExpectedWithdrawals() ([]*enginev1.Withdrawal, error) + LastWithdrawalValidatorIndex() (types.ValidatorIndex, error) + NextWithdrawalIndex() (uint64, error) +} + // WriteOnlyBlockRoots defines a struct which only has write access to block roots methods. type WriteOnlyBlockRoots interface { SetBlockRoots(val [][]byte) error From a80cab19d81226afd81284401a7249a5c036307a Mon Sep 17 00:00:00 2001 From: Potuz Date: Tue, 8 Nov 2022 09:32:42 -0300 Subject: [PATCH 7/8] Radek's review #2 --- beacon-chain/core/blocks/withdrawals.go | 2 +- beacon-chain/state/interfaces.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/beacon-chain/core/blocks/withdrawals.go b/beacon-chain/core/blocks/withdrawals.go index c95abd73df7f..ff5c1f65ab92 100644 --- a/beacon-chain/core/blocks/withdrawals.go +++ b/beacon-chain/core/blocks/withdrawals.go @@ -108,7 +108,7 @@ func ProcessWithdrawals(st state.BeaconState, withdrawals []*enginev1.Withdrawal } if len(withdrawals) > 0 { if err := st.SetNextWithdrawalIndex(withdrawals[len(withdrawals)-1].WithdrawalIndex + 1); err != nil { - return nil, errors.Wrap(err, "could not set withdrawal index") + return nil, errors.Wrap(err, "could not set next withdrawal index") } if err := st.SetLastWithdrawalValidatorIndex(withdrawals[len(withdrawals)-1].ValidatorIndex); err != nil { return nil, errors.Wrap(err, "could not set latest withdrawal validator index") diff --git a/beacon-chain/state/interfaces.go b/beacon-chain/state/interfaces.go index 70f2c1819761..1a50af2286ff 100644 --- a/beacon-chain/state/interfaces.go +++ b/beacon-chain/state/interfaces.go @@ -18,6 +18,7 @@ import ( type BeaconState interface { SpecParametersProvider ReadOnlyBeaconState + ReadOnlyWithdrawals WriteOnlyBeaconState Copy() BeaconState HashTreeRoot(ctx context.Context) ([32]byte, error) From b8829da081b49f6e3a6c7edcce3d29ee2e724204 Mon Sep 17 00:00:00 2001 From: Potuz Date: Tue, 8 Nov 2022 09:35:02 -0300 Subject: [PATCH 8/8] fix test --- beacon-chain/core/blocks/withdrawals_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/core/blocks/withdrawals_test.go b/beacon-chain/core/blocks/withdrawals_test.go index 0ce0537180dd..19c838c85f10 100644 --- a/beacon-chain/core/blocks/withdrawals_test.go +++ b/beacon-chain/core/blocks/withdrawals_test.go @@ -469,7 +469,7 @@ func TestProcessWithdrawals(t *testing.T) { LastWithdrawalValidatorIndex: 4, FullWithdrawalIndices: []types.ValidatorIndex{7, 19, 28, 1}, Withdrawals: []*enginev1.Withdrawal{ - fullWithdrawal(7, 22), fullWithdrawal(19, 23), partialWithdrawal(27, 24), + fullWithdrawal(7, 22), fullWithdrawal(19, 23), partialWithdrawal(28, 24), fullWithdrawal(1, 25), }, },