Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions app/obolapi/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@

// GetFullDeposit gets the full deposit message for a given validator public key, lock hash and share index.
// It respects the timeout specified in the Client instance.
func (c Client) GetFullDeposit(ctx context.Context, valPubkey string, lockHash []byte, threshold int) ([]eth2p0.DepositData, error) {
func (c Client) GetFullDeposit(ctx context.Context, valPubkey string, lockHash []byte, threshold int, partialPubKeys [][]byte) ([]eth2p0.DepositData, error) {

Check failure on line 80 in app/obolapi/deposit.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 37 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=ObolNetwork_charon&issues=AZ48-0IPIe9tYq10aMbS&open=AZ48-0IPIe9tYq10aMbS&pullRequest=4526
valPubkeyBytes, err := from0x(valPubkey, len(eth2p0.BLSPubKey{}))
if err != nil {
return []eth2p0.DepositData{}, errors.Wrap(err, "validator pubkey to bytes")
Expand Down Expand Up @@ -112,6 +112,12 @@
return []eth2p0.DepositData{}, errors.Wrap(err, "withdrawal credentials to bytes")
}

// Build a hex-encoded public-share -> 1-based share-index lookup so we can resolve the correct index.
partialPubKeyToIdx := make(map[string]int, len(partialPubKeys))
for i, parPubKey := range partialPubKeys {
partialPubKeyToIdx[hex.EncodeToString(parPubKey)] = i + 1
}

// do aggregation
fullDeposits := []eth2p0.DepositData{}

Expand Down Expand Up @@ -147,7 +153,18 @@
return []eth2p0.DepositData{}, errors.Wrap(err, "invalid partial signature")
}

rawSignatures[sigIdx+1] = sig
shareIdx := sigIdx + 1

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 = idx
}

rawSignatures[shareIdx] = sig
}

fullSig, err := tbls.ThresholdAggregate(rawSignatures)
Expand Down
3 changes: 1 addition & 2 deletions app/obolapi/deposit_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ type PartialDepositRequest struct {

// FullDepositResponse contains all partial signatures, public key, amounts and withdrawal credentials to construct
// a full deposit message for a validator.
// Signatures are ordered by share index.
type FullDepositResponse struct {
PublicKey string `json:"public_key"`
PublicKey string `json:"pubkey"`
Comment thread
KaloyanTanev marked this conversation as resolved.
WithdrawalCredentials string `json:"withdrawal_credentials"`
Amounts []Amount `json:"amounts"`
}
Expand Down
24 changes: 15 additions & 9 deletions app/obolapi/deposit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ import (
)

func TestAPIDeposit(t *testing.T) {
kn := 4
// Use a 3-of-5 cluster and submit from non-contiguous nodes (2, 4, 5) to
// exercise correct share-index mapping in ThresholdAggregate.
const (
numNodes = 5
threshold = 3
)

beaconMock, err := beaconmock.New(t.Context())
require.NoError(t, err)
Expand All @@ -40,11 +45,11 @@ func TestAPIDeposit(t *testing.T) {

random := rand.New(rand.NewSource(int64(0)))

lock, peers, shares := cluster.NewForT(
lock, _, shares := cluster.NewForT(
t,
1,
kn,
kn,
threshold,
numNodes,
0,
random,
)
Expand All @@ -69,7 +74,10 @@ func TestAPIDeposit(t *testing.T) {
cl, err := obolapi.New(srv.URL)
require.NoError(t, err)

for idx := range len(peers) {
// Submit from nodes 2, 4, 5 (0-indexed: 1, 3, 4) — non-contiguous share indices.
for _, idx := range []int{1, 3, 4} {
shareIndex := uint64(idx + 1)

signature, err := tbls.Sign(shares[0][idx], depositMessageSigRoot[:])
require.NoError(t, err)

Expand All @@ -80,11 +88,9 @@ func TestAPIDeposit(t *testing.T) {
Signature: eth2p0.BLSSignature(signature),
}

// send all the partial deposits
require.NoError(t, cl.PostPartialDeposits(t.Context(), lock.LockHash, uint64(idx+1), []eth2p0.DepositData{depositData}), "share index: %d", idx+1)
require.NoError(t, cl.PostPartialDeposits(t.Context(), lock.LockHash, shareIndex, []eth2p0.DepositData{depositData}), "share index: %d", shareIndex)
}

// get full exit
_, err = cl.GetFullDeposit(t.Context(), lock.Validators[0].PublicKeyHex(), lock.LockHash, lock.Threshold)
_, err = cl.GetFullDeposit(t.Context(), lock.Validators[0].PublicKeyHex(), lock.LockHash, lock.Threshold, lock.Validators[0].PubShares)
require.NoError(t, err, "full deposit")
}
1 change: 1 addition & 0 deletions cmd/cmd_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ func TestFlagsToLogFieldsRedactsHeaders(t *testing.T) {
if f.Key != "beacon-node-headers" {
return
}

require.NotContains(t, f.String, "b2JvbDpzZWNyZXQ=")
require.Contains(t, f.String, "Authorization=xxxxx")
})
Expand Down
15 changes: 14 additions & 1 deletion cmd/depositfetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
cmd.Flags().StringVar(&config.DepositDataDir, "deposit-data-dir", defaultDepositDataDir, "Path to the directory in which fetched deposit data will be stored.")
}

func runDepositFetch(ctx context.Context, config depositFetchConfig) error {

Check failure on line 59 in cmd/depositfetch.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 22 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=ObolNetwork_charon&issues=AZ48-0LbIe9tYq10aMbT&open=AZ48-0LbIe9tYq10aMbT&pullRequest=4526
cl, err := cluster.LoadClusterLockAndVerify(ctx, config.LockFilePath)
if err != nil {
return err
Expand All @@ -72,7 +72,20 @@
for _, pubkey := range config.ValidatorPublicKeys {
log.Info(ctx, "Fetching full deposit message", z.Str("validator_pubkey", pubkey))

dd, err := oAPI.GetFullDeposit(ctx, pubkey, cl.LockHash, cl.Threshold)
var pubShares [][]byte

for _, v := range cl.Validators {
if v.PublicKeyHex() == pubkey {
pubShares = v.PubShares
break
}
}

Comment thread
KaloyanTanev marked this conversation as resolved.
if len(pubShares) == 0 {
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)
if err != nil {
return errors.Wrap(err, "fetch full deposit data from Obol API")
}
Expand Down
9 changes: 8 additions & 1 deletion eth2util/deposit/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,14 @@ func MarshalDepositData(depositDatas []eth2p0.DepositData, network string) ([]by

err = tbls.Verify(blsPubkey, sigData[:], blsSig)
if err != nil {
return nil, errors.Wrap(err, "invalid deposit data signature")
return nil, errors.Wrap(err, "invalid deposit data signature",
z.Str("pubkey", hex.EncodeToString(depositData.PublicKey[:])),
z.Str("signature", hex.EncodeToString(depositData.Signature[:])),
z.Str("msg.pubkey", hex.EncodeToString(msg.PublicKey[:])),
z.Str("msg.withdrawal_credentials", hex.EncodeToString(msg.WithdrawalCredentials)),
z.U64("msg.amount", uint64(msg.Amount)),
z.Str("network", network),
)
}

dataRoot, err := depositData.HashTreeRoot()
Expand Down
42 changes: 17 additions & 25 deletions testutil/obolapimock/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@ const (
fetchFullDepositTmpl = "/deposit_data/" + lockHashPath + "/" + valPubkeyPath
)

// depositBlob represents an Obol API DepositBlob with its share index.
// depositBlob represents an Obol API DepositBlob.
type depositBlob struct {
obolapi.FullDepositResponse

shareIdx uint64
}

func (ts *testServer) HandleSubmitPartialDeposit(writer http.ResponseWriter, request *http.Request) {
Expand Down Expand Up @@ -112,9 +110,14 @@ func (ts *testServer) HandleSubmitPartialDeposit(writer http.ResponseWriter, req
return
}

newPartial := obolapi.Partial{
PartialPublicKey: hex.EncodeToString(publicKeyShare[:]),
PartialDepositSignature: depositData.Signature.String(),
}

existingDeposit, ok := ts.partialDeposits[depositData.PublicKey.String()]

amounts := []obolapi.Amount{}
var amounts []obolapi.Amount
if ok {
amounts = existingDeposit.Amounts
}
Expand All @@ -123,36 +126,25 @@ func (ts *testServer) HandleSubmitPartialDeposit(writer http.ResponseWriter, req

for idx, amt := range amounts {
if amt.Amount == strconv.FormatUint(uint64(depositData.Amount), 10) {
amt.Partials = append(amt.Partials, obolapi.Partial{
PartialDepositSignature: depositData.Signature.String(),
PartialPublicKey: "",
})
amt.Partials = append(amt.Partials, newPartial)
amounts[idx] = amt
amtFound = true
}
}

existingDeposit.Amounts = amounts

if !amtFound {
amounts = append(amounts, obolapi.Amount{
Amount: strconv.FormatUint(uint64(depositData.Amount), 10),
Partials: []obolapi.Partial{
{
PartialDepositSignature: depositData.Signature.String(),
PartialPublicKey: "",
},
},
Amount: strconv.FormatUint(uint64(depositData.Amount), 10),
Partials: []obolapi.Partial{newPartial},
})
}

ts.partialDeposits[depositData.PublicKey.String()] = depositBlob{
FullDepositResponse: obolapi.FullDepositResponse{
PublicKey: depositData.PublicKey.String(),
WithdrawalCredentials: hex.EncodeToString(depositData.WithdrawalCredentials),
Amounts: amounts,
},
shareIdx: shareIndex,
}
ts.partialDeposits[depositData.PublicKey.String()] = depositBlob{
FullDepositResponse: obolapi.FullDepositResponse{
PublicKey: depositData.PublicKey.String(),
WithdrawalCredentials: hex.EncodeToString(depositData.WithdrawalCredentials),
Amounts: amounts,
},
}
}

Expand Down
Loading