diff --git a/consensus/cometbft/service/prepare_proposal.go b/consensus/cometbft/service/prepare_proposal.go index d323d48b6e..bc19d7ca27 100644 --- a/consensus/cometbft/service/prepare_proposal.go +++ b/consensus/cometbft/service/prepare_proposal.go @@ -26,6 +26,7 @@ import ( "time" "github.com/berachain/beacon-kit/consensus/types" + "github.com/berachain/beacon-kit/errors" "github.com/berachain/beacon-kit/primitives/math" cmtabci "github.com/cometbft/cometbft/abci/types" ) @@ -71,7 +72,10 @@ func (s *Service) prepareProposal( s.prepareProposalState.Context(), slotData, ) - if err != nil { + switch { + case errors.IsFatal(err): + return nil, err + case err != nil: s.logger.Error( "failed to prepare proposal", "height", req.Height, @@ -79,9 +83,7 @@ func (s *Service) prepareProposal( "err", err, ) return &cmtabci.PrepareProposalResponse{Txs: [][]byte{}}, nil + default: + return &cmtabci.PrepareProposalResponse{Txs: [][]byte{blkBz, sidecarsBz}}, nil } - - return &cmtabci.PrepareProposalResponse{ - Txs: [][]byte{blkBz, sidecarsBz}, - }, nil } diff --git a/consensus/cometbft/service/process_proposal.go b/consensus/cometbft/service/process_proposal.go index ddcef37153..6613164a54 100644 --- a/consensus/cometbft/service/process_proposal.go +++ b/consensus/cometbft/service/process_proposal.go @@ -25,6 +25,7 @@ import ( "fmt" "time" + "github.com/berachain/beacon-kit/errors" cmtabci "github.com/cometbft/cometbft/abci/types" ) @@ -68,20 +69,23 @@ func (s *Service) processProposal( // errors to consensus indicate that the node was not able to understand // whether the block was valid or not. Viceversa, we signal that a block // is invalid by its status, but we do return nil error in such a case. - status := cmtabci.PROCESS_PROPOSAL_STATUS_ACCEPT err := s.Blockchain.ProcessProposal( s.processProposalState.Context(), req, ) - if err != nil { - status = cmtabci.PROCESS_PROPOSAL_STATUS_REJECT + switch { + case errors.IsFatal(err): + return nil, err + case err != nil: s.logger.Error( - "failed to process proposal", + "proposal rejected", "height", req.Height, "time", req.Time, "hash", fmt.Sprintf("%X", req.Hash), "err", err, ) + return &cmtabci.ProcessProposalResponse{Status: cmtabci.PROCESS_PROPOSAL_STATUS_REJECT}, nil + default: + return &cmtabci.ProcessProposalResponse{Status: cmtabci.PROCESS_PROPOSAL_STATUS_ACCEPT}, nil } - return &cmtabci.ProcessProposalResponse{Status: status}, nil } diff --git a/errors/mod.go b/errors/errors.go similarity index 81% rename from errors/mod.go rename to errors/errors.go index 5b369e80fd..803a7dbcb6 100644 --- a/errors/mod.go +++ b/errors/errors.go @@ -58,12 +58,14 @@ type DetailedError struct { fatal bool } -// WrapNonFatal returns the error message. -func WrapNonFatal(err error) error { - return &DetailedError{ - error: err, - fatal: false, - } +// Error returns a string representation. +func (e *DetailedError) Error() string { + return e.error.Error() +} + +// Unwrap returns the wrapped error. +func (e *DetailedError) Unwrap() error { + return e.error } // WrapFatal creates a new DetailedError with the @@ -78,7 +80,7 @@ func WrapFatal(err error) error { // IsFatal checks if the provided error is a // DetailedError and if it is fatal. func IsFatal(err error) bool { - // If the error is nil, obviouisly it is not fatal. + // If the error is nil, obviously it is not fatal. if err == nil { return false } @@ -100,23 +102,6 @@ func IsFatal(err error) bool { return customErr.fatal } - // All other errors are fatal. - return true -} - -// JoinFatal checks if any of the provided errors is a -// DetailedError and if it is fatal. -func JoinFatal(errs ...error) error { - fatal := false - for _, err := range errs { - if IsFatal(err) { - fatal = true - break - } - } - retErr := stderrors.Join(errs...) - if fatal { - return WrapFatal(retErr) - } - return WrapNonFatal(retErr) + // Only declared fatal errors are fatal. + return false } diff --git a/execution/engine/engine.go b/execution/engine/engine.go index 6bc2bf9293..32217d11a1 100644 --- a/execution/engine/engine.go +++ b/execution/engine/engine.go @@ -139,7 +139,7 @@ func (ee *Engine) NotifyForkchoiceUpdate( "err", err, ) ee.metrics.markForkchoiceUpdateFatalError(err) - return nil, backoff.Permanent(err) + return nil, backoff.Permanent(errors.WrapFatal(err)) default: ee.logger.Info( @@ -245,7 +245,7 @@ func (ee *Engine) NotifyNewPayload( lastValidHash = &common.ExecutionHash{} } ee.metrics.markNewPayloadFatalError(payloadHash, *lastValidHash, err) - return nil, backoff.Permanent(err) + return nil, backoff.Permanent(errors.WrapFatal(err)) default: ee.logger.Error( "NotifyNewPayload: EL returns unknown error.", diff --git a/testing/simulated/chaos_test.go b/testing/simulated/chaos_test.go index a021a505e3..29dc95d3fa 100644 --- a/testing/simulated/chaos_test.go +++ b/testing/simulated/chaos_test.go @@ -73,8 +73,8 @@ func (s *SimulatedSuite) TestProcessProposal_CrashedExecutionClient_Errors() { ProposerAddress: pubkey.Address(), Time: proposalTime, }) - s.Require().NoError(err) - s.Require().Equal(types.PROCESS_PROPOSAL_STATUS_REJECT, processResp.Status) + s.Require().Error(err) + s.Require().Nil(processResp) s.Require().Contains(s.LogBuffer.String(), client.ErrBadConnection.Error()) }