*: strenghten checks for fetching deposits from API

app/obolapi/deposit.go

@@ -14,6 +14,7 @@
 
 	"github.com/obolnetwork/charon/app/errors"
 	"github.com/obolnetwork/charon/app/z"
+	"github.com/obolnetwork/charon/eth2util/deposit"
 	"github.com/obolnetwork/charon/tbls"
 	"github.com/obolnetwork/charon/tbls/tblsconv"
 )
@@ -75,9 +76,16 @@
 	return nil
 }
 
-// GetFullDeposit gets the full deposit message for a given validator public key, lock hash and share index.
+// GetFullDeposit gets the full deposit messages for a given validator public key and lock hash.
+// threshold is the minimum number of valid partial signatures required per amount group to aggregate.
+// partialPubKeys is the validator's ordered list of public-key shares from the cluster lock; it
+// resolves each partial signature's true share index via its PartialPublicKey.
+// network is the eth2 network name used to derive the deposit signing domain; it BLS-verifies each
+// partial signature against its claimed share and the final aggregated signature against the
+// validator's group public key, so a misreported PartialPublicKey or share index cannot poison the
+// aggregated signature.
 // It respects the timeout specified in the Client instance.
-func (c Client) GetFullDeposit(ctx context.Context, valPubkey string, lockHash []byte, threshold int, partialPubKeys [][]byte) ([]eth2p0.DepositData, error) {
+func (c Client) GetFullDeposit(ctx context.Context, valPubkey string, lockHash []byte, threshold int, partialPubKeys [][]byte, network string) ([]eth2p0.DepositData, error) {
 	valPubkeyBytes, err := from0x(valPubkey, len(eth2p0.BLSPubKey{}))
 	if err != nil {
 		return []eth2p0.DepositData{}, errors.Wrap(err, "validator pubkey to bytes")
@@ -124,25 +132,28 @@
 	for _, am := range dr.Amounts {
 		rawSignatures := make(map[int]tbls.Signature)
 
-		if len(am.Partials) < threshold {
-			submittedPubKeys := []string{}
-			for _, sigStr := range am.Partials {
-				submittedPubKeys = append(submittedPubKeys, sigStr.PartialPublicKey)
-			}
+		amountUint, err := strconv.ParseUint(am.Amount, 10, 64)
+		if err != nil {
+			return []eth2p0.DepositData{}, errors.Wrap(err, "parse amount to uint")
+		}
+
+		depositMsg := eth2p0.DepositMessage{
+			PublicKey:             eth2p0.BLSPubKey(valPubkeyBytes),
+			WithdrawalCredentials: withdrawalCredentialsBytes,
+			Amount:                eth2p0.Gwei(amountUint),
+		}
 
-			return []eth2p0.DepositData{}, errors.New("not enough partial signatures to meet threshold", z.Any("submitted_public_keys", submittedPubKeys), z.Int("submitted_public_keys_length", len(submittedPubKeys)), z.Int("required_threshold", threshold))
+		sigRoot, err := deposit.GetMessageSigningRoot(depositMsg, network)
+		if err != nil {
+			return []eth2p0.DepositData{}, errors.Wrap(err, "compute deposit message signing root")
 		}
 
-		for sigIdx, sigStr := range am.Partials {
+		for _, sigStr := range am.Partials {
 			if len(sigStr.PartialDepositSignature) == 0 {
 				// ignore, the associated share index didn't push a partial signature yet
 				continue
 			}
 
-			if len(sigStr.PartialDepositSignature) < 2 {
-				return []eth2p0.DepositData{}, errors.New("signature string has invalid size", z.Int("size", len(sigStr.PartialDepositSignature)))
-			}
-
 			sigBytes, err := from0x(sigStr.PartialDepositSignature, 96) // a signature is 96 bytes long
 			if err != nil {
 				return []eth2p0.DepositData{}, errors.Wrap(err, "partial signature unmarshal")
@@ -153,34 +164,61 @@
 				return []eth2p0.DepositData{}, errors.Wrap(err, "invalid partial signature")
 			}
 
-			shareIdx := sigIdx + 1
+			pk := strings.TrimPrefix(strings.ToLower(sigStr.PartialPublicKey), "0x")
+			if pk == "" {
+				return []eth2p0.DepositData{}, errors.New("partial public key missing from Obol API response")
+			}
 
-			if pk := strings.TrimPrefix(strings.ToLower(sigStr.PartialPublicKey), "0x"); pk != "" {
-				idx, ok := partialPubKeyToIdx[pk]
-				if !ok {
-					return []eth2p0.DepositData{}, errors.New("partial public key not found in validator public shares", z.Str("partial_public_key", sigStr.PartialPublicKey))
-				}
+			shareIdx, ok := partialPubKeyToIdx[pk]
+			if !ok {
+				return []eth2p0.DepositData{}, errors.New("partial public key not found in validator public shares", z.Str("partial_public_key", sigStr.PartialPublicKey))
+			}
 
-				shareIdx = idx
+			pubShare, err := tblsconv.PubkeyFromBytes(partialPubKeys[shareIdx-1])
+			if err != nil {
+				return []eth2p0.DepositData{}, errors.Wrap(err, "invalid validator public share", z.Int("share_index", shareIdx))
+			}
+
+			if err := tbls.Verify(pubShare, sigRoot[:], sig); err != nil {
+				return []eth2p0.DepositData{}, errors.Wrap(err, "partial deposit signature failed BLS verification", z.Int("share_index", shareIdx), z.Str("partial_public_key", sigStr.PartialPublicKey))
+			}
+
+			if _, dup := rawSignatures[shareIdx]; dup {
+				return []eth2p0.DepositData{}, errors.New("duplicate partial signature for share index", z.Int("share_index", shareIdx))
 			}
 
 			rawSignatures[shareIdx] = sig
 		}
 
+		// Threshold check happens after the loop because entries with empty PartialDepositSignature
+		// are skipped; the count of *usable* sigs is what matters, not the size of am.Partials.
+		if len(rawSignatures) < threshold {
+			submittedPubKeys := make([]string, 0, len(am.Partials))
+			for _, sigStr := range am.Partials {
+				submittedPubKeys = append(submittedPubKeys, sigStr.PartialPublicKey)
+			}
+
+			return []eth2p0.DepositData{}, errors.New("not enough partial signatures to meet threshold", z.Any("submitted_public_keys", submittedPubKeys), z.Int("valid_signatures", len(rawSignatures)), z.Int("required_threshold", threshold))
+		}
+
 		fullSig, err := tbls.ThresholdAggregate(rawSignatures)
 		if err != nil {
 			return []eth2p0.DepositData{}, errors.Wrap(err, "partial signatures threshold aggregate")
 		}
 
-		amountUint, err := strconv.ParseUint(am.Amount, 10, 64)
+		valPubKey, err := tblsconv.PubkeyFromBytes(valPubkeyBytes)
 		if err != nil {
-			return []eth2p0.DepositData{}, errors.Wrap(err, "parse amount to uint")
+			return []eth2p0.DepositData{}, errors.Wrap(err, "invalid validator public key")
+		}
+
+		if err := tbls.Verify(valPubKey, sigRoot[:], fullSig); err != nil {
+			return []eth2p0.DepositData{}, errors.Wrap(err, "aggregated deposit signature failed BLS verification", z.Str("validator_pubkey", valPubkey), z.U64("amount", uint64(depositMsg.Amount)))
 		}
 
 		fullDeposits = append(fullDeposits, eth2p0.DepositData{
-			PublicKey:             eth2p0.BLSPubKey(valPubkeyBytes),
-			WithdrawalCredentials: withdrawalCredentialsBytes,
-			Amount:                eth2p0.Gwei(amountUint),
+			PublicKey:             depositMsg.PublicKey,
+			WithdrawalCredentials: depositMsg.WithdrawalCredentials,
+			Amount:                depositMsg.Amount,
 			Signature:             eth2p0.BLSSignature(fullSig),
 		})
 	}

app/obolapi/deposit_test.go

@@ -5,8 +5,12 @@ package obolapi_test
 import (
 	"context"
 	"encoding/hex"
+	"encoding/json"
+	"fmt"
 	"math/rand"
+	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"testing"
 
 	eth2p0 "github.com/attestantio/go-eth2-client/spec/phase0"
@@ -91,6 +95,151 @@ func TestAPIDeposit(t *testing.T) {
 		require.NoError(t, cl.PostPartialDeposits(t.Context(), lock.LockHash, shareIndex, []eth2p0.DepositData{depositData}), "share index: %d", shareIndex)
 	}
 
-	_, err = cl.GetFullDeposit(t.Context(), lock.Validators[0].PublicKeyHex(), lock.LockHash, lock.Threshold, lock.Validators[0].PubShares)
+	_, err = cl.GetFullDeposit(t.Context(), lock.Validators[0].PublicKeyHex(), lock.LockHash, lock.Threshold, lock.Validators[0].PubShares, network)
 	require.NoError(t, err, "full deposit")
 }
+
+// depositFixture builds a 3-of-5 cluster, signs a full set of partial deposit signatures, and
+// assembles a baseline FullDepositResponse. Negative-path tests tamper one field of the returned
+// response before serving it from a canned httptest.Server.
+type depositFixture struct {
+	lock     cluster.Lock
+	network  string
+	response obolapi.FullDepositResponse
+}
+
+func newDepositFixture(t *testing.T) depositFixture {
+	t.Helper()
+
+	const (
+		numNodes  = 5
+		threshold = 3
+	)
+
+	random := rand.New(rand.NewSource(int64(0)))
+
+	lock, _, shares := cluster.NewForT(t, 1, threshold, numNodes, 0, random)
+
+	wc, err := hex.DecodeString("010000000000000000000000000000000000000000000000000000000000dead")
+	require.NoError(t, err)
+
+	depositMsg := eth2p0.DepositMessage{
+		PublicKey:             eth2p0.BLSPubKey(lock.Validators[0].PubKey),
+		WithdrawalCredentials: wc,
+		Amount:                eth2p0.Gwei(deposit.OneEthInGwei * 32),
+	}
+
+	network, err := eth2util.ForkVersionToNetwork(lock.ForkVersion)
+	require.NoError(t, err)
+
+	sigRoot, err := deposit.GetMessageSigningRoot(depositMsg, network)
+	require.NoError(t, err)
+
+	partials := make([]obolapi.Partial, 0, numNodes)
+
+	for shareIdx, share := range shares[0] {
+		sig, err := tbls.Sign(share, sigRoot[:])
+		require.NoError(t, err)
+
+		partials = append(partials, obolapi.Partial{
+			PartialPublicKey:        "0x" + hex.EncodeToString(lock.Validators[0].PubShares[shareIdx]),
+			PartialDepositSignature: fmt.Sprintf("%#x", sig[:]),
+		})
+	}
+
+	return depositFixture{
+		lock:    lock,
+		network: network,
+		response: obolapi.FullDepositResponse{
+			PublicKey:             lock.Validators[0].PublicKeyHex(),
+			WithdrawalCredentials: "0x" + hex.EncodeToString(wc),
+			Amounts: []obolapi.Amount{{
+				Amount:   strconv.FormatUint(uint64(depositMsg.Amount), 10),
+				Partials: partials,
+			}},
+		},
+	}
+}
+
+// serveCannedDeposit starts an httptest.Server that always returns the provided response JSON.
+func serveCannedDeposit(t *testing.T, resp obolapi.FullDepositResponse) *httptest.Server {
+	t.Helper()
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		require.NoError(t, json.NewEncoder(w).Encode(resp))
+	}))
+	t.Cleanup(srv.Close)
+
+	return srv
+}
+
+func TestGetFullDeposit_MissingPartialPubkey(t *testing.T) {
+	f := newDepositFixture(t)
+	f.response.Amounts[0].Partials[1].PartialPublicKey = ""
+
+	srv := serveCannedDeposit(t, f.response)
+
+	cl, err := obolapi.New(srv.URL)
+	require.NoError(t, err)
+
+	_, err = cl.GetFullDeposit(t.Context(), f.lock.Validators[0].PublicKeyHex(), f.lock.LockHash, f.lock.Threshold, f.lock.Validators[0].PubShares, f.network)
+	require.ErrorContains(t, err, "partial public key missing from Obol API response")
+}
+
+func TestGetFullDeposit_UnknownPartialPubkey(t *testing.T) {
+	f := newDepositFixture(t)
+	// Replace with a syntactically valid 48-byte pubkey that isn't one of the validator's shares.
+	f.response.Amounts[0].Partials[1].PartialPublicKey = "0x" + hex.EncodeToString(make([]byte, 48))
+
+	srv := serveCannedDeposit(t, f.response)
+
+	cl, err := obolapi.New(srv.URL)
+	require.NoError(t, err)
+
+	_, err = cl.GetFullDeposit(t.Context(), f.lock.Validators[0].PublicKeyHex(), f.lock.LockHash, f.lock.Threshold, f.lock.Validators[0].PubShares, f.network)
+	require.ErrorContains(t, err, "partial public key not found in validator public shares")
+}
+
+func TestGetFullDeposit_TamperedPartialSignature(t *testing.T) {
+	f := newDepositFixture(t)
+	// Swap signatures of shares 1 and 2: each is valid in isolation, but no longer matches its claimed share.
+	f.response.Amounts[0].Partials[0].PartialDepositSignature, f.response.Amounts[0].Partials[1].PartialDepositSignature = f.response.Amounts[0].Partials[1].PartialDepositSignature, f.response.Amounts[0].Partials[0].PartialDepositSignature
+
+	srv := serveCannedDeposit(t, f.response)
+
+	cl, err := obolapi.New(srv.URL)
+	require.NoError(t, err)
+
+	_, err = cl.GetFullDeposit(t.Context(), f.lock.Validators[0].PublicKeyHex(), f.lock.LockHash, f.lock.Threshold, f.lock.Validators[0].PubShares, f.network)
+	require.ErrorContains(t, err, "partial deposit signature failed BLS verification")
+}
+
+func TestGetFullDeposit_DuplicateShareIndex(t *testing.T) {
+	f := newDepositFixture(t)
+	// Make two entries claim the same share: copy partial[0] over partial[1].
+	f.response.Amounts[0].Partials[1] = f.response.Amounts[0].Partials[0]
+
+	srv := serveCannedDeposit(t, f.response)
+
+	cl, err := obolapi.New(srv.URL)
+	require.NoError(t, err)
+
+	_, err = cl.GetFullDeposit(t.Context(), f.lock.Validators[0].PublicKeyHex(), f.lock.LockHash, f.lock.Threshold, f.lock.Validators[0].PubShares, f.network)
+	require.ErrorContains(t, err, "duplicate partial signature for share index")
+}
+
+func TestGetFullDeposit_NotEnoughSignatures(t *testing.T) {
+	f := newDepositFixture(t)
+	// Blank out enough partial signatures to drop below threshold (3 of 5 → blank 3).
+	for i := range 3 {
+		f.response.Amounts[0].Partials[i].PartialDepositSignature = ""
+	}
+
+	srv := serveCannedDeposit(t, f.response)
+
+	cl, err := obolapi.New(srv.URL)
+	require.NoError(t, err)
+
+	_, err = cl.GetFullDeposit(t.Context(), f.lock.Validators[0].PublicKeyHex(), f.lock.LockHash, f.lock.Threshold, f.lock.Validators[0].PubShares, f.network)
+	require.ErrorContains(t, err, "not enough partial signatures to meet threshold")
+}

cmd/depositfetch.go

@@ -67,6 +67,11 @@ func runDepositFetch(ctx context.Context, config depositFetchConfig) error {
 		return errors.Wrap(err, "create Obol API client", z.Str("publish_address", config.PublishAddress))
 	}
 
+	network, err := eth2util.ForkVersionToNetwork(cl.ForkVersion)
+	if err != nil {
+		return err
+	}
+
 	depositDatas := map[eth2p0.Gwei][]eth2p0.DepositData{}
 
 	for _, pubkey := range config.ValidatorPublicKeys {
@@ -75,7 +80,7 @@ func runDepositFetch(ctx context.Context, config depositFetchConfig) error {
 		var pubShares [][]byte
 
 		for _, v := range cl.Validators {
-			if v.PublicKeyHex() == pubkey {
+			if strings.EqualFold(v.PublicKeyHex(), pubkey) {
 				pubShares = v.PubShares
 				break
 			}
@@ -85,7 +90,7 @@ func runDepositFetch(ctx context.Context, config depositFetchConfig) error {
 			return errors.New("validator public key not found in cluster lock", z.Str("validator_pubkey", pubkey))
 		}
 
-		dd, err := oAPI.GetFullDeposit(ctx, pubkey, cl.LockHash, cl.Threshold, pubShares)
+		dd, err := oAPI.GetFullDeposit(ctx, pubkey, cl.LockHash, cl.Threshold, pubShares, network)
 		if err != nil {
 			return errors.Wrap(err, "fetch full deposit data from Obol API")
 		}
@@ -108,11 +113,6 @@ func runDepositFetch(ctx context.Context, config depositFetchConfig) error {
 		return errors.Wrap(err, "create deposit data dir")
 	}
 
-	network, err := eth2util.ForkVersionToNetwork(cl.ForkVersion)
-	if err != nil {
-		return err
-	}
-
 	for _, depositDatas := range depositDatas {
 		err = deposit.WriteDepositDataFile(depositDatas, network, path)
 		if err != nil {