Skip to content

Commit

Permalink
Handle errors as no-op for execution requests (#14826)
Browse files Browse the repository at this point in the history
* Update electra core processing error handling

* Add test for IsExecutionRequestError

* Add TestProcessOperationsWithNilRequests

* gazelle

---------

Co-authored-by: Preston Van Loon <[email protected]>
  • Loading branch information
terencechain and prestonvanloon authored Jan 31, 2025
1 parent f9c2021 commit 910609a
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 5 deletions.
1 change: 1 addition & 0 deletions beacon-chain/blockchain/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ go_library(
"//beacon-chain/cache:go_default_library",
"//beacon-chain/core/altair:go_default_library",
"//beacon-chain/core/blocks:go_default_library",
"//beacon-chain/core/electra:go_default_library",
"//beacon-chain/core/epoch/precompute:go_default_library",
"//beacon-chain/core/feed:go_default_library",
"//beacon-chain/core/feed/state:go_default_library",
Expand Down
3 changes: 2 additions & 1 deletion beacon-chain/blockchain/receive_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/electra"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/feed"
statefeed "github.com/prysmaticlabs/prysm/v5/beacon-chain/core/feed/state"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/helpers"
Expand Down Expand Up @@ -468,7 +469,7 @@ func (s *Service) validateStateTransition(ctx context.Context, preState state.Be
stateTransitionStartTime := time.Now()
postState, err := transition.ExecuteStateTransition(ctx, preState, signed)
if err != nil {
if ctx.Err() != nil {
if ctx.Err() != nil || electra.IsExecutionRequestError(err) {
return nil, err
}
return nil, invalidBlock{error: err}
Expand Down
5 changes: 5 additions & 0 deletions beacon-chain/core/electra/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ go_library(
"consolidations.go",
"deposits.go",
"effective_balance_updates.go",
"error.go",
"registry_updates.go",
"transition.go",
"transition_no_verify_sig.go",
Expand Down Expand Up @@ -55,13 +56,16 @@ go_test(
"deposit_fuzz_test.go",
"deposits_test.go",
"effective_balance_updates_test.go",
"error_test.go",
"export_test.go",
"registry_updates_test.go",
"transition_no_verify_sig_test.go",
"transition_test.go",
"upgrade_test.go",
"validator_test.go",
"withdrawals_test.go",
],
data = glob(["testdata/**"]),
embed = [":go_default_library"],
deps = [
"//beacon-chain/core/helpers:go_default_library",
Expand All @@ -86,6 +90,7 @@ go_test(
"@com_github_ethereum_go_ethereum//common:go_default_library",
"@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
"@com_github_google_gofuzz//:go_default_library",
"@com_github_pkg_errors//:go_default_library",
"@com_github_sirupsen_logrus//:go_default_library",
"@com_github_sirupsen_logrus//hooks/test:go_default_library",
],
Expand Down
16 changes: 16 additions & 0 deletions beacon-chain/core/electra/error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package electra

import "github.com/pkg/errors"

type execReqErr struct {
error
}

// IsExecutionRequestError returns true if the error has `execReqErr`.
func IsExecutionRequestError(e error) bool {
if e == nil {
return false
}
var d execReqErr
return errors.As(e, &d)
}
45 changes: 45 additions & 0 deletions beacon-chain/core/electra/error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package electra

import (
"testing"

"github.com/pkg/errors"
)

func TestIsExecutionRequestError(t *testing.T) {
tests := []struct {
name string
err error
want bool
}{
{
name: "nil error",
err: nil,
want: false,
},
{
name: "random error",
err: errors.New("some error"),
want: false,
},
{
name: "execution request error",
err: execReqErr{errors.New("execution request failed")},
want: true,
},
{
name: "wrapped execution request error",
err: errors.Wrap(execReqErr{errors.New("execution request failed")}, "wrapped"),
want: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := IsExecutionRequestError(tt.err)
if got != tt.want {
t.Errorf("IsExecutionRequestError(%v) = %v, want %v", tt.err, got, tt.want)
}
})
}
}
22 changes: 18 additions & 4 deletions beacon-chain/core/electra/transition_no_verify_sig.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package electra

import (
"context"
"fmt"

"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/blocks"
Expand Down Expand Up @@ -82,16 +81,31 @@ func ProcessOperations(
if err != nil {
return nil, errors.Wrap(err, "could not get execution requests")
}
for _, d := range requests.Deposits {
if d == nil {
return nil, errors.New("nil deposit request")
}
}
st, err = ProcessDepositRequests(ctx, st, requests.Deposits)
if err != nil {
return nil, errors.Wrap(err, "could not process deposit requests")
return nil, execReqErr{errors.Wrap(err, "could not process deposit requests")}
}
for _, w := range requests.Withdrawals {
if w == nil {
return nil, errors.New("nil withdrawal request")
}
}
st, err = ProcessWithdrawalRequests(ctx, st, requests.Withdrawals)
if err != nil {
return nil, errors.Wrap(err, "could not process withdrawal requests")
return nil, execReqErr{errors.Wrap(err, "could not process withdrawal requests")}
}
for _, c := range requests.Consolidations {
if c == nil {
return nil, errors.New("nil consolidation request")
}
}
if err := ProcessConsolidationRequests(ctx, st, requests.Consolidations); err != nil {
return nil, fmt.Errorf("could not process consolidation requests: %w", err)
return nil, execReqErr{errors.Wrap(err, "could not process consolidation requests")}
}
return st, nil
}
61 changes: 61 additions & 0 deletions beacon-chain/core/electra/transition_no_verify_sig_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package electra_test

import (
"context"
"testing"

"github.com/prysmaticlabs/prysm/v5/beacon-chain/core/electra"
"github.com/prysmaticlabs/prysm/v5/consensus-types/blocks"
enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v5/testing/require"
"github.com/prysmaticlabs/prysm/v5/testing/util"
)

func TestProcessOperationsWithNilRequests(t *testing.T) {
tests := []struct {
name string
modifyBlk func(blockElectra *ethpb.SignedBeaconBlockElectra)
errMsg string
}{
{
name: "Nil deposit request",
modifyBlk: func(blk *ethpb.SignedBeaconBlockElectra) {
blk.Block.Body.ExecutionRequests.Deposits = []*enginev1.DepositRequest{nil}
},
errMsg: "nil deposit request",
},
{
name: "Nil withdrawal request",
modifyBlk: func(blk *ethpb.SignedBeaconBlockElectra) {
blk.Block.Body.ExecutionRequests.Withdrawals = []*enginev1.WithdrawalRequest{nil}
},
errMsg: "nil withdrawal request",
},
{
name: "Nil consolidation request",
modifyBlk: func(blk *ethpb.SignedBeaconBlockElectra) {
blk.Block.Body.ExecutionRequests.Consolidations = []*enginev1.ConsolidationRequest{nil}
},
errMsg: "nil consolidation request",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
st, ks := util.DeterministicGenesisStateElectra(t, 128)
blk, err := util.GenerateFullBlockElectra(st, ks, util.DefaultBlockGenConfig(), 1)
require.NoError(t, err)

tc.modifyBlk(blk)

b, err := blocks.NewSignedBeaconBlock(blk)
require.NoError(t, err)

require.NoError(t, st.SetSlot(1))

_, err = electra.ProcessOperations(context.Background(), st, b.Block())
require.ErrorContains(t, tc.errMsg, err)
})
}
}
3 changes: 3 additions & 0 deletions changelog/tt_fix_electra_core_processing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Changed

- Update electra core processing to not mark block bad if execution request error.

0 comments on commit 910609a

Please sign in to comment.