From 5ceecf69fd1dac0fb40a6248269a06feda2bbf32 Mon Sep 17 00:00:00 2001 From: terence Date: Fri, 23 Jan 2026 13:33:28 -0800 Subject: [PATCH] Move deposit helpers out of blocks to break blocks <-> gloas cycle (#16277) - moves deposit-related helpers (deposit signature verification, batch verification, merkle proof verification, and activation helper) from `beacon-chain/core/blocks` into `beacon-chain/core/helpers` - updates call sites (Altair/Electra) to use helpers Why? - In gloas, the blocks package needs to call into gloas logic (e.g. clearing builder pending payments/withdrawals on proposer slashing) - gloas also introduces deposit-request processing which needs deposit signature verification previously located in blocks. That creates a Bazel/Go dependency cycle (blocks -> gloas -> blocks) - the natural layering is for blocks and fork logic to depend on a lower level util package for deposit verification, so moving deposit helpers to core/helpers breaks the cycle --- beacon-chain/core/altair/deposit.go | 9 ++++----- beacon-chain/core/blocks/BUILD.bazel | 5 ----- .../core/blocks/block_operations_fuzz_test.go | 3 ++- beacon-chain/core/electra/BUILD.bazel | 1 - beacon-chain/core/electra/deposits.go | 11 +++++------ beacon-chain/core/helpers/BUILD.bazel | 7 +++++++ beacon-chain/core/{blocks => helpers}/deposit.go | 2 +- .../core/{blocks => helpers}/deposit_test.go | 16 ++++++++-------- beacon-chain/state/testing/BUILD.bazel | 2 +- beacon-chain/state/testing/generators.go | 4 ++-- .../terencechain_move-deposit-to-helpers.md | 2 ++ specrefs/functions.yml | 2 +- testing/util/altair.go | 3 +-- 13 files changed, 34 insertions(+), 33 deletions(-) rename beacon-chain/core/{blocks => helpers}/deposit.go (99%) rename beacon-chain/core/{blocks => helpers}/deposit_test.go (90%) create mode 100644 changelog/terencechain_move-deposit-to-helpers.md diff --git a/beacon-chain/core/altair/deposit.go b/beacon-chain/core/altair/deposit.go index c23c879b8c..eb85589adb 100644 --- a/beacon-chain/core/altair/deposit.go +++ b/beacon-chain/core/altair/deposit.go @@ -3,7 +3,6 @@ package altair import ( "context" - "github.com/OffchainLabs/prysm/v7/beacon-chain/core/blocks" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers" "github.com/OffchainLabs/prysm/v7/beacon-chain/state" "github.com/OffchainLabs/prysm/v7/config/params" @@ -24,7 +23,7 @@ func ProcessPreGenesisDeposits( if err != nil { return nil, errors.Wrap(err, "could not process deposit") } - beaconState, err = blocks.ActivateValidatorWithEffectiveBalance(beaconState, deposits) + beaconState, err = helpers.ActivateValidatorWithEffectiveBalance(beaconState, deposits) if err != nil { return nil, err } @@ -37,7 +36,7 @@ func ProcessDeposits( beaconState state.BeaconState, deposits []*ethpb.Deposit, ) (state.BeaconState, error) { - allSignaturesVerified, err := blocks.BatchVerifyDepositsSignatures(ctx, deposits) + allSignaturesVerified, err := helpers.BatchVerifyDepositsSignatures(ctx, deposits) if err != nil { return nil, err } @@ -82,7 +81,7 @@ func ProcessDeposits( // signature=deposit.data.signature, // ) func ProcessDeposit(beaconState state.BeaconState, deposit *ethpb.Deposit, allSignaturesVerified bool) (state.BeaconState, error) { - if err := blocks.VerifyDeposit(beaconState, deposit); err != nil { + if err := helpers.VerifyDeposit(beaconState, deposit); err != nil { if deposit == nil || deposit.Data == nil { return nil, err } @@ -122,7 +121,7 @@ func ApplyDeposit(beaconState state.BeaconState, data *ethpb.Deposit_Data, allSi index, ok := beaconState.ValidatorIndexByPubkey(bytesutil.ToBytes48(pubKey)) if !ok { if !allSignaturesVerified { - valid, err := blocks.IsValidDepositSignature(data) + valid, err := helpers.IsValidDepositSignature(data) if err != nil { return nil, err } diff --git a/beacon-chain/core/blocks/BUILD.bazel b/beacon-chain/core/blocks/BUILD.bazel index 08678167c7..6ff9a95759 100644 --- a/beacon-chain/core/blocks/BUILD.bazel +++ b/beacon-chain/core/blocks/BUILD.bazel @@ -5,7 +5,6 @@ go_library( srcs = [ "attestation.go", "attester_slashing.go", - "deposit.go", "error.go", "eth1_data.go", "exit.go", @@ -34,8 +33,6 @@ go_library( "//consensus-types/interfaces:go_default_library", "//consensus-types/primitives:go_default_library", "//container/slice:go_default_library", - "//container/trie:go_default_library", - "//contracts/deposit:go_default_library", "//crypto/bls:go_default_library", "//crypto/hash:go_default_library", "//encoding/bytesutil:go_default_library", @@ -62,7 +59,6 @@ go_test( "attester_slashing_test.go", "block_operations_fuzz_test.go", "block_regression_test.go", - "deposit_test.go", "eth1_data_test.go", "exit_test.go", "exports_test.go", @@ -91,7 +87,6 @@ go_test( "//consensus-types/blocks:go_default_library", "//consensus-types/interfaces:go_default_library", "//consensus-types/primitives:go_default_library", - "//container/trie:go_default_library", "//crypto/bls:go_default_library", "//crypto/bls/common:go_default_library", "//crypto/hash:go_default_library", diff --git a/beacon-chain/core/blocks/block_operations_fuzz_test.go b/beacon-chain/core/blocks/block_operations_fuzz_test.go index e12b1217e1..2ca9612a2e 100644 --- a/beacon-chain/core/blocks/block_operations_fuzz_test.go +++ b/beacon-chain/core/blocks/block_operations_fuzz_test.go @@ -3,6 +3,7 @@ package blocks import ( "testing" + "github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers" v "github.com/OffchainLabs/prysm/v7/beacon-chain/core/validators" state_native "github.com/OffchainLabs/prysm/v7/beacon-chain/state/state-native" fieldparams "github.com/OffchainLabs/prysm/v7/config/fieldparams" @@ -318,7 +319,7 @@ func TestFuzzverifyDeposit_10000(t *testing.T) { fuzzer.Fuzz(deposit) s, err := state_native.InitializeFromProtoUnsafePhase0(state) require.NoError(t, err) - err = VerifyDeposit(s, deposit) + err = helpers.VerifyDeposit(s, deposit) _ = err fuzz.FreeMemory(i) } diff --git a/beacon-chain/core/electra/BUILD.bazel b/beacon-chain/core/electra/BUILD.bazel index de1fc87870..2a1499325b 100644 --- a/beacon-chain/core/electra/BUILD.bazel +++ b/beacon-chain/core/electra/BUILD.bazel @@ -20,7 +20,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//beacon-chain/core/altair:go_default_library", - "//beacon-chain/core/blocks:go_default_library", "//beacon-chain/core/epoch:go_default_library", "//beacon-chain/core/epoch/precompute:go_default_library", "//beacon-chain/core/helpers:go_default_library", diff --git a/beacon-chain/core/electra/deposits.go b/beacon-chain/core/electra/deposits.go index f311f90021..e292ad13ba 100644 --- a/beacon-chain/core/electra/deposits.go +++ b/beacon-chain/core/electra/deposits.go @@ -3,7 +3,6 @@ package electra import ( "context" - "github.com/OffchainLabs/prysm/v7/beacon-chain/core/blocks" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/signing" "github.com/OffchainLabs/prysm/v7/beacon-chain/state" @@ -37,7 +36,7 @@ func ProcessDeposits( defer span.End() // Attempt to verify all deposit signatures at once, if this fails then fall back to processing // individual deposits with signature verification enabled. - allSignaturesVerified, err := blocks.BatchVerifyDepositsSignatures(ctx, deposits) + allSignaturesVerified, err := helpers.BatchVerifyDepositsSignatures(ctx, deposits) if err != nil { return nil, errors.Wrap(err, "could not verify deposit signatures in batch") } @@ -82,7 +81,7 @@ func ProcessDeposits( // signature=deposit.data.signature, // ) func ProcessDeposit(beaconState state.BeaconState, deposit *ethpb.Deposit, allSignaturesVerified bool) (state.BeaconState, error) { - if err := blocks.VerifyDeposit(beaconState, deposit); err != nil { + if err := helpers.VerifyDeposit(beaconState, deposit); err != nil { if deposit == nil || deposit.Data == nil { return nil, err } @@ -377,7 +376,7 @@ func batchProcessNewPendingDeposits(ctx context.Context, state state.BeaconState return nil } - allSignaturesVerified, err := blocks.BatchVerifyPendingDepositsSignatures(ctx, pendingDeposits) + allSignaturesVerified, err := helpers.BatchVerifyPendingDepositsSignatures(ctx, pendingDeposits) if err != nil { return errors.Wrap(err, "batch signature verification failed") } @@ -386,7 +385,7 @@ func batchProcessNewPendingDeposits(ctx context.Context, state state.BeaconState validSig := allSignaturesVerified if !allSignaturesVerified { - validSig, err = blocks.IsValidDepositSignature(ðpb.Deposit_Data{ + validSig, err = helpers.IsValidDepositSignature(ðpb.Deposit_Data{ PublicKey: bytesutil.SafeCopyBytes(pd.PublicKey), WithdrawalCredentials: bytesutil.SafeCopyBytes(pd.WithdrawalCredentials), Amount: pd.Amount, @@ -441,7 +440,7 @@ func ApplyPendingDeposit(ctx context.Context, st state.BeaconState, deposit *eth defer span.End() index, ok := st.ValidatorIndexByPubkey(bytesutil.ToBytes48(deposit.PublicKey)) if !ok { - verified, err := blocks.IsValidDepositSignature(ðpb.Deposit_Data{ + verified, err := helpers.IsValidDepositSignature(ðpb.Deposit_Data{ PublicKey: bytesutil.SafeCopyBytes(deposit.PublicKey), WithdrawalCredentials: bytesutil.SafeCopyBytes(deposit.WithdrawalCredentials), Amount: deposit.Amount, diff --git a/beacon-chain/core/helpers/BUILD.bazel b/beacon-chain/core/helpers/BUILD.bazel index bfaeed336e..157a47babd 100644 --- a/beacon-chain/core/helpers/BUILD.bazel +++ b/beacon-chain/core/helpers/BUILD.bazel @@ -6,6 +6,7 @@ go_library( "attestation.go", "beacon_committee.go", "block.go", + "deposit.go", "genesis.go", "legacy.go", "log.go", @@ -23,6 +24,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//beacon-chain/cache:go_default_library", + "//beacon-chain/core/signing:go_default_library", "//beacon-chain/core/time:go_default_library", "//beacon-chain/forkchoice/types:go_default_library", "//beacon-chain/state:go_default_library", @@ -31,6 +33,7 @@ go_library( "//consensus-types/primitives:go_default_library", "//container/slice:go_default_library", "//container/trie:go_default_library", + "//contracts/deposit:go_default_library", "//crypto/bls:go_default_library", "//crypto/hash:go_default_library", "//encoding/bytesutil:go_default_library", @@ -54,6 +57,7 @@ go_test( "attestation_test.go", "beacon_committee_test.go", "block_test.go", + "deposit_test.go", "legacy_test.go", "private_access_fuzz_noop_test.go", # keep "private_access_test.go", @@ -72,6 +76,7 @@ go_test( tags = ["CI_race_detection"], deps = [ "//beacon-chain/cache:go_default_library", + "//beacon-chain/core/signing:go_default_library", "//beacon-chain/core/time:go_default_library", "//beacon-chain/forkchoice/types:go_default_library", "//beacon-chain/state:go_default_library", @@ -80,6 +85,8 @@ go_test( "//config/params:go_default_library", "//consensus-types/primitives:go_default_library", "//container/slice:go_default_library", + "//container/trie:go_default_library", + "//crypto/bls:go_default_library", "//crypto/hash:go_default_library", "//encoding/bytesutil:go_default_library", "//proto/prysm/v1alpha1:go_default_library", diff --git a/beacon-chain/core/blocks/deposit.go b/beacon-chain/core/helpers/deposit.go similarity index 99% rename from beacon-chain/core/blocks/deposit.go rename to beacon-chain/core/helpers/deposit.go index 2eca62610b..e413187765 100644 --- a/beacon-chain/core/blocks/deposit.go +++ b/beacon-chain/core/helpers/deposit.go @@ -1,4 +1,4 @@ -package blocks +package helpers import ( "context" diff --git a/beacon-chain/core/blocks/deposit_test.go b/beacon-chain/core/helpers/deposit_test.go similarity index 90% rename from beacon-chain/core/blocks/deposit_test.go rename to beacon-chain/core/helpers/deposit_test.go index 4d382fd000..e2ff82211f 100644 --- a/beacon-chain/core/blocks/deposit_test.go +++ b/beacon-chain/core/helpers/deposit_test.go @@ -1,9 +1,9 @@ -package blocks_test +package helpers_test import ( "testing" - "github.com/OffchainLabs/prysm/v7/beacon-chain/core/blocks" + "github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/signing" state_native "github.com/OffchainLabs/prysm/v7/beacon-chain/state/state-native" fieldparams "github.com/OffchainLabs/prysm/v7/config/fieldparams" @@ -45,7 +45,7 @@ func TestBatchVerifyDepositsSignatures_Ok(t *testing.T) { deposit.Proof = proof require.NoError(t, err) - verified, err := blocks.BatchVerifyDepositsSignatures(t.Context(), []*ethpb.Deposit{deposit}) + verified, err := helpers.BatchVerifyDepositsSignatures(t.Context(), []*ethpb.Deposit{deposit}) require.NoError(t, err) require.Equal(t, true, verified) } @@ -68,7 +68,7 @@ func TestBatchVerifyDepositsSignatures_InvalidSignature(t *testing.T) { deposit.Proof = proof require.NoError(t, err) - verified, err := blocks.BatchVerifyDepositsSignatures(t.Context(), []*ethpb.Deposit{deposit}) + verified, err := helpers.BatchVerifyDepositsSignatures(t.Context(), []*ethpb.Deposit{deposit}) require.NoError(t, err) require.Equal(t, false, verified) } @@ -99,7 +99,7 @@ func TestVerifyDeposit_MerkleBranchFailsVerification(t *testing.T) { }) require.NoError(t, err) want := "deposit root did not verify" - err = blocks.VerifyDeposit(beaconState, deposit) + err = helpers.VerifyDeposit(beaconState, deposit) require.ErrorContains(t, want, err) } @@ -123,7 +123,7 @@ func TestIsValidDepositSignature_Ok(t *testing.T) { require.NoError(t, err) sig := sk.Sign(sr[:]) depositData.Signature = sig.Marshal() - valid, err := blocks.IsValidDepositSignature(depositData) + valid, err := helpers.IsValidDepositSignature(depositData) require.NoError(t, err) require.Equal(t, true, valid) } @@ -163,7 +163,7 @@ func TestBatchVerifyPendingDepositsSignatures_Ok(t *testing.T) { sig2 := sk2.Sign(sr2[:]) pendingDeposit2.Signature = sig2.Marshal() - verified, err := blocks.BatchVerifyPendingDepositsSignatures(t.Context(), []*ethpb.PendingDeposit{pendingDeposit, pendingDeposit2}) + verified, err := helpers.BatchVerifyPendingDepositsSignatures(t.Context(), []*ethpb.PendingDeposit{pendingDeposit, pendingDeposit2}) require.NoError(t, err) require.Equal(t, true, verified) } @@ -174,7 +174,7 @@ func TestBatchVerifyPendingDepositsSignatures_InvalidSignature(t *testing.T) { WithdrawalCredentials: make([]byte, 32), Signature: make([]byte, 96), } - verified, err := blocks.BatchVerifyPendingDepositsSignatures(t.Context(), []*ethpb.PendingDeposit{pendingDeposit}) + verified, err := helpers.BatchVerifyPendingDepositsSignatures(t.Context(), []*ethpb.PendingDeposit{pendingDeposit}) require.NoError(t, err) require.Equal(t, false, verified) } diff --git a/beacon-chain/state/testing/BUILD.bazel b/beacon-chain/state/testing/BUILD.bazel index ff601fb2e0..199b7d9ffc 100644 --- a/beacon-chain/state/testing/BUILD.bazel +++ b/beacon-chain/state/testing/BUILD.bazel @@ -16,7 +16,7 @@ go_library( "//beacon-chain/state:__subpackages__", ], deps = [ - "//beacon-chain/core/blocks:go_default_library", + "//beacon-chain/core/helpers:go_default_library", "//beacon-chain/core/signing:go_default_library", "//beacon-chain/state:go_default_library", "//config/fieldparams:go_default_library", diff --git a/beacon-chain/state/testing/generators.go b/beacon-chain/state/testing/generators.go index 2af5dab71f..43e9429d62 100644 --- a/beacon-chain/state/testing/generators.go +++ b/beacon-chain/state/testing/generators.go @@ -3,7 +3,7 @@ package testing import ( "testing" - "github.com/OffchainLabs/prysm/v7/beacon-chain/core/blocks" + "github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/signing" "github.com/OffchainLabs/prysm/v7/config/params" "github.com/OffchainLabs/prysm/v7/consensus-types/primitives" @@ -31,7 +31,7 @@ func GeneratePendingDeposit(t *testing.T, key common.SecretKey, amount uint64, w Amount: dm.Amount, Signature: sig.Marshal(), } - valid, err := blocks.IsValidDepositSignature(depositData) + valid, err := helpers.IsValidDepositSignature(depositData) require.NoError(t, err) require.Equal(t, true, valid) return ðpb.PendingDeposit{ diff --git a/changelog/terencechain_move-deposit-to-helpers.md b/changelog/terencechain_move-deposit-to-helpers.md new file mode 100644 index 0000000000..d4cc9d9c21 --- /dev/null +++ b/changelog/terencechain_move-deposit-to-helpers.md @@ -0,0 +1,2 @@ +### Ignored +- Move deposit helpers from `beacon-chain/core/blocks` to `beacon-chain/core/helpers` (refactor only). diff --git a/specrefs/functions.yml b/specrefs/functions.yml index 542eaa4fe6..9613c17737 100644 --- a/specrefs/functions.yml +++ b/specrefs/functions.yml @@ -4230,7 +4230,7 @@ sources: - file: beacon-chain/core/electra/deposits.go search: func IsValidDepositSignature( - - file: beacon-chain/core/blocks/deposit.go + - file: beacon-chain/core/helpers/deposit.go search: func IsValidDepositSignature( spec: | diff --git a/testing/util/altair.go b/testing/util/altair.go index 0d4fce1d99..490e4a8163 100644 --- a/testing/util/altair.go +++ b/testing/util/altair.go @@ -7,7 +7,6 @@ import ( "github.com/OffchainLabs/go-bitfield" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/altair" - "github.com/OffchainLabs/prysm/v7/beacon-chain/core/blocks" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/signing" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/time" @@ -75,7 +74,7 @@ func processPreGenesisDeposits( if err != nil { return nil, errors.Wrap(err, "could not process deposit") } - beaconState, err = blocks.ActivateValidatorWithEffectiveBalance(beaconState, deposits) + beaconState, err = helpers.ActivateValidatorWithEffectiveBalance(beaconState, deposits) if err != nil { return nil, err }