core: addressed some panic instances (#2783)

Removed some unnecessary panic() calls and added recover() where appropriate.

category: refactor
ticket: none

core/proto.go

@@ -5,6 +5,7 @@ package core
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	ssz "github.com/ferranbt/fastssz"
@@ -44,10 +45,15 @@ func DutyFromProto(duty *pbv1.Duty) Duty {
 }
 
 // ParSignedDataFromProto returns the data from a protobuf.
-func ParSignedDataFromProto(typ DutyType, data *pbv1.ParSignedData) (ParSignedData, error) {
-	// TODO(corver): This can panic due to json unmarshalling unexpected data.
-	//  For now, it is a good way to catch compatibility issues. But we should
-	//  recover panics and return an error before launching mainnet.
+func ParSignedDataFromProto(typ DutyType, data *pbv1.ParSignedData) (odata ParSignedData, oerr error) {
+	defer func() {
+		// This is to respect the technical possibility of unmarshalling to panic.
+		// However, our protobuf generated types do not have custom marshallers that may panic.
+		if r := recover(); r != nil {
+			rowStr := fmt.Sprintf("%v", r)
+			oerr = errors.Wrap(errors.New(rowStr), "panic recovered")
+		}
+	}()
 
 	if err := protonil.Check(data); err != nil {
 		return ParSignedData{}, errors.Wrap(err, "invalid partial signed proto")

core/tracing.go

@@ -6,7 +6,7 @@ import (
 	"context"
 	"fmt"
 	"hash/fnv"
-	"math"
+	"strconv"
 	"strings"
 
 	eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0"
@@ -30,7 +30,8 @@ func StartDutyTrace(ctx context.Context, duty Duty, spanName string, opts ...tra
 	ctx, outerSpan = tracer.Start(tracer.RootedCtx(ctx, traceID), fmt.Sprintf("core/duty.%s", strings.Title(duty.Type.String())))
 	ctx, innerSpan = tracer.Start(ctx, spanName, opts...)
 
-	outerSpan.SetAttributes(attribute.Int64("slot", safeInt64(duty.Slot)))
+	slotStr := strconv.FormatUint(duty.Slot, 10)
+	outerSpan.SetAttributes(attribute.String("slot", slotStr))
 
 	return ctx, withEndSpan{
 		Span:    innerSpan,
@@ -134,13 +135,3 @@ func WithTracing() WireOption {
 		}
 	}
 }
-
-// safeInt64 converts the provided uint64 value to an int64 integer.
-// It panics if the provided value can't fit in an int64.
-func safeInt64(value uint64) int64 {
-	if value <= math.MaxInt64 {
-		return int64(value)
-	}
-
-	panic("integer overflow")
-}

eth2util/rlp/rlp.go

@@ -101,10 +101,9 @@ func decodeLength(item []byte) (offset int, length int, err error) {
 	}
 
 	if prefix < 0xc0 {
-		length = int(prefix - 0xb7)    // length of the string in bytes in binary form
-		if length > 8 || length <= 0 { // This is impossible based on outer if else checks
-			panic("length not in expected range [1,8]")
-		}
+		// Due to above checks, prefix will be >= 0xb8 and < 0xc0,
+		// therefore the length must be in [1,8]
+		length = int(prefix - 0xb7) // length of the string in bytes in binary form
 
 		offset := 1 + length
 
@@ -124,10 +123,9 @@ func decodeLength(item []byte) (offset int, length int, err error) {
 		return 1, int(prefix - 0xc0), nil
 	}
 
+	// Due to above checks, prefix will be >= 0xf8 and <= 0xff
+	// therefore the length must be in [1,8]
 	length = int(prefix - 0xf7)
-	if length > 8 || length <= 0 { // This is impossible based on outer if else checks
-		panic("length not in expected range [1,8]")
-	}
 
 	offset = 1 + length