From 1112e01c06a0e26fe03d40367e6af78f2471d479 Mon Sep 17 00:00:00 2001 From: Manu NALEPA Date: Mon, 4 Dec 2023 18:10:32 +0100 Subject: [PATCH] Make Prysm VC compatible with the version `v5.3.0` of the slashing protections interchange tests. (#13232) * `TestStore_GenesisValidatorsRoot_ReadAndWrite`: Make all test cases independents. In a test with multiple test cases, each test case should be independents. (aka: Removing test case `A` should not impact test case `B`) * `SaveGenesisValidatorsRoot`: Allow to overwrite the genesis validator root if the root is the same. * `ProposalHistoryForSlot`: Add `signingRootExists` Currently, it is not possible with `ProposalHistoryForSlot` to know if a proposal is stored with and `0x00000....` signing root or with an empty signing root. Both cases result to `proposalExists == true` and `signingRoot == 0x00000`. This commit adds a new return boolean: `signingRootExists`. If a proposal has been saved with a `0x00000...` signing root, then: - `proposalExists` is set to `true`, and - `signingRootExists` is set to `true`, and - `signingRoot` is set to `0x00000...` If a proposal has been saved with an empty signing root, then: - `proposalExists` is set to `true`, and - `signingRootExists` is set to `false`, and - (`signingRoot` is set to `0x00000...`) * `ImportStandardProtectionJSON`: When importing EIP-3076 Slashing Protection Interchange Format, do not filter any more slashable keys. Note: Those keys are still saved into the black-listed public keys list. There is two reason not to do so: - The EIP-3076 test cases do not know about Prysm's internal black-listed public keys list. Tests will expect, without looking into this internal black-listed public keys list, to deny a further signature. If we filter these keys from the DB (even if we keep them into the black-listed keys list), then some tests will fail. - If we import a interchange file containing slashable keys and we filter them, then, if we re-export the DB, those slashing offences won't appear in the exported interchange file. * `transformSignedBlocks`: Store an 0-len byte slice When importing an EIP-3076 interchange format, and when no signing root is specified into the file, we currently store a `0x00000.....` signing root. In such a case, instead storing `0x00000...`, this commit stores a 0-len byte array, so we can differentiate real `0x000.....` signing root and no signing-root at all. * `slashableProposalCheck`: Manage lack of sign root Currently, `slashableProposalCheck` does not really make a difference between a `0x0000.....` signing root and a missing signing root. (Signing roots can be missing when importing an EIP-3076 interchange file.) This commit differentiate, for `slashableProposalCheck`, `0x0000....` signing root and a missing signing root. * `AttestationRecord.SigningRoot`: ==> `[]byte` When importing attestations from EIP-3076 interchange format, the signing root of an attestation may be missing. Currently, Prysm consider any missing attestation signing root as `0x000...`. However, it may conflict with signing root which really are equal to `0x000...`. This commit transforms `AttestationRecord.SigningRoot` from `[32]byte` to `[]byte`, and change the minimal set of functions (sic) to support this new type. * `CheckSlashableAttestation`: Empty signing root Regarding slashing roots, 2 attestations are slashable, if: - both signing roots are defined and differs, or - one attestation exists, but without a signing root * `filterSlashablePubKeysFromAttestations`: Err sort Rergarding `CheckSlashableAttestation`, we consider that: - If slashable == NotSlashable and err != nil, then CheckSlashableAttestation failed. - If slashable != NotSlashable, then err contains the reason why the attestation is slashable. * `setupEIP3076SpecTests`: Update to `v5.3.0` This commit: - Updates the version of EIP-3076 tests to `v.5.2.1`. - Setups on anti-slashing DB per test case, instead per step. * `ImportStandardProtectionJSON`: Reduce cycl cmplxt * `AttestationHistoryForPubKey`: copy signing root BoltDB documentation specifies: | Byte slices returned from Bolt are only valid during a transaction. | Once the transaction has been committed or rolled back then the memory | they point to can be reused by a new page or can be unmapped | from virtual memory and you'll see an unexpected fault address panic | when accessing it. --- WORKSPACE | 4 +- proto/prysm/v1alpha1/slashings/BUILD.bazel | 6 +- .../prysm/v1alpha1/slashings/double_votes.go | 11 +-- .../v1alpha1/slashings/double_votes_test.go | 40 ++++++--- validator/client/attest_protect.go | 6 +- validator/client/propose_protect.go | 62 +++++++++----- validator/client/propose_protect_test.go | 4 +- .../slashing_protection_interchange_test.go | 34 ++++---- validator/client/validator.go | 2 +- validator/db/iface/interface.go | 8 +- validator/db/kv/attester_protection.go | 58 +++++++++---- validator/db/kv/attester_protection_test.go | 24 +++--- validator/db/kv/backup_test.go | 6 +- validator/db/kv/genesis.go | 3 +- validator/db/kv/genesis_test.go | 40 +++++++-- validator/db/kv/proposer_protection.go | 26 ++++-- validator/db/kv/proposer_protection_test.go | 34 ++++++-- .../slashing-protection-history/BUILD.bazel | 1 - .../slashing-protection-history/export.go | 6 +- .../slashing-protection-history/import.go | 81 ++++++++++++++----- .../import_test.go | 9 ++- .../round_trip_test.go | 8 +- validator/testing/protection_history.go | 2 +- 23 files changed, 319 insertions(+), 156 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index b59874616f..7e331592c2 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -244,8 +244,8 @@ filegroup( visibility = ["//visibility:public"], ) """, - sha256 = "91434d5fd5e1c6eb7b0174fed2afe25e09bddf00e1e4c431db931b2cee4e7773", - url = "https://github.com/eth-clients/slashing-protection-interchange-tests/archive/b8413ca42dc92308019d0d4db52c87e9e125c4e9.tar.gz", + sha256 = "516d551cfb3e50e4ac2f42db0992f4ceb573a7cb1616d727a725c8161485329f", + url = "https://github.com/eth-clients/slashing-protection-interchange-tests/archive/refs/tags/v5.3.0.tar.gz", ) http_archive( diff --git a/proto/prysm/v1alpha1/slashings/BUILD.bazel b/proto/prysm/v1alpha1/slashings/BUILD.bazel index 18a8f8ce0a..3e82d80365 100644 --- a/proto/prysm/v1alpha1/slashings/BUILD.bazel +++ b/proto/prysm/v1alpha1/slashings/BUILD.bazel @@ -8,10 +8,7 @@ go_library( ], importpath = "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1/slashings", visibility = ["//visibility:public"], - deps = [ - "//config/params:go_default_library", - "//proto/prysm/v1alpha1:go_default_library", - ], + deps = ["//proto/prysm/v1alpha1:go_default_library"], ) go_test( @@ -22,7 +19,6 @@ go_test( ], embed = [":go_default_library"], deps = [ - "//config/params:go_default_library", "//consensus-types/primitives:go_default_library", "//proto/prysm/v1alpha1:go_default_library", ], diff --git a/proto/prysm/v1alpha1/slashings/double_votes.go b/proto/prysm/v1alpha1/slashings/double_votes.go index 30c3b018b0..77db09bc72 100644 --- a/proto/prysm/v1alpha1/slashings/double_votes.go +++ b/proto/prysm/v1alpha1/slashings/double_votes.go @@ -1,16 +1,17 @@ package slashings -import "github.com/prysmaticlabs/prysm/v4/config/params" +import ( + "bytes" +) // SigningRootsDiffer verifies that an incoming vs. existing attestation has a different signing root. // If the existing signing root is empty, then we consider an attestation as different always. -func SigningRootsDiffer(existingSigningRoot, incomingSigningRoot [32]byte) bool { - zeroHash := params.BeaconConfig().ZeroHash +func SigningRootsDiffer(existingSigningRoot, incomingSigningRoot []byte) bool { // If the existing signing root is empty, we always consider the incoming // attestation as a double vote to be safe. - if existingSigningRoot == zeroHash { + if len(existingSigningRoot) == 0 { return true } // Otherwise, we consider any sort of inequality to be a double vote. - return existingSigningRoot != incomingSigningRoot + return !bytes.Equal(existingSigningRoot, incomingSigningRoot) } diff --git a/proto/prysm/v1alpha1/slashings/double_votes_test.go b/proto/prysm/v1alpha1/slashings/double_votes_test.go index 57769571de..6624cb1be6 100644 --- a/proto/prysm/v1alpha1/slashings/double_votes_test.go +++ b/proto/prysm/v1alpha1/slashings/double_votes_test.go @@ -2,14 +2,12 @@ package slashings import ( "testing" - - "github.com/prysmaticlabs/prysm/v4/config/params" ) func TestSigningRootsDiffer(t *testing.T) { type args struct { - existingSigningRoot [32]byte - incomingSigningRoot [32]byte + existingSigningRoot []byte + incomingSigningRoot []byte } tests := []struct { name string @@ -17,34 +15,50 @@ func TestSigningRootsDiffer(t *testing.T) { want bool }{ { - name: "Empty existing signing root is slashable", + name: "nil existing signing root is slashable", args: args{ - existingSigningRoot: params.BeaconConfig().ZeroHash, - incomingSigningRoot: [32]byte{1}, + existingSigningRoot: nil, + incomingSigningRoot: []byte{1}, + }, + want: true, + }, + { + name: "empty existing signing root is slashable", + args: args{ + existingSigningRoot: []byte{}, + incomingSigningRoot: []byte{1}, }, want: true, }, { name: "Non-empty, different existing signing root is slashable", args: args{ - existingSigningRoot: [32]byte{2}, - incomingSigningRoot: [32]byte{1}, + existingSigningRoot: []byte{2}, + incomingSigningRoot: []byte{1}, }, want: true, }, { name: "Non-empty, same existing signing root and incoming signing root is not slashable", args: args{ - existingSigningRoot: [32]byte{2}, - incomingSigningRoot: [32]byte{2}, + existingSigningRoot: []byte{2}, + incomingSigningRoot: []byte{2}, }, want: false, }, + { + name: "Both nil are considered slashable", + args: args{ + existingSigningRoot: nil, + incomingSigningRoot: nil, + }, + want: true, + }, { name: "Both empty are considered slashable", args: args{ - existingSigningRoot: params.BeaconConfig().ZeroHash, - incomingSigningRoot: params.BeaconConfig().ZeroHash, + existingSigningRoot: []byte{}, + incomingSigningRoot: []byte{}, }, want: true, }, diff --git a/validator/client/attest_protect.go b/validator/client/attest_protect.go index 63ed889f03..f9a273a8e8 100644 --- a/validator/client/attest_protect.go +++ b/validator/client/attest_protect.go @@ -22,11 +22,13 @@ func (v *validator) slashableAttestationCheck( ctx context.Context, indexedAtt *ethpb.IndexedAttestation, pubKey [fieldparams.BLSPubkeyLength]byte, - signingRoot [32]byte, + signingRoot32 [32]byte, ) error { ctx, span := trace.StartSpan(ctx, "validator.postAttSignUpdate") defer span.End() + signingRoot := signingRoot32[:] + // Based on EIP3076, validator should refuse to sign any attestation with source epoch less // than the minimum source epoch present in that signer’s attestations. lowestSourceEpoch, exists, err := v.db.LowestSignedSourceEpoch(ctx, pubKey) @@ -76,7 +78,7 @@ func (v *validator) slashableAttestationCheck( return errors.Wrap(err, failedAttLocalProtectionErr) } - if err := v.db.SaveAttestationForPubKey(ctx, pubKey, signingRoot, indexedAtt); err != nil { + if err := v.db.SaveAttestationForPubKey(ctx, pubKey, signingRoot32, indexedAtt); err != nil { return errors.Wrap(err, "could not save attestation history for validator public key") } diff --git a/validator/client/propose_protect.go b/validator/client/propose_protect.go index a041cadf84..60f89ff36b 100644 --- a/validator/client/propose_protect.go +++ b/validator/client/propose_protect.go @@ -6,20 +6,22 @@ import ( "github.com/pkg/errors" fieldparams "github.com/prysmaticlabs/prysm/v4/config/fieldparams" - "github.com/prysmaticlabs/prysm/v4/config/params" "github.com/prysmaticlabs/prysm/v4/consensus-types/interfaces" "github.com/sirupsen/logrus" ) var failedBlockSignLocalErr = "attempted to sign a double proposal, block rejected by local protection" +// slashableProposalCheck checks if a block proposal is slashable by comparing it with the +// block proposals history for the given public key in our DB. If it is not, we then update the history +// with new values and save it to the database. func (v *validator) slashableProposalCheck( ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signedBlock interfaces.ReadOnlySignedBeaconBlock, signingRoot [32]byte, ) error { fmtKey := fmt.Sprintf("%#x", pubKey[:]) blk := signedBlock.Block() - prevSigningRoot, proposalAtSlotExists, err := v.db.ProposalHistoryForSlot(ctx, pubKey, blk.Slot()) + prevSigningRoot, proposalAtSlotExists, prevSigningRootExists, err := v.db.ProposalHistoryForSlot(ctx, pubKey, blk.Slot()) if err != nil { if v.emitAccountMetrics { ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc() @@ -32,36 +34,56 @@ func (v *validator) slashableProposalCheck( return err } - // If a proposal exists in our history for the slot, we check the following: - // If the signing root is empty (zero hash), then we consider it slashable. If signing root is not empty, - // we check if it is different than the incoming block's signing root. If that is the case, - // we consider that proposal slashable. - signingRootIsDifferent := prevSigningRoot == params.BeaconConfig().ZeroHash || prevSigningRoot != signingRoot - if proposalAtSlotExists && signingRootIsDifferent { + // Based on EIP-3076 - Condition 2 + // ------------------------------- + if lowestProposalExists { + // If the block slot is (strictly) less than the lowest signed proposal slot in the DB, we consider it slashable. + if blk.Slot() < lowestSignedProposalSlot { + return fmt.Errorf( + "could not sign block with slot < lowest signed slot in db, block slot: %d < lowest signed slot: %d", + blk.Slot(), + lowestSignedProposalSlot, + ) + } + + // If the block slot is equal to the lowest signed proposal slot and + // - condition1: there is no signed proposal in the DB for this slot, or + // - condition2: there is a signed proposal in the DB for this slot, but with no associated signing root, or + // - condition3: there is a signed proposal in the DB for this slot, but the signing root differs, + // ==> we consider it slashable. + condition1 := !proposalAtSlotExists + condition2 := proposalAtSlotExists && !prevSigningRootExists + condition3 := proposalAtSlotExists && prevSigningRootExists && prevSigningRoot != signingRoot + if blk.Slot() == lowestSignedProposalSlot && (condition1 || condition2 || condition3) { + return fmt.Errorf( + "could not sign block with slot == lowest signed slot in db if it is not a repeat signing, block slot: %d == slowest signed slot: %d", + blk.Slot(), + lowestSignedProposalSlot, + ) + } + } + + // Based on EIP-3076 - Condition 1 + // ------------------------------- + // If there is a signed proposal in the DB for this slot and + // - there is no associated signing root, or + // - the signing root differs, + // ==> we consider it slashable. + if proposalAtSlotExists && (!prevSigningRootExists || prevSigningRoot != signingRoot) { if v.emitAccountMetrics { ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc() } return errors.New(failedBlockSignLocalErr) } - // Based on EIP3076, validator should refuse to sign any proposal with slot less - // than or equal to the minimum signed proposal present in the DB for that public key. - // In the case the slot of the incoming block is equal to the minimum signed proposal, we - // then also check the signing root is different. - if lowestProposalExists && signingRootIsDifferent && lowestSignedProposalSlot >= blk.Slot() { - return fmt.Errorf( - "could not sign block with slot <= lowest signed slot in db, lowest signed slot: %d >= block slot: %d", - lowestSignedProposalSlot, - blk.Slot(), - ) - } - + // Save the proposal for this slot. if err := v.db.SaveProposalHistoryForSlot(ctx, pubKey, blk.Slot(), signingRoot[:]); err != nil { if v.emitAccountMetrics { ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc() } return errors.Wrap(err, "failed to save updated proposal history") } + return nil } diff --git a/validator/client/propose_protect_test.go b/validator/client/propose_protect_test.go index b788cf2ac1..d613dde740 100644 --- a/validator/client/propose_protect_test.go +++ b/validator/client/propose_protect_test.go @@ -39,7 +39,7 @@ func Test_slashableProposalCheck_PreventsLowerThanMinProposal(t *testing.T) { wsb, err := blocks.NewSignedBeaconBlock(blk) require.NoError(t, err) err = validator.slashableProposalCheck(context.Background(), pubKeyBytes, wsb, [32]byte{4}) - require.ErrorContains(t, "could not sign block with slot <= lowest signed", err) + require.ErrorContains(t, "could not sign block with slot < lowest signed", err) // We expect the same block with a slot equal to the lowest // signed slot to pass validation if signing roots are equal. @@ -61,7 +61,7 @@ func Test_slashableProposalCheck_PreventsLowerThanMinProposal(t *testing.T) { wsb, err = blocks.NewSignedBeaconBlock(blk) require.NoError(t, err) err = validator.slashableProposalCheck(context.Background(), pubKeyBytes, wsb, [32]byte{4}) - require.ErrorContains(t, failedBlockSignLocalErr, err) + require.ErrorContains(t, "could not sign block with slot == lowest signed", err) // We expect the same block with a slot > than the lowest // signed slot to pass validation. diff --git a/validator/client/slashing_protection_interchange_test.go b/validator/client/slashing_protection_interchange_test.go index eb13e8baf1..ae21b686a1 100644 --- a/validator/client/slashing_protection_interchange_test.go +++ b/validator/client/slashing_protection_interchange_test.go @@ -43,17 +43,17 @@ type eip3076TestCase struct { } `json:"data"` } `json:"interchange"` Blocks []struct { - Pubkey string `json:"pubkey"` - Slot string `json:"slot"` - SigningRoot string `json:"signing_root"` - ShouldSucceed bool `json:"should_succeed"` + Pubkey string `json:"pubkey"` + Slot string `json:"slot"` + SigningRoot string `json:"signing_root"` + ShouldSucceedComplete bool `json:"should_succeed_complete"` } `json:"blocks"` Attestations []struct { - Pubkey string `json:"pubkey"` - SourceEpoch string `json:"source_epoch"` - TargetEpoch string `json:"target_epoch"` - SigningRoot string `json:"signing_root"` - ShouldSucceed bool `json:"should_succeed"` + Pubkey string `json:"pubkey"` + SourceEpoch string `json:"source_epoch"` + TargetEpoch string `json:"target_epoch"` + SigningRoot string `json:"signing_root"` + ShouldSucceedComplete bool `json:"should_succeed_complete"` } `json:"attestations"` } `json:"steps"` } @@ -82,11 +82,12 @@ func TestEIP3076SpecTests(t *testing.T) { if tt.Name == "" { t.Skip("Skipping eip3076TestCase with empty name") } - for _, step := range tt.Steps { - // Set up validator client, one new validator client per eip3076TestCase. - // This ensures we initialize a new (empty) slashing protection database. - validator, _, _, _ := setup(t) + // Set up validator client, one new validator client per eip3076TestCase. + // This ensures we initialize a new (empty) slashing protection database. + validator, _, _, _ := setup(t) + + for _, step := range tt.Steps { if tt.GenesisValidatorsRoot != "" { r, err := history.RootFromHex(tt.GenesisValidatorsRoot) require.NoError(t, validator.db.SaveGenesisValidatorsRoot(context.Background(), r[:])) @@ -127,7 +128,7 @@ func TestEIP3076SpecTests(t *testing.T) { wsb, err := blocks.NewSignedBeaconBlock(b) require.NoError(t, err) err = validator.slashableProposalCheck(context.Background(), pk, wsb, signingRoot) - if sb.ShouldSucceed { + if sb.ShouldSucceedComplete { require.NoError(t, err) } else { require.NotEqual(t, nil, err, "pre validation should have failed for block") @@ -159,14 +160,15 @@ func TestEIP3076SpecTests(t *testing.T) { } err = validator.slashableAttestationCheck(context.Background(), ia, pk, signingRoot) - if sa.ShouldSucceed { + if sa.ShouldSucceedComplete { require.NoError(t, err) } else { require.NotNil(t, err, "pre validation should have failed for attestation") } } - require.NoError(t, err, validator.db.Close()) } + + require.NoError(t, validator.db.Close(), "failed to close slashing protection database") }) } } diff --git a/validator/client/validator.go b/validator/client/validator.go index 6a41573ab6..5de69273d7 100644 --- a/validator/client/validator.go +++ b/validator/client/validator.go @@ -503,7 +503,7 @@ func (v *validator) CheckDoppelGanger(ctx context.Context) error { ðpb.DoppelGangerRequest_ValidatorRequest{ PublicKey: r.PubKey[:], Epoch: r.Target, - SignedRoot: r.SigningRoot[:], + SignedRoot: r.SigningRoot, }) } resp, err := v.validatorClient.CheckDoppelGanger(ctx, req) diff --git a/validator/db/iface/interface.go b/validator/db/iface/interface.go index 2615e12a3a..7eca91e316 100644 --- a/validator/db/iface/interface.go +++ b/validator/db/iface/interface.go @@ -34,7 +34,7 @@ type ValidatorDB interface { HighestSignedProposal(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte) (primitives.Slot, bool, error) LowestSignedProposal(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte) (primitives.Slot, bool, error) ProposalHistoryForPubKey(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte) ([]*kv.Proposal, error) - ProposalHistoryForSlot(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte, slot primitives.Slot) ([32]byte, bool, error) + ProposalHistoryForSlot(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte, slot primitives.Slot) ([32]byte, bool, bool, error) SaveProposalHistoryForSlot(ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, slot primitives.Slot, signingRoot []byte) error ProposedPublicKeys(ctx context.Context) ([][fieldparams.BLSPubkeyLength]byte, error) @@ -43,18 +43,18 @@ type ValidatorDB interface { // slashing protection imports. EIPImportBlacklistedPublicKeys(ctx context.Context) ([][fieldparams.BLSPubkeyLength]byte, error) SaveEIPImportBlacklistedPublicKeys(ctx context.Context, publicKeys [][fieldparams.BLSPubkeyLength]byte) error - SigningRootAtTargetEpoch(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte, target primitives.Epoch) ([32]byte, error) + SigningRootAtTargetEpoch(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte, target primitives.Epoch) ([]byte, error) LowestSignedTargetEpoch(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte) (primitives.Epoch, bool, error) LowestSignedSourceEpoch(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte) (primitives.Epoch, bool, error) AttestedPublicKeys(ctx context.Context) ([][fieldparams.BLSPubkeyLength]byte, error) CheckSlashableAttestation( - ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoot [32]byte, att *ethpb.IndexedAttestation, + ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoot []byte, att *ethpb.IndexedAttestation, ) (kv.SlashingKind, error) SaveAttestationForPubKey( ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoot [32]byte, att *ethpb.IndexedAttestation, ) error SaveAttestationsForPubKey( - ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoots [][32]byte, atts []*ethpb.IndexedAttestation, + ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoots [][]byte, atts []*ethpb.IndexedAttestation, ) error AttestationHistoryForPubKey( ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, diff --git a/validator/db/kv/attester_protection.go b/validator/db/kv/attester_protection.go index 3069609e98..ec5bc2ced0 100644 --- a/validator/db/kv/attester_protection.go +++ b/validator/db/kv/attester_protection.go @@ -33,7 +33,7 @@ type AttestationRecord struct { PubKey [fieldparams.BLSPubkeyLength]byte Source primitives.Epoch Target primitives.Epoch - SigningRoot [32]byte + SigningRoot []byte } // NewQueuedAttestationRecords constructor allocates the underlying slice and @@ -127,8 +127,9 @@ func (s *Store) AttestationHistoryForPubKey(ctx context.Context, pubKey [fieldpa Target: targetEpoch, } signingRoot := signingRootsBucket.Get(bytesutil.EpochToBytesBigEndian(targetEpoch)) - if signingRoot != nil { - copy(record.SigningRoot[:], signingRoot) + if len(signingRoot) != 0 { + record.SigningRoot = make([]byte, fieldparams.RootLength) + copy(record.SigningRoot, signingRoot) } records = append(records, record) } @@ -141,7 +142,7 @@ func (s *Store) AttestationHistoryForPubKey(ctx context.Context, pubKey [fieldpa // CheckSlashableAttestation verifies an incoming attestation is // not a double vote for a validator public key nor a surround vote. func (s *Store) CheckSlashableAttestation( - ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoot [32]byte, att *ethpb.IndexedAttestation, + ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoot []byte, att *ethpb.IndexedAttestation, ) (SlashingKind, error) { ctx, span := trace.StartSpan(ctx, "Validator.CheckSlashableAttestation") defer span.End() @@ -161,13 +162,12 @@ func (s *Store) CheckSlashableAttestation( if signingRootsBucket != nil { targetEpochBytes := bytesutil.EpochToBytesBigEndian(att.Data.Target.Epoch) existingSigningRoot := signingRootsBucket.Get(targetEpochBytes) - if existingSigningRoot != nil { - var existing [32]byte - copy(existing[:], existingSigningRoot) - if slashings.SigningRootsDiffer(existing, signingRoot) { - slashKind = DoubleVote - return fmt.Errorf(doubleVoteMessage, att.Data.Target.Epoch, existingSigningRoot) - } + + // If a signing root exists in the database, and if this database signing root is empty => We consider the new attestation as a double vote. + // If a signing root exists in the database, and if this database signing differs from the signing root of the new attestation => We consider the new attestation as a double vote. + if existingSigningRoot != nil && (len(existingSigningRoot) == 0 || slashings.SigningRootsDiffer(existingSigningRoot, signingRoot)) { + slashKind = DoubleVote + return fmt.Errorf(doubleVoteMessage, att.Data.Target.Epoch, existingSigningRoot) } } @@ -281,7 +281,7 @@ func (_ *Store) checkSurroundingVote( // SaveAttestationsForPubKey stores a batch of attestations all at once. func (s *Store) SaveAttestationsForPubKey( - ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoots [][32]byte, atts []*ethpb.IndexedAttestation, + ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, signingRoots [][]byte, atts []*ethpb.IndexedAttestation, ) error { ctx, span := trace.StartSpan(ctx, "Validator.SaveAttestationsForPubKey") defer span.End() @@ -317,7 +317,7 @@ func (s *Store) SaveAttestationForPubKey( PubKey: pubKey, Source: att.Data.Source.Epoch, Target: att.Data.Target.Epoch, - SigningRoot: signingRoot, + SigningRoot: signingRoot[:], }, } @@ -448,7 +448,7 @@ func (s *Store) saveAttestationRecords(ctx context.Context, atts []*AttestationR if err != nil { return errors.Wrap(err, "could not create signing roots bucket") } - if err := signingRootsBucket.Put(targetEpochBytes, att.SigningRoot[:]); err != nil { + if err := signingRootsBucket.Put(targetEpochBytes, att.SigningRoot); err != nil { return errors.Wrapf(err, "could not save signing root for epoch %d", att.Target) } sourceEpochsBucket, err := pkBucket.CreateBucketIfNotExists(attestationSourceEpochsBucket) @@ -537,24 +537,48 @@ func (s *Store) AttestedPublicKeys(ctx context.Context) ([][fieldparams.BLSPubke // SigningRootAtTargetEpoch checks for an existing signing root at a specified // target epoch for a given validator public key. -func (s *Store) SigningRootAtTargetEpoch(ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, target primitives.Epoch) ([32]byte, error) { +func (s *Store) SigningRootAtTargetEpoch(ctx context.Context, pubKey [fieldparams.BLSPubkeyLength]byte, target primitives.Epoch) ([]byte, error) { ctx, span := trace.StartSpan(ctx, "Validator.SigningRootAtTargetEpoch") defer span.End() - var signingRoot [32]byte + + var ( + signingRoot32 [32]byte + signingRootExists bool + ) + + signingRoot := make([]byte, 0, 32) + err := s.view(func(tx *bolt.Tx) error { bucket := tx.Bucket(pubKeysBucket) + + // If no bucket exists for this public key, we return nil. pkBucket := bucket.Bucket(pubKey[:]) if pkBucket == nil { return nil } + + // If no attestation bucket exists for this public key, we return nil. signingRootsBucket := pkBucket.Bucket(attestationSigningRootsBucket) if signingRootsBucket == nil { return nil } + + // If no signing root exists for this target epoch, we return nil. sr := signingRootsBucket.Get(bytesutil.EpochToBytesBigEndian(target)) - copy(signingRoot[:], sr) + if len(sr) == 0 { + return nil + } + + signingRootExists = true + copy(signingRoot32[:], sr) + return nil }) + + if signingRootExists { + signingRoot = signingRoot32[:] + } + return signingRoot, err } diff --git a/validator/db/kv/attester_protection_test.go b/validator/db/kv/attester_protection_test.go index 3320f892b0..71f444f945 100644 --- a/validator/db/kv/attester_protection_test.go +++ b/validator/db/kv/attester_protection_test.go @@ -100,7 +100,7 @@ func TestStore_CheckSlashableAttestation_DoubleVote(t *testing.T) { slashingKind, err := validatorDB.CheckSlashableAttestation( ctx, pubKeys[0], - tt.incomingSigningRoot, + tt.incomingSigningRoot[:], tt.incomingAttestation, ) if tt.want { @@ -133,7 +133,7 @@ func TestStore_CheckSlashableAttestation_SurroundVote_MultipleTargetsPerSource(t // our first attestation. Given there can be multiple attested target epochs per // source epoch, we expect our logic to be able to catch this slashable offense. evilAtt := createAttestation(firstAtt.Data.Source.Epoch-1, firstAtt.Data.Target.Epoch+1) - slashable, err := validatorDB.CheckSlashableAttestation(ctx, pubKeys[0], [32]byte{2}, evilAtt) + slashable, err := validatorDB.CheckSlashableAttestation(ctx, pubKeys[0], []byte{2}, evilAtt) require.NotNil(t, err) assert.Equal(t, SurroundingVote, slashable) } @@ -171,31 +171,31 @@ func TestStore_CheckSlashableAttestation_SurroundVote_54kEpochs(t *testing.T) { tests := []struct { name string - signingRoot [32]byte + signingRoot []byte attestation *ethpb.IndexedAttestation want SlashingKind }{ { name: "surround vote at half of the weak subjectivity period", - signingRoot: [32]byte{}, + signingRoot: []byte{}, attestation: createAttestation(numEpochs/2, numEpochs), want: SurroundingVote, }, { name: "spanning genesis to weak subjectivity period surround vote", - signingRoot: [32]byte{}, + signingRoot: []byte{}, attestation: createAttestation(0, numEpochs), want: SurroundingVote, }, { name: "simple surround vote at end of weak subjectivity period", - signingRoot: [32]byte{}, + signingRoot: []byte{}, attestation: createAttestation(numEpochs-3, numEpochs), want: SurroundingVote, }, { name: "non-slashable vote", - signingRoot: [32]byte{}, + signingRoot: []byte{}, attestation: createAttestation(numEpochs, numEpochs+1), want: NotSlashable, }, @@ -335,11 +335,11 @@ func TestStore_SaveAttestationsForPubKey(t *testing.T) { pubKeys := make([][fieldparams.BLSPubkeyLength]byte, numValidators) validatorDB := setupDB(t, pubKeys) atts := make([]*ethpb.IndexedAttestation, 0) - signingRoots := make([][32]byte, 0) + signingRoots := make([][]byte, 0) for i := primitives.Epoch(1); i < 10; i++ { atts = append(atts, createAttestation(i-1, i)) - var sr [32]byte - copy(sr[:], fmt.Sprintf("%d", i)) + var sr []byte + copy(sr, fmt.Sprintf("%d", i)) signingRoots = append(signingRoots, sr) } err := validatorDB.SaveAttestationsForPubKey( @@ -361,7 +361,7 @@ func TestStore_SaveAttestationsForPubKey(t *testing.T) { slashingKind, err := validatorDB.CheckSlashableAttestation( ctx, pubKeys[0], - [32]byte{}, + []byte{}, att, ) require.NotNil(t, err) @@ -544,7 +544,7 @@ func benchCheckSurroundVote( b.ResetTimer() for i := 0; i < b.N; i++ { for _, pubKey := range pubKeys { - slashingKind, err := validatorDB.CheckSlashableAttestation(ctx, pubKey, [32]byte{}, surroundingVote) + slashingKind, err := validatorDB.CheckSlashableAttestation(ctx, pubKey, []byte{}, surroundingVote) if shouldSurround { require.NotNil(b, err) assert.Equal(b, SurroundingVote, slashingKind) diff --git a/validator/db/kv/backup_test.go b/validator/db/kv/backup_test.go index 07e83c4ecd..9fe1c12276 100644 --- a/validator/db/kv/backup_test.go +++ b/validator/db/kv/backup_test.go @@ -88,13 +88,15 @@ func TestStore_NestedBackup(t *testing.T) { require.NoError(t, err) require.DeepEqual(t, root[:], genesisRoot) + signingRoot32 := [32]byte{'C'} + hist, err := backedDB.AttestationHistoryForPubKey(context.Background(), keys[0]) require.NoError(t, err) require.DeepEqual(t, &AttestationRecord{ PubKey: keys[0], Source: 10, Target: 0, - SigningRoot: [32]byte{'C'}, + SigningRoot: signingRoot32[:], }, hist[0]) hist, err = backedDB.AttestationHistoryForPubKey(context.Background(), keys[1]) @@ -103,7 +105,7 @@ func TestStore_NestedBackup(t *testing.T) { PubKey: keys[1], Source: 10, Target: 0, - SigningRoot: [32]byte{'C'}, + SigningRoot: signingRoot32[:], }, hist[0]) ep, exists, err := backedDB.LowestSignedSourceEpoch(context.Background(), keys[0]) diff --git a/validator/db/kv/genesis.go b/validator/db/kv/genesis.go index 62548db3c1..4ba6616b19 100644 --- a/validator/db/kv/genesis.go +++ b/validator/db/kv/genesis.go @@ -1,6 +1,7 @@ package kv import ( + "bytes" "context" "fmt" @@ -12,7 +13,7 @@ func (s *Store) SaveGenesisValidatorsRoot(_ context.Context, genValRoot []byte) err := s.db.Update(func(tx *bolt.Tx) error { bkt := tx.Bucket(genesisInfoBucket) enc := bkt.Get(genesisValidatorsRootKey) - if len(enc) != 0 { + if len(enc) != 0 && !bytes.Equal(enc, genValRoot) { return fmt.Errorf("cannot overwrite existing genesis validators root: %#x", enc) } return bkt.Put(genesisValidatorsRootKey, genValRoot) diff --git a/validator/db/kv/genesis_test.go b/validator/db/kv/genesis_test.go index 97f087a977..6fe5dd31e2 100644 --- a/validator/db/kv/genesis_test.go +++ b/validator/db/kv/genesis_test.go @@ -11,34 +11,62 @@ import ( func TestStore_GenesisValidatorsRoot_ReadAndWrite(t *testing.T) { ctx := context.Background() - db := setupDB(t, [][fieldparams.BLSPubkeyLength]byte{}) + tests := []struct { name string + init []byte want []byte write []byte wantErr bool }{ { - name: "empty then write", - want: nil, - write: params.BeaconConfig().ZeroHash[:], + name: "empty then write", + init: nil, + want: params.BeaconConfig().ZeroHash[:], + write: params.BeaconConfig().ZeroHash[:], + wantErr: false, }, { - name: "zero then overwrite rejected", + name: "zero then overwrite with the same value", + init: params.BeaconConfig().ZeroHash[:], + want: params.BeaconConfig().ZeroHash[:], + write: params.BeaconConfig().ZeroHash[:], + wantErr: false, + }, + { + name: "zero then overwrite with a different value", + init: params.BeaconConfig().ZeroHash[:], want: params.BeaconConfig().ZeroHash[:], write: []byte{5}, wantErr: true, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // Initialize the database with the initial value. + db := setupDB(t, [][fieldparams.BLSPubkeyLength]byte{}) + err := db.SaveGenesisValidatorsRoot(ctx, tt.init) + require.NoError(t, err) + + // Read the value from the database (just to ensure our setup is OK). got, err := db.GenesisValidatorsRoot(ctx) require.NoError(t, err) - require.DeepEqual(t, tt.want, got) + require.DeepEqual(t, tt.init, got) + + // Write the value to the database. err = db.SaveGenesisValidatorsRoot(ctx, tt.write) if (err != nil) != tt.wantErr { t.Errorf("GenesisValidatorsRoot() error = %v, wantErr %v", err, tt.wantErr) } + + // Read the value from the database. + got, err = db.GenesisValidatorsRoot(ctx) + require.NoError(t, err) + require.DeepEqual(t, tt.want, got) + + // Close the database. + require.NoError(t, db.Close()) }) } } diff --git a/validator/db/kv/proposer_protection.go b/validator/db/kv/proposer_protection.go index e1cc14b96f..0ba9e17762 100644 --- a/validator/db/kv/proposer_protection.go +++ b/validator/db/kv/proposer_protection.go @@ -44,30 +44,42 @@ func (s *Store) ProposedPublicKeys(ctx context.Context) ([][fieldparams.BLSPubke } // ProposalHistoryForSlot accepts a validator public key and returns the corresponding signing root as well -// as a boolean that tells us if we have a proposal history stored at the slot. It is possible we have proposed -// a slot but stored a nil signing root, so the boolean helps give full information. -func (s *Store) ProposalHistoryForSlot(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte, slot primitives.Slot) ([32]byte, bool, error) { +// as a boolean that tells us if we have a proposal history stored at the slot and a boolean that tells us if we have +// a signed root at the slot. +func (s *Store) ProposalHistoryForSlot(ctx context.Context, publicKey [fieldparams.BLSPubkeyLength]byte, slot primitives.Slot) ([32]byte, bool, bool, error) { ctx, span := trace.StartSpan(ctx, "Validator.ProposalHistoryForSlot") defer span.End() - var err error - var proposalExists bool - var signingRoot [32]byte + var ( + err error + proposalExists, signingRootExists bool + signingRoot [32]byte + ) + err = s.view(func(tx *bolt.Tx) error { bucket := tx.Bucket(historicProposalsBucket) valBucket := bucket.Bucket(publicKey[:]) if valBucket == nil { return nil } + signingRootBytes := valBucket.Get(bytesutil.SlotToBytesBigEndian(slot)) if signingRootBytes == nil { return nil } + + // If we are at this point, we are sure we have a proposal history for the slot. proposalExists = true + if len(signingRootBytes) == 0 { + return nil + } + + // If we are at this point, we are sure we have a signing root for the slot. + signingRootExists = true copy(signingRoot[:], signingRootBytes) return nil }) - return signingRoot, proposalExists, err + return signingRoot, proposalExists, signingRootExists, err } // ProposalHistoryForPubKey returns the entire proposal history for a given public key. diff --git a/validator/db/kv/proposer_protection_test.go b/validator/db/kv/proposer_protection_test.go index 26a318461c..e82120463a 100644 --- a/validator/db/kv/proposer_protection_test.go +++ b/validator/db/kv/proposer_protection_test.go @@ -12,25 +12,41 @@ import ( "github.com/prysmaticlabs/prysm/v4/testing/require" ) +func TestNewProposalHistoryForSlot_ReturnsNilIfNoHistory(t *testing.T) { + valPubkey := [fieldparams.BLSPubkeyLength]byte{1, 2, 3} + db := setupDB(t, [][fieldparams.BLSPubkeyLength]byte{}) + + _, proposalExists, signingRootExists, err := db.ProposalHistoryForSlot(context.Background(), valPubkey, 0) + require.NoError(t, err) + assert.Equal(t, false, proposalExists) + assert.Equal(t, false, signingRootExists) +} + func TestProposalHistoryForSlot_InitializesNewPubKeys(t *testing.T) { pubkeys := [][fieldparams.BLSPubkeyLength]byte{{30}, {25}, {20}} db := setupDB(t, pubkeys) for _, pub := range pubkeys { - signingRoot, _, err := db.ProposalHistoryForSlot(context.Background(), pub, 0) + _, proposalExists, signingRootExists, err := db.ProposalHistoryForSlot(context.Background(), pub, 0) require.NoError(t, err) - expected := bytesutil.PadTo([]byte{}, 32) - require.DeepEqual(t, expected, signingRoot[:], "Expected proposal history slot signing root to be empty") + assert.Equal(t, false, proposalExists) + assert.Equal(t, false, signingRootExists) } } -func TestNewProposalHistoryForSlot_ReturnsNilIfNoHistory(t *testing.T) { - valPubkey := [fieldparams.BLSPubkeyLength]byte{1, 2, 3} +func TestNewProposalHistoryForSlot_SigningRootNil(t *testing.T) { + pubkey := [fieldparams.BLSPubkeyLength]byte{1, 2, 3} + slot := primitives.Slot(2) + db := setupDB(t, [][fieldparams.BLSPubkeyLength]byte{}) - _, proposalExists, err := db.ProposalHistoryForSlot(context.Background(), valPubkey, 0) + err := db.SaveProposalHistoryForSlot(context.Background(), pubkey, slot, nil) + require.NoError(t, err, "Saving proposal history failed: %v") + + _, proposalExists, signingRootExists, err := db.ProposalHistoryForSlot(context.Background(), pubkey, slot) require.NoError(t, err) - assert.Equal(t, false, proposalExists) + assert.Equal(t, true, proposalExists) + assert.Equal(t, false, signingRootExists) } func TestSaveProposalHistoryForSlot_OK(t *testing.T) { @@ -41,8 +57,10 @@ func TestSaveProposalHistoryForSlot_OK(t *testing.T) { err := db.SaveProposalHistoryForSlot(context.Background(), pubkey, slot, []byte{1}) require.NoError(t, err, "Saving proposal history failed: %v") - signingRoot, _, err := db.ProposalHistoryForSlot(context.Background(), pubkey, slot) + signingRoot, proposalExists, signingRootExists, err := db.ProposalHistoryForSlot(context.Background(), pubkey, slot) require.NoError(t, err, "Failed to get proposal history") + assert.Equal(t, true, proposalExists) + assert.Equal(t, true, signingRootExists) require.NotNil(t, signingRoot) require.DeepEqual(t, bytesutil.PadTo([]byte{1}, 32), signingRoot[:], "Expected DB to keep object the same") diff --git a/validator/slashing-protection-history/BUILD.bazel b/validator/slashing-protection-history/BUILD.bazel index df0cbea75a..1d3b9b9dad 100644 --- a/validator/slashing-protection-history/BUILD.bazel +++ b/validator/slashing-protection-history/BUILD.bazel @@ -16,7 +16,6 @@ go_library( ], deps = [ "//config/fieldparams:go_default_library", - "//config/params:go_default_library", "//consensus-types/primitives:go_default_library", "//encoding/bytesutil:go_default_library", "//monitoring/progress:go_default_library", diff --git a/validator/slashing-protection-history/export.go b/validator/slashing-protection-history/export.go index d78fd19064..2e8d374bf8 100644 --- a/validator/slashing-protection-history/export.go +++ b/validator/slashing-protection-history/export.go @@ -1,7 +1,6 @@ package history import ( - "bytes" "context" "fmt" "sort" @@ -9,7 +8,6 @@ import ( "github.com/pkg/errors" fieldparams "github.com/prysmaticlabs/prysm/v4/config/fieldparams" - "github.com/prysmaticlabs/prysm/v4/config/params" "github.com/prysmaticlabs/prysm/v4/encoding/bytesutil" "github.com/prysmaticlabs/prysm/v4/monitoring/progress" "github.com/prysmaticlabs/prysm/v4/validator/db" @@ -158,8 +156,8 @@ func signedAttestationsByPubKey(ctx context.Context, validatorDB db.Database, pu } } var root string - if !bytes.Equal(att.SigningRoot[:], params.BeaconConfig().ZeroHash[:]) { - root, err = rootToHexString(att.SigningRoot[:]) + if len(att.SigningRoot) != 0 { + root, err = rootToHexString(att.SigningRoot) if err != nil { return nil, errors.Wrap(err, "could not convert signing root to hex string") } diff --git a/validator/slashing-protection-history/import.go b/validator/slashing-protection-history/import.go index f04e7413dd..0daae13a6d 100644 --- a/validator/slashing-protection-history/import.go +++ b/validator/slashing-protection-history/import.go @@ -27,10 +27,12 @@ func ImportStandardProtectionJSON(ctx context.Context, validatorDB db.Database, if err != nil { return errors.Wrap(err, "could not read slashing protection JSON file") } + interchangeJSON := &format.EIPSlashingProtectionFormat{} if err := json.Unmarshal(encodedJSON, interchangeJSON); err != nil { return errors.Wrap(err, "could not unmarshal slashing protection JSON file") } + if interchangeJSON.Data == nil { log.Warn("No slashing protection data to import") return nil @@ -47,6 +49,7 @@ func ImportStandardProtectionJSON(ctx context.Context, validatorDB db.Database, if err != nil { return errors.Wrap(err, "could not parse unique entries for blocks by public key") } + signedAttsByPubKey, err := parseAttestationsForUniquePublicKeys(interchangeJSON.Data) if err != nil { return errors.Wrap(err, "could not parse unique entries for attestations by public key") @@ -54,6 +57,7 @@ func ImportStandardProtectionJSON(ctx context.Context, validatorDB db.Database, attestingHistoryByPubKey := make(map[[fieldparams.BLSPubkeyLength]byte][]*kv.AttestationRecord) proposalHistoryByPubKey := make(map[[fieldparams.BLSPubkeyLength]byte]kv.ProposalHistoryForPubkey) + for pubKey, signedBlocks := range signedBlocksByPubKey { // Transform the processed signed blocks data from the JSON // file into the internal Prysm representation of proposal history. @@ -61,6 +65,7 @@ func ImportStandardProtectionJSON(ctx context.Context, validatorDB db.Database, if err != nil { return errors.Wrapf(err, "could not parse signed blocks in JSON file for key %#x", pubKey) } + proposalHistoryByPubKey[pubKey] = *proposalHistory } @@ -71,28 +76,23 @@ func ImportStandardProtectionJSON(ctx context.Context, validatorDB db.Database, if err != nil { return errors.Wrapf(err, "could not parse signed attestations in JSON file for key %#x", pubKey) } + attestingHistoryByPubKey[pubKey] = historicalAtt } // We validate and filter out public keys parsed from JSON to ensure we are // not importing those which are slashable with respect to other data within the same JSON. slashableProposerKeys := filterSlashablePubKeysFromBlocks(ctx, proposalHistoryByPubKey) - slashableAttesterKeys, err := filterSlashablePubKeysFromAttestations( - ctx, validatorDB, attestingHistoryByPubKey, - ) + + slashableAttesterKeys, err := filterSlashablePubKeysFromAttestations(ctx, validatorDB, attestingHistoryByPubKey) if err != nil { return errors.Wrap(err, "could not filter slashable attester public keys from JSON data") } - slashablePublicKeys := make([][fieldparams.BLSPubkeyLength]byte, 0, len(slashableAttesterKeys)+len(slashableProposerKeys)) - for _, pubKey := range slashableProposerKeys { - delete(proposalHistoryByPubKey, pubKey) - slashablePublicKeys = append(slashablePublicKeys, pubKey) - } - for _, pubKey := range slashableAttesterKeys { - delete(attestingHistoryByPubKey, pubKey) - slashablePublicKeys = append(slashablePublicKeys, pubKey) - } + slashablePublicKeysCount := len(slashableProposerKeys) + len(slashableAttesterKeys) + slashablePublicKeys := make([][fieldparams.BLSPubkeyLength]byte, 0, slashablePublicKeysCount) + slashablePublicKeys = append(slashablePublicKeys, slashableProposerKeys...) + slashablePublicKeys = append(slashablePublicKeys, slashableAttesterKeys...) if err := validatorDB.SaveEIPImportBlacklistedPublicKeys(ctx, slashablePublicKeys); err != nil { return errors.Wrap(err, "could not save slashable public keys to database") @@ -101,39 +101,63 @@ func ImportStandardProtectionJSON(ctx context.Context, validatorDB db.Database, // We save the histories to disk as atomic operations, ensuring that this only occurs // until after we successfully parse all data from the JSON file. If there is any error // in parsing the JSON proposal and attesting histories, we will not reach this point. + if err := saveProposals(ctx, proposalHistoryByPubKey, validatorDB); err != nil { + return errors.Wrap(err, "could not save proposals") + } + + if err := saveAttestations(ctx, attestingHistoryByPubKey, validatorDB); err != nil { + return errors.Wrap(err, "could not save attestations") + } + + return nil +} + +func saveProposals(ctx context.Context, proposalHistoryByPubKey map[[fieldparams.BLSPubkeyLength]byte]kv.ProposalHistoryForPubkey, validatorDB db.Database) error { for pubKey, proposalHistory := range proposalHistoryByPubKey { bar := initializeProgressBar( len(proposalHistory.Proposals), fmt.Sprintf("Importing proposals for validator public key %#x", bytesutil.Trunc(pubKey[:])), ) + for _, proposal := range proposalHistory.Proposals { if err := bar.Add(1); err != nil { log.WithError(err).Debug("Could not increase progress bar") } - if err = validatorDB.SaveProposalHistoryForSlot(ctx, pubKey, proposal.Slot, proposal.SigningRoot); err != nil { + + if err := validatorDB.SaveProposalHistoryForSlot(ctx, pubKey, proposal.Slot, proposal.SigningRoot); err != nil { return errors.Wrap(err, "could not save proposal history from imported JSON to database") } } } + + return nil +} + +func saveAttestations(ctx context.Context, attestingHistoryByPubKey map[[fieldparams.BLSPubkeyLength]byte][]*kv.AttestationRecord, validatorDB db.Database) error { bar := initializeProgressBar( len(attestingHistoryByPubKey), "Importing attesting history for validator public keys", ) + for pubKey, attestations := range attestingHistoryByPubKey { if err := bar.Add(1); err != nil { log.WithError(err).Debug("Could not increase progress bar") } + indexedAtts := make([]*ethpb.IndexedAttestation, len(attestations)) - signingRoots := make([][32]byte, len(attestations)) + signingRoots := make([][]byte, len(attestations)) + for i, att := range attestations { indexedAtt := createAttestation(att.Source, att.Target) indexedAtts[i] = indexedAtt signingRoots[i] = att.SigningRoot } + if err := validatorDB.SaveAttestationsForPubKey(ctx, pubKey, signingRoots, indexedAtts); err != nil { return errors.Wrap(err, "could not save attestations from imported JSON to database") } } + return nil } @@ -269,7 +293,7 @@ func filterSlashablePubKeysFromAttestations( // First we need to find attestations that are slashable with respect to other // attestations within the same JSON import. for pubKey, signedAtts := range signedAttsByPubKey { - signingRootsByTarget := make(map[primitives.Epoch][32]byte) + signingRootsByTarget := make(map[primitives.Epoch][]byte) targetEpochsBySource := make(map[primitives.Epoch][]primitives.Epoch) Loop: for _, att := range signedAtts { @@ -299,11 +323,14 @@ func filterSlashablePubKeysFromAttestations( for pubKey, signedAtts := range signedAttsByPubKey { for _, att := range signedAtts { indexedAtt := createAttestation(att.Source, att.Target) + + // If slashable == NotSlashable and err != nil, then CheckSlashableAttestation failed. + // If slashable != NotSlashable, then err contains the reason why the attestation is slashable. slashable, err := validatorDB.CheckSlashableAttestation(ctx, pubKey, att.SigningRoot, indexedAtt) - if err != nil { + if err != nil && slashable == kv.NotSlashable { return nil, err } - // Malformed data should not prevent us from completing this function. + if slashable != kv.NotSlashable { slashablePubKeys = append(slashablePubKeys, pubKey) break @@ -320,19 +347,25 @@ func transformSignedBlocks(_ context.Context, signedBlocks []*format.SignedBlock if err != nil { return nil, fmt.Errorf("%s is not a valid slot: %w", proposal.Slot, err) } - var signingRoot [32]byte + // Signing roots are optional in the standard JSON file. + // If the signing root is not provided, we use a default value which is a zero-length byte slice. + signingRoot := make([]byte, 0, fieldparams.RootLength) + if proposal.SigningRoot != "" { - signingRoot, err = RootFromHex(proposal.SigningRoot) + signingRoot32, err := RootFromHex(proposal.SigningRoot) if err != nil { return nil, fmt.Errorf("%s is not a valid root: %w", proposal.SigningRoot, err) } + signingRoot = signingRoot32[:] } + proposals[i] = kv.Proposal{ Slot: slot, - SigningRoot: signingRoot[:], + SigningRoot: signingRoot, } } + return &kv.ProposalHistoryForPubkey{ Proposals: proposals, }, nil @@ -349,13 +382,17 @@ func transformSignedAttestations(pubKey [fieldparams.BLSPubkeyLength]byte, atts if err != nil { return nil, fmt.Errorf("%s is not a valid epoch: %w", attestation.SourceEpoch, err) } - var signingRoot [32]byte + // Signing roots are optional in the standard JSON file. + // If the signing root is not provided, we use a default value which is a zero-length byte slice. + signingRoot := make([]byte, 0, fieldparams.RootLength) + if attestation.SigningRoot != "" { - signingRoot, err = RootFromHex(attestation.SigningRoot) + signingRoot32, err := RootFromHex(attestation.SigningRoot) if err != nil { return nil, fmt.Errorf("%s is not a valid root: %w", attestation.SigningRoot, err) } + signingRoot = signingRoot32[:] } historicalAtts = append(historicalAtts, &kv.AttestationRecord{ PubKey: pubKey, diff --git a/validator/slashing-protection-history/import_test.go b/validator/slashing-protection-history/import_test.go index 908343a3d5..84c47386c0 100644 --- a/validator/slashing-protection-history/import_test.go +++ b/validator/slashing-protection-history/import_test.go @@ -87,7 +87,7 @@ func TestStore_ImportInterchangeData_BadFormat_PreventsDBWrites(t *testing.T) { }, }, } - slashingKind, err := validatorDB.CheckSlashableAttestation(ctx, publicKeys[i], [32]byte{}, indexedAtt) + slashingKind, err := validatorDB.CheckSlashableAttestation(ctx, publicKeys[i], []byte{}, indexedAtt) // We expect we do not have an attesting history for each attestation require.NoError(t, err) require.Equal(t, kv.NotSlashable, slashingKind) @@ -139,7 +139,7 @@ func TestStore_ImportInterchangeData_OK(t *testing.T) { }, }, } - slashingKind, err := validatorDB.CheckSlashableAttestation(ctx, publicKeys[i], [32]byte{}, indexedAtt) + slashingKind, err := validatorDB.CheckSlashableAttestation(ctx, publicKeys[i], []byte{}, indexedAtt) // We expect we have an attesting history for the attestation and when // attempting to verify the same att is slashable with a different signing root, // we expect to receive a double vote slashing kind. @@ -1051,8 +1051,11 @@ func Test_filterSlashablePubKeysFromAttestations(t *testing.T) { attestingHistory, err := transformSignedAttestations(pubKey, signedAtts) require.NoError(t, err) for _, att := range attestingHistory { + var signingRoot [32]byte + copy(signingRoot[:], att.SigningRoot) + indexedAtt := createAttestation(att.Source, att.Target) - err := validatorDB.SaveAttestationForPubKey(ctx, pubKey, att.SigningRoot, indexedAtt) + err := validatorDB.SaveAttestationForPubKey(ctx, pubKey, signingRoot, indexedAtt) require.NoError(t, err) } } diff --git a/validator/slashing-protection-history/round_trip_test.go b/validator/slashing-protection-history/round_trip_test.go index 9778ae914f..6f54c51812 100644 --- a/validator/slashing-protection-history/round_trip_test.go +++ b/validator/slashing-protection-history/round_trip_test.go @@ -207,10 +207,14 @@ func TestImportInterchangeData_OK(t *testing.T) { wantedAttsByRoot := make(map[[32]byte]*kv.AttestationRecord) for _, att := range attestingHistory[i] { - wantedAttsByRoot[att.SigningRoot] = att + var signingRoot [32]byte + copy(signingRoot[:], att.SigningRoot) + wantedAttsByRoot[signingRoot] = att } for _, att := range receivedAttestingHistory { - wantedAtt, ok := wantedAttsByRoot[att.SigningRoot] + var signingRoot [32]byte + copy(signingRoot[:], att.SigningRoot) + wantedAtt, ok := wantedAttsByRoot[signingRoot] require.Equal(t, true, ok) require.DeepEqual(t, wantedAtt, att) } diff --git a/validator/testing/protection_history.go b/validator/testing/protection_history.go index 6c55d72702..9a187973ba 100644 --- a/validator/testing/protection_history.go +++ b/validator/testing/protection_history.go @@ -74,7 +74,7 @@ func MockAttestingAndProposalHistories(pubkeys [][fieldparams.BLSPubkeyLength]by historicalAtts = append(historicalAtts, &kv.AttestationRecord{ Source: i - 1, Target: i, - SigningRoot: signingRoot, + SigningRoot: signingRoot[:], PubKey: pubkeys[v], }) }