diff --git a/app/obolapi/deposit.go b/app/obolapi/deposit.go index 66a4a811c..8cc94dbca 100644 --- a/app/obolapi/deposit.go +++ b/app/obolapi/deposit.go @@ -14,6 +14,7 @@ import ( "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 @@ func (c Client) PostPartialDeposits(ctx context.Context, lockHash []byte, shareI 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 @@ func (c Client) GetFullDeposit(ctx context.Context, valPubkey string, lockHash [ 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 @@ func (c Client) GetFullDeposit(ctx context.Context, valPubkey string, lockHash [ 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), }) } diff --git a/app/obolapi/deposit_test.go b/app/obolapi/deposit_test.go index 529362196..d61faa72b 100644 --- a/app/obolapi/deposit_test.go +++ b/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") +} diff --git a/cmd/depositfetch.go b/cmd/depositfetch.go index b92e81673..7a65d951e 100644 --- a/cmd/depositfetch.go +++ b/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 {