Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Aggregated Public Key Method #14178

Merged
merged 5 commits into from
Jul 8, 2024
Merged

Add Aggregated Public Key Method #14178

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c32a9ba
Add aggregated key method
nisdas Jul 3, 2024
9bf8b78
Gazelle
nisdas Jul 3, 2024
633bf4f
Merge branch 'develop' of https://github.com/prysmaticlabs/geth-shard…
nisdas Jul 3, 2024
d3c3ad2
Manu's Review
nisdas Jul 3, 2024
2d715a0
Merge branch 'develop' into aggKeyMethod
nisdas Jul 8, 2024
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
7 changes: 1 addition & 6 deletions beacon-chain/core/blocks/signature.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,7 @@ func createAttestationSignatureBatch(
return nil, err
}
indices := ia.GetAttestingIndices()
pubkeys := make([][]byte, len(indices))
for i := 0; i < len(indices); i++ {
pubkeyAtIdx := beaconState.PubkeyAtIndex(primitives.ValidatorIndex(indices[i]))
pubkeys[i] = pubkeyAtIdx[:]
}
aggP, err := bls.AggregatePublicKeys(pubkeys)
aggP, err := beaconState.AggregatedKeyFromIndices(indices)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The previous function is AggregatePublicKeys. Should'nt this function name AggregateKeyFromIndices?

if err != nil {
return nil, err
}
Expand Down
1 change: 1 addition & 0 deletions beacon-chain/state/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ go_library(
"//config/fieldparams:go_default_library",
"//consensus-types/interfaces:go_default_library",
"//consensus-types/primitives:go_default_library",
"//crypto/bls:go_default_library",
"//proto/engine/v1:go_default_library",
"//proto/prysm/v1alpha1:go_default_library",
"@com_github_prometheus_client_golang//prometheus:go_default_library",
Expand Down
2 changes: 2 additions & 0 deletions beacon-chain/state/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v5/crypto/bls"
enginev1 "github.com/prysmaticlabs/prysm/v5/proto/engine/v1"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
)
Expand Down Expand Up @@ -129,6 +130,7 @@ type ReadOnlyValidators interface {
ValidatorIndexByPubkey(key [fieldparams.BLSPubkeyLength]byte) (primitives.ValidatorIndex, bool)
PublicKeys() ([][fieldparams.BLSPubkeyLength]byte, error)
PubkeyAtIndex(idx primitives.ValidatorIndex) [fieldparams.BLSPubkeyLength]byte
AggregatedKeyFromIndices(idxs []uint64) (bls.PublicKey, error)
NumValidators() int
ReadFromEveryValidator(f func(idx int, val ReadOnlyValidator) error) error
}
Expand Down
2 changes: 2 additions & 0 deletions beacon-chain/state/state-native/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ go_library(
"//consensus-types/primitives:go_default_library",
"//container/multi-value-slice:go_default_library",
"//container/slice:go_default_library",
"//crypto/bls:go_default_library",
"//crypto/hash:go_default_library",
"//encoding/bytesutil:go_default_library",
"//encoding/ssz:go_default_library",
Expand Down Expand Up @@ -141,6 +142,7 @@ go_test(
"//consensus-types/interfaces:go_default_library",
"//consensus-types/primitives:go_default_library",
"//container/trie:go_default_library",
"//crypto/bls:go_default_library",
"//crypto/rand:go_default_library",
"//encoding/bytesutil:go_default_library",
"//proto/engine/v1:go_default_library",
Expand Down
31 changes: 31 additions & 0 deletions beacon-chain/state/state-native/getters_validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/prysmaticlabs/prysm/v5/config/params"
consensus_types "github.com/prysmaticlabs/prysm/v5/consensus-types"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v5/crypto/bls"
"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v5/runtime/version"
Expand Down Expand Up @@ -223,6 +224,36 @@ func (b *BeaconState) PubkeyAtIndex(idx primitives.ValidatorIndex) [fieldparams.
return bytesutil.ToBytes48(v.PublicKey)
}

// AggregatedKeyFromIndices builds an aggregated public key from the provided
// validator indices.
func (b *BeaconState) AggregatedKeyFromIndices(idxs []uint64) (bls.PublicKey, error) {
b.lock.RLock()
defer b.lock.RUnlock()

pubKeys := make([][]byte, len(idxs))
for i, idx := range idxs {
var v *ethpb.Validator
if features.Get().EnableExperimentalState {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case is not tested at all, is it expected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its fine in this particular case as its simply for the experimental state, we don't do the same for a few other methods too. Also the experimental state has been tested in e2e, so these paths are tested, just not in the current unit tests. Once we flip the flag, it will get tested as it becomes the default path

var err error
v, err = b.validatorsMultiValue.At(b, idx)
if err != nil {
return nil, err
}
} else {
if idx >= uint64(len(b.validators)) {
return nil, consensus_types.ErrOutOfBounds
}
v = b.validators[idx]
}

if v == nil {
return nil, ErrNilWrappedValidator
}
pubKeys[i] = v.PublicKey
}
return bls.AggregatePublicKeys(pubKeys)
}

// PublicKeys builds a list of all validator public keys, with each key's index aligned to its validator index.
func (b *BeaconState) PublicKeys() ([][fieldparams.BLSPubkeyLength]byte, error) {
b.lock.RLock()
Expand Down
17 changes: 17 additions & 0 deletions beacon-chain/state/state-native/getters_validator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
testtmpl "github.com/prysmaticlabs/prysm/v5/beacon-chain/state/testing"
"github.com/prysmaticlabs/prysm/v5/config/params"
consensus_types "github.com/prysmaticlabs/prysm/v5/consensus-types"
"github.com/prysmaticlabs/prysm/v5/crypto/bls"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v5/testing/assert"
"github.com/prysmaticlabs/prysm/v5/testing/require"
"github.com/prysmaticlabs/prysm/v5/testing/util"
)
Expand Down Expand Up @@ -145,3 +147,18 @@ func TestPendingBalanceToWithdraw(t *testing.T) {
require.NoError(t, err)
require.Equal(t, uint64(600), ab)
}

func TestAggregatedKeyFromIndices(t *testing.T) {
dState, _ := util.DeterministicGenesisState(t, 10)
pKey1 := dState.PubkeyAtIndex(3)
pKey2 := dState.PubkeyAtIndex(7)
pKey3 := dState.PubkeyAtIndex(9)

aggKey, err := bls.AggregatePublicKeys([][]byte{pKey1[:], pKey2[:], pKey3[:]})
require.NoError(t, err)

retKey, err := dState.AggregatedKeyFromIndices([]uint64{3, 7, 9})
require.NoError(t, err)

assert.Equal(t, true, aggKey.Equals(retKey), "unequal aggregated keys")
}
Loading