prysm-pulse/beacon-chain/slasher/helpers.go

162 lines
6.0 KiB
Go
Raw Normal View History

package slasher
import (
"bytes"
"strconv"
slashertypes "github.com/prysmaticlabs/prysm/v5/beacon-chain/slasher/types"
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/config/params"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v5/container/slice"
ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/sirupsen/logrus"
)
// Group a list of attestations into batches by validator chunk index.
// This way, we can detect on the batch of attestations for each validator chunk index
// concurrently, and also allowing us to effectively use a single 2D chunk
// for slashing detection through this logical grouping.
func (s *Service) groupByValidatorChunkIndex(
attestations []*slashertypes.IndexedAttestationWrapper,
) map[uint64][]*slashertypes.IndexedAttestationWrapper {
groupedAttestations := make(map[uint64][]*slashertypes.IndexedAttestationWrapper)
Slasher: Refactor and add tests (#13589) * `helpers.go`: Improve naming consistency. * `detect_attestations.go`: Improve readability. * `receive.go`: Add `attsQueueSize` in log message. * `checkSlashableAttestations`: Improve logging. `avgBatchProcessingTime` is not displayed any more if not batch is processed. * `loadChunks`: Use explicit `chunkKind` and `chunkIndices`. * `getChunk`: Use specific `chunkIndex` and `chunkKind`. * `validatorIndicesInChunk` -> `validatorIndexesInChunk`. * `epochUpdateForValidator`: Use explicit arguments. * `getChunk`: Change order of arguments. * `latestEpochWrittenForValidator`: Use `ok` parameter. So the default value is not any more considered as the absence of value. * `applyAttestationForValidator`: Use explicit arguments. * `updateSpans`: Use explicit arguments. * `saveUpdatedChunks`: Use explicit arguments. * `checkSurrounds`: Use explicit arguments. We see here that, previously, in `checkSlashableAttestations`, `checkSurrounds` was called with the default value of `slashertypes`: `MinSpan`. Now, we set it expliciterly at `MinSpan`, which may explicit a bug. * `epochUpdateForValidator`: Set modified by the function argument first. * `applyAttestationForValidator`: Set mutated argument `chunksByChunkIdx`first. * `applyAttestationForValidator`: Rename variables. * `Test_processQueuedAttestations`: Fix test. Two tests were actually exactly the same. * `updateSpans`: Keep happy path in the outer scope. Even if in this case the "happy" path means slashing. * `checkSurrounds`: Rename variable. * `getChunk`: Avoid side effects. It adds a few lines for callers, but it does not modify any more arguments and it does what it says: getting a chunk. * `CheckSlashable`: Flatten. * `detect_attestations_test.go`: Simplify. * `CheckSlashable`: Add error log in case of missing attestation. * `processQueuedAttestations`: Extract a sub function. So testing will be easier. * `processAttesterSlashings` and `processProposerSlashings`: Improve. * `processAttesterSlashings`: Return processed slashings. * `createAttestationWrapper`: Rename variables. * `signingRoot` ==> `headerRoot` or `dataRoot`. Before this commit, there is two typse of `signing root`s floating around. - The first one is a real signing root, aka a hash tree root computed from an object root and a domain. This real signing root is the object ready to be signed. - The second one is a "false" signing root, which is actually just the hash tree root of an object. This object is either the `Data` field of an attestation, or the `Header` field of a block. Having 2 differents objects with the same name `signing root` is quite confusing. This commit renames wrongly named `signing root` objects. * `createAttestationWrapper` => `createAttestationWrapperEmptySig`. So it's clear for the user that the created attestation wrapper has an empty signature. * Implement `createAttestationWrapper`. * `processAttestations`: Return processed attester slashings. * Test `processAttestations` instead of `processQueuedAttestations`. By testing `processAttestations` instead of `processQueuedAttestations`, we get rid of a lot of tests fixtures, including the 200 ms sleep. The whole testing duration is shorter. * `Test_processAttestations`: Allow multiple steps. * `Test_processAttestations`: Add double steps tests. Some new failing tests are commented with a corresponding github issue. * `NextChunkStartEpoch`: Fix function comment. Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com> * `chunks.go`: Avoid templating log messages. * `checkSlashableAttestations`: Simplify duration computation. --------- Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
2024-02-09 21:02:18 +00:00
for _, attestation := range attestations {
validatorChunkIndexes := make(map[uint64]bool)
for _, validatorIndex := range attestation.IndexedAttestation.AttestingIndices {
validatorChunkIndex := s.params.validatorChunkIndex(primitives.ValidatorIndex(validatorIndex))
validatorChunkIndexes[validatorChunkIndex] = true
}
Slasher: Refactor and add tests (#13589) * `helpers.go`: Improve naming consistency. * `detect_attestations.go`: Improve readability. * `receive.go`: Add `attsQueueSize` in log message. * `checkSlashableAttestations`: Improve logging. `avgBatchProcessingTime` is not displayed any more if not batch is processed. * `loadChunks`: Use explicit `chunkKind` and `chunkIndices`. * `getChunk`: Use specific `chunkIndex` and `chunkKind`. * `validatorIndicesInChunk` -> `validatorIndexesInChunk`. * `epochUpdateForValidator`: Use explicit arguments. * `getChunk`: Change order of arguments. * `latestEpochWrittenForValidator`: Use `ok` parameter. So the default value is not any more considered as the absence of value. * `applyAttestationForValidator`: Use explicit arguments. * `updateSpans`: Use explicit arguments. * `saveUpdatedChunks`: Use explicit arguments. * `checkSurrounds`: Use explicit arguments. We see here that, previously, in `checkSlashableAttestations`, `checkSurrounds` was called with the default value of `slashertypes`: `MinSpan`. Now, we set it expliciterly at `MinSpan`, which may explicit a bug. * `epochUpdateForValidator`: Set modified by the function argument first. * `applyAttestationForValidator`: Set mutated argument `chunksByChunkIdx`first. * `applyAttestationForValidator`: Rename variables. * `Test_processQueuedAttestations`: Fix test. Two tests were actually exactly the same. * `updateSpans`: Keep happy path in the outer scope. Even if in this case the "happy" path means slashing. * `checkSurrounds`: Rename variable. * `getChunk`: Avoid side effects. It adds a few lines for callers, but it does not modify any more arguments and it does what it says: getting a chunk. * `CheckSlashable`: Flatten. * `detect_attestations_test.go`: Simplify. * `CheckSlashable`: Add error log in case of missing attestation. * `processQueuedAttestations`: Extract a sub function. So testing will be easier. * `processAttesterSlashings` and `processProposerSlashings`: Improve. * `processAttesterSlashings`: Return processed slashings. * `createAttestationWrapper`: Rename variables. * `signingRoot` ==> `headerRoot` or `dataRoot`. Before this commit, there is two typse of `signing root`s floating around. - The first one is a real signing root, aka a hash tree root computed from an object root and a domain. This real signing root is the object ready to be signed. - The second one is a "false" signing root, which is actually just the hash tree root of an object. This object is either the `Data` field of an attestation, or the `Header` field of a block. Having 2 differents objects with the same name `signing root` is quite confusing. This commit renames wrongly named `signing root` objects. * `createAttestationWrapper` => `createAttestationWrapperEmptySig`. So it's clear for the user that the created attestation wrapper has an empty signature. * Implement `createAttestationWrapper`. * `processAttestations`: Return processed attester slashings. * Test `processAttestations` instead of `processQueuedAttestations`. By testing `processAttestations` instead of `processQueuedAttestations`, we get rid of a lot of tests fixtures, including the 200 ms sleep. The whole testing duration is shorter. * `Test_processAttestations`: Allow multiple steps. * `Test_processAttestations`: Add double steps tests. Some new failing tests are commented with a corresponding github issue. * `NextChunkStartEpoch`: Fix function comment. Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com> * `chunks.go`: Avoid templating log messages. * `checkSlashableAttestations`: Simplify duration computation. --------- Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
2024-02-09 21:02:18 +00:00
for validatorChunkIndex := range validatorChunkIndexes {
groupedAttestations[validatorChunkIndex] = append(
groupedAttestations[validatorChunkIndex],
Slasher: Refactor and add tests (#13589) * `helpers.go`: Improve naming consistency. * `detect_attestations.go`: Improve readability. * `receive.go`: Add `attsQueueSize` in log message. * `checkSlashableAttestations`: Improve logging. `avgBatchProcessingTime` is not displayed any more if not batch is processed. * `loadChunks`: Use explicit `chunkKind` and `chunkIndices`. * `getChunk`: Use specific `chunkIndex` and `chunkKind`. * `validatorIndicesInChunk` -> `validatorIndexesInChunk`. * `epochUpdateForValidator`: Use explicit arguments. * `getChunk`: Change order of arguments. * `latestEpochWrittenForValidator`: Use `ok` parameter. So the default value is not any more considered as the absence of value. * `applyAttestationForValidator`: Use explicit arguments. * `updateSpans`: Use explicit arguments. * `saveUpdatedChunks`: Use explicit arguments. * `checkSurrounds`: Use explicit arguments. We see here that, previously, in `checkSlashableAttestations`, `checkSurrounds` was called with the default value of `slashertypes`: `MinSpan`. Now, we set it expliciterly at `MinSpan`, which may explicit a bug. * `epochUpdateForValidator`: Set modified by the function argument first. * `applyAttestationForValidator`: Set mutated argument `chunksByChunkIdx`first. * `applyAttestationForValidator`: Rename variables. * `Test_processQueuedAttestations`: Fix test. Two tests were actually exactly the same. * `updateSpans`: Keep happy path in the outer scope. Even if in this case the "happy" path means slashing. * `checkSurrounds`: Rename variable. * `getChunk`: Avoid side effects. It adds a few lines for callers, but it does not modify any more arguments and it does what it says: getting a chunk. * `CheckSlashable`: Flatten. * `detect_attestations_test.go`: Simplify. * `CheckSlashable`: Add error log in case of missing attestation. * `processQueuedAttestations`: Extract a sub function. So testing will be easier. * `processAttesterSlashings` and `processProposerSlashings`: Improve. * `processAttesterSlashings`: Return processed slashings. * `createAttestationWrapper`: Rename variables. * `signingRoot` ==> `headerRoot` or `dataRoot`. Before this commit, there is two typse of `signing root`s floating around. - The first one is a real signing root, aka a hash tree root computed from an object root and a domain. This real signing root is the object ready to be signed. - The second one is a "false" signing root, which is actually just the hash tree root of an object. This object is either the `Data` field of an attestation, or the `Header` field of a block. Having 2 differents objects with the same name `signing root` is quite confusing. This commit renames wrongly named `signing root` objects. * `createAttestationWrapper` => `createAttestationWrapperEmptySig`. So it's clear for the user that the created attestation wrapper has an empty signature. * Implement `createAttestationWrapper`. * `processAttestations`: Return processed attester slashings. * Test `processAttestations` instead of `processQueuedAttestations`. By testing `processAttestations` instead of `processQueuedAttestations`, we get rid of a lot of tests fixtures, including the 200 ms sleep. The whole testing duration is shorter. * `Test_processAttestations`: Allow multiple steps. * `Test_processAttestations`: Add double steps tests. Some new failing tests are commented with a corresponding github issue. * `NextChunkStartEpoch`: Fix function comment. Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com> * `chunks.go`: Avoid templating log messages. * `checkSlashableAttestations`: Simplify duration computation. --------- Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
2024-02-09 21:02:18 +00:00
attestation,
)
}
}
Slasher: Refactor and add tests (#13589) * `helpers.go`: Improve naming consistency. * `detect_attestations.go`: Improve readability. * `receive.go`: Add `attsQueueSize` in log message. * `checkSlashableAttestations`: Improve logging. `avgBatchProcessingTime` is not displayed any more if not batch is processed. * `loadChunks`: Use explicit `chunkKind` and `chunkIndices`. * `getChunk`: Use specific `chunkIndex` and `chunkKind`. * `validatorIndicesInChunk` -> `validatorIndexesInChunk`. * `epochUpdateForValidator`: Use explicit arguments. * `getChunk`: Change order of arguments. * `latestEpochWrittenForValidator`: Use `ok` parameter. So the default value is not any more considered as the absence of value. * `applyAttestationForValidator`: Use explicit arguments. * `updateSpans`: Use explicit arguments. * `saveUpdatedChunks`: Use explicit arguments. * `checkSurrounds`: Use explicit arguments. We see here that, previously, in `checkSlashableAttestations`, `checkSurrounds` was called with the default value of `slashertypes`: `MinSpan`. Now, we set it expliciterly at `MinSpan`, which may explicit a bug. * `epochUpdateForValidator`: Set modified by the function argument first. * `applyAttestationForValidator`: Set mutated argument `chunksByChunkIdx`first. * `applyAttestationForValidator`: Rename variables. * `Test_processQueuedAttestations`: Fix test. Two tests were actually exactly the same. * `updateSpans`: Keep happy path in the outer scope. Even if in this case the "happy" path means slashing. * `checkSurrounds`: Rename variable. * `getChunk`: Avoid side effects. It adds a few lines for callers, but it does not modify any more arguments and it does what it says: getting a chunk. * `CheckSlashable`: Flatten. * `detect_attestations_test.go`: Simplify. * `CheckSlashable`: Add error log in case of missing attestation. * `processQueuedAttestations`: Extract a sub function. So testing will be easier. * `processAttesterSlashings` and `processProposerSlashings`: Improve. * `processAttesterSlashings`: Return processed slashings. * `createAttestationWrapper`: Rename variables. * `signingRoot` ==> `headerRoot` or `dataRoot`. Before this commit, there is two typse of `signing root`s floating around. - The first one is a real signing root, aka a hash tree root computed from an object root and a domain. This real signing root is the object ready to be signed. - The second one is a "false" signing root, which is actually just the hash tree root of an object. This object is either the `Data` field of an attestation, or the `Header` field of a block. Having 2 differents objects with the same name `signing root` is quite confusing. This commit renames wrongly named `signing root` objects. * `createAttestationWrapper` => `createAttestationWrapperEmptySig`. So it's clear for the user that the created attestation wrapper has an empty signature. * Implement `createAttestationWrapper`. * `processAttestations`: Return processed attester slashings. * Test `processAttestations` instead of `processQueuedAttestations`. By testing `processAttestations` instead of `processQueuedAttestations`, we get rid of a lot of tests fixtures, including the 200 ms sleep. The whole testing duration is shorter. * `Test_processAttestations`: Allow multiple steps. * `Test_processAttestations`: Add double steps tests. Some new failing tests are commented with a corresponding github issue. * `NextChunkStartEpoch`: Fix function comment. Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com> * `chunks.go`: Avoid templating log messages. * `checkSlashableAttestations`: Simplify duration computation. --------- Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
2024-02-09 21:02:18 +00:00
return groupedAttestations
}
// Group attestations by the chunk index their source epoch corresponds to.
func (s *Service) groupByChunkIndex(
attestations []*slashertypes.IndexedAttestationWrapper,
) map[uint64][]*slashertypes.IndexedAttestationWrapper {
attestationsByChunkIndex := make(map[uint64][]*slashertypes.IndexedAttestationWrapper)
Slasher: Refactor and add tests (#13589) * `helpers.go`: Improve naming consistency. * `detect_attestations.go`: Improve readability. * `receive.go`: Add `attsQueueSize` in log message. * `checkSlashableAttestations`: Improve logging. `avgBatchProcessingTime` is not displayed any more if not batch is processed. * `loadChunks`: Use explicit `chunkKind` and `chunkIndices`. * `getChunk`: Use specific `chunkIndex` and `chunkKind`. * `validatorIndicesInChunk` -> `validatorIndexesInChunk`. * `epochUpdateForValidator`: Use explicit arguments. * `getChunk`: Change order of arguments. * `latestEpochWrittenForValidator`: Use `ok` parameter. So the default value is not any more considered as the absence of value. * `applyAttestationForValidator`: Use explicit arguments. * `updateSpans`: Use explicit arguments. * `saveUpdatedChunks`: Use explicit arguments. * `checkSurrounds`: Use explicit arguments. We see here that, previously, in `checkSlashableAttestations`, `checkSurrounds` was called with the default value of `slashertypes`: `MinSpan`. Now, we set it expliciterly at `MinSpan`, which may explicit a bug. * `epochUpdateForValidator`: Set modified by the function argument first. * `applyAttestationForValidator`: Set mutated argument `chunksByChunkIdx`first. * `applyAttestationForValidator`: Rename variables. * `Test_processQueuedAttestations`: Fix test. Two tests were actually exactly the same. * `updateSpans`: Keep happy path in the outer scope. Even if in this case the "happy" path means slashing. * `checkSurrounds`: Rename variable. * `getChunk`: Avoid side effects. It adds a few lines for callers, but it does not modify any more arguments and it does what it says: getting a chunk. * `CheckSlashable`: Flatten. * `detect_attestations_test.go`: Simplify. * `CheckSlashable`: Add error log in case of missing attestation. * `processQueuedAttestations`: Extract a sub function. So testing will be easier. * `processAttesterSlashings` and `processProposerSlashings`: Improve. * `processAttesterSlashings`: Return processed slashings. * `createAttestationWrapper`: Rename variables. * `signingRoot` ==> `headerRoot` or `dataRoot`. Before this commit, there is two typse of `signing root`s floating around. - The first one is a real signing root, aka a hash tree root computed from an object root and a domain. This real signing root is the object ready to be signed. - The second one is a "false" signing root, which is actually just the hash tree root of an object. This object is either the `Data` field of an attestation, or the `Header` field of a block. Having 2 differents objects with the same name `signing root` is quite confusing. This commit renames wrongly named `signing root` objects. * `createAttestationWrapper` => `createAttestationWrapperEmptySig`. So it's clear for the user that the created attestation wrapper has an empty signature. * Implement `createAttestationWrapper`. * `processAttestations`: Return processed attester slashings. * Test `processAttestations` instead of `processQueuedAttestations`. By testing `processAttestations` instead of `processQueuedAttestations`, we get rid of a lot of tests fixtures, including the 200 ms sleep. The whole testing duration is shorter. * `Test_processAttestations`: Allow multiple steps. * `Test_processAttestations`: Add double steps tests. Some new failing tests are commented with a corresponding github issue. * `NextChunkStartEpoch`: Fix function comment. Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com> * `chunks.go`: Avoid templating log messages. * `checkSlashableAttestations`: Simplify duration computation. --------- Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
2024-02-09 21:02:18 +00:00
for _, attestation := range attestations {
chunkIndex := s.params.chunkIndex(attestation.IndexedAttestation.Data.Source.Epoch)
attestationsByChunkIndex[chunkIndex] = append(attestationsByChunkIndex[chunkIndex], attestation)
}
Slasher: Refactor and add tests (#13589) * `helpers.go`: Improve naming consistency. * `detect_attestations.go`: Improve readability. * `receive.go`: Add `attsQueueSize` in log message. * `checkSlashableAttestations`: Improve logging. `avgBatchProcessingTime` is not displayed any more if not batch is processed. * `loadChunks`: Use explicit `chunkKind` and `chunkIndices`. * `getChunk`: Use specific `chunkIndex` and `chunkKind`. * `validatorIndicesInChunk` -> `validatorIndexesInChunk`. * `epochUpdateForValidator`: Use explicit arguments. * `getChunk`: Change order of arguments. * `latestEpochWrittenForValidator`: Use `ok` parameter. So the default value is not any more considered as the absence of value. * `applyAttestationForValidator`: Use explicit arguments. * `updateSpans`: Use explicit arguments. * `saveUpdatedChunks`: Use explicit arguments. * `checkSurrounds`: Use explicit arguments. We see here that, previously, in `checkSlashableAttestations`, `checkSurrounds` was called with the default value of `slashertypes`: `MinSpan`. Now, we set it expliciterly at `MinSpan`, which may explicit a bug. * `epochUpdateForValidator`: Set modified by the function argument first. * `applyAttestationForValidator`: Set mutated argument `chunksByChunkIdx`first. * `applyAttestationForValidator`: Rename variables. * `Test_processQueuedAttestations`: Fix test. Two tests were actually exactly the same. * `updateSpans`: Keep happy path in the outer scope. Even if in this case the "happy" path means slashing. * `checkSurrounds`: Rename variable. * `getChunk`: Avoid side effects. It adds a few lines for callers, but it does not modify any more arguments and it does what it says: getting a chunk. * `CheckSlashable`: Flatten. * `detect_attestations_test.go`: Simplify. * `CheckSlashable`: Add error log in case of missing attestation. * `processQueuedAttestations`: Extract a sub function. So testing will be easier. * `processAttesterSlashings` and `processProposerSlashings`: Improve. * `processAttesterSlashings`: Return processed slashings. * `createAttestationWrapper`: Rename variables. * `signingRoot` ==> `headerRoot` or `dataRoot`. Before this commit, there is two typse of `signing root`s floating around. - The first one is a real signing root, aka a hash tree root computed from an object root and a domain. This real signing root is the object ready to be signed. - The second one is a "false" signing root, which is actually just the hash tree root of an object. This object is either the `Data` field of an attestation, or the `Header` field of a block. Having 2 differents objects with the same name `signing root` is quite confusing. This commit renames wrongly named `signing root` objects. * `createAttestationWrapper` => `createAttestationWrapperEmptySig`. So it's clear for the user that the created attestation wrapper has an empty signature. * Implement `createAttestationWrapper`. * `processAttestations`: Return processed attester slashings. * Test `processAttestations` instead of `processQueuedAttestations`. By testing `processAttestations` instead of `processQueuedAttestations`, we get rid of a lot of tests fixtures, including the 200 ms sleep. The whole testing duration is shorter. * `Test_processAttestations`: Allow multiple steps. * `Test_processAttestations`: Add double steps tests. Some new failing tests are commented with a corresponding github issue. * `NextChunkStartEpoch`: Fix function comment. Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com> * `chunks.go`: Avoid templating log messages. * `checkSlashableAttestations`: Simplify duration computation. --------- Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
2024-02-09 21:02:18 +00:00
return attestationsByChunkIndex
}
// This function returns a list of valid attestations, a list of attestations that are
// valid in the future, and the number of attestations dropped.
func (s *Service) filterAttestations(
Beacon node slasher improvement (#13549) * Slasher: Ensure all gorouting are stopped before running `Stop` actions. Fixes #13550. In tests, `exitChan` are now useless since waitgroup are used to wait for all goroutines to be stopped. * `slasher.go`: Add comments and rename some variables. - NFC * `detect_blocks.go`: Improve. - NFC - Rename some variables. - Add comments. - Use second element of `range` when possible. * `chunks.go`: Remove `_`receivers. - NFC * `validateAttestationIntegrity`: Improve documentation. - NFC * `filterAttestations`: Avoid `else`and rename variable. - NFC * `slasher.go`: Fix and add comments. * `SaveAttestationRecordsForValidators`: Remove unused code. * `LastEpochWrittenForValidators`: Name variables consistently. - NFC Avoid mixes between `indice(s)`and `index(es)`. * `SaveLastEpochsWrittenForValidators`: Name variables consistently. - NFC * `CheckAttesterDoubleVotes`: Rename variables and add comments. - NFC * `schema.go`: Add comments. - NFC * `processQueuedAttestations`: Add comments. - NFC * `checkDoubleVotes`: Rename variable. - NFC * `Test_processQueuedAttestations`: Ensure there is no error log. * `shouldNotBeSlashable` => `shouldBeSlashable` * `Test_processQueuedAttestations`: Add 2 test cases: - Same target with different signing roots - Same target with same signing roots * `checkDoubleVotesOnDisk` ==> `checkDoubleVotes`. Before this commit, `checkDoubleVotes` did two tasks: - Checking if there are any slashable double votes in the input list of attestations with respect to each other. - Checking if there are any slashable double votes in the input list of attestations with respect to our database. However, `checkDoubleVotes` is called only in `checkSlashableAttestations`. And `checkSlashableAttestations` is called only in: - `processQueuedAttestations`, and in - `IsSlashableAttestation` Study of case `processQueuedAttestations`: --------------------------------------------- In `processQueuedAttestations`, `checkSlashableAttestations` is ALWAYS called after `Database.SaveAttestationRecordsForValidators`. It means that, when calling `checkSlashableAttestations`, `validAtts` are ALREADY stored in the DB. Each attestation of `validAtts` will be checked twice: - Against the other attestations of `validAtts` (the portion of deleted code) - Against the content of the database. One of those two checks is redundent. ==> We can remove the check against other attestations in `validAtts`. Study of case `Database.SaveAttestationRecordsForValidators`: ---------------------------------------------------------------- In `Database.SaveAttestationRecordsForValidators`, `checkSlashableAttestations` is ALWAYS called with a list of attestations containing only ONE attestation. This only attestaion will be checked twice: - Against itself, and an attestation cannot conflict with itself. - Against the content of the database. ==> We can remove the check against other attestations in `validAtts`. ========================= In both cases, we showed that we can remove the check of attestation against the content of `validAtts`, and the corresponding test `Test_checkDoubleVotes_SlashableInputAttestations`. * `Test_processQueuedBlocks_DetectsDoubleProposals`: Wrap proposals. So we can add new proposals later. * Fix slasher multiple proposals false negative. If a first batch of blocks is sent with: - validator 1 - slot 4 - signing root 1 - validator 1 - slot 5 - signing root 1 Then, if a second batch of blocks is sent with: - validator 1 - slot 4 - signing root 2 Because we have two blocks proposed by the same validator (1) and for the same slot (4), but with two different signing roots (1 and 2), the validator 1 should be slashed. This is not the case before this commit. A new test case has been added as well to check this. Fixes #13551 * `params.go`: Change comments. - NFC * `CheckSlashable`: Keep the happy path without indentation. * `detectAllAttesterSlashings` => `checkSurrounds`. * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * `CheckAttesterDoubleVotes`: Keep happy path without indentation. Well, even if, in our case, "happy path" mean slashing. * 'SaveAttestationRecordsForValidators': Save the first attestation. In case of multiple votes, arbitrarily save the first attestation. Saving the first one in particular has no functional impact, since in any case all attestations will be tested against the content of the database. So all but the first one will be detected as slashable. However, saving the first one and not an other one let us not to modify the end to end tests, since they expect the first one to be saved in the database. * Rename `min` => `minimum`. Not to conflict with the new `min` built-in function. * `couldNotSaveSlashableAtt` ==> `couldNotCheckSlashableAtt` --------- Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
2024-01-31 09:49:14 +00:00
attWrappers []*slashertypes.IndexedAttestationWrapper, currentEpoch primitives.Epoch,
) (valid, validInFuture []*slashertypes.IndexedAttestationWrapper, numDropped int) {
Beacon node slasher improvement (#13549) * Slasher: Ensure all gorouting are stopped before running `Stop` actions. Fixes #13550. In tests, `exitChan` are now useless since waitgroup are used to wait for all goroutines to be stopped. * `slasher.go`: Add comments and rename some variables. - NFC * `detect_blocks.go`: Improve. - NFC - Rename some variables. - Add comments. - Use second element of `range` when possible. * `chunks.go`: Remove `_`receivers. - NFC * `validateAttestationIntegrity`: Improve documentation. - NFC * `filterAttestations`: Avoid `else`and rename variable. - NFC * `slasher.go`: Fix and add comments. * `SaveAttestationRecordsForValidators`: Remove unused code. * `LastEpochWrittenForValidators`: Name variables consistently. - NFC Avoid mixes between `indice(s)`and `index(es)`. * `SaveLastEpochsWrittenForValidators`: Name variables consistently. - NFC * `CheckAttesterDoubleVotes`: Rename variables and add comments. - NFC * `schema.go`: Add comments. - NFC * `processQueuedAttestations`: Add comments. - NFC * `checkDoubleVotes`: Rename variable. - NFC * `Test_processQueuedAttestations`: Ensure there is no error log. * `shouldNotBeSlashable` => `shouldBeSlashable` * `Test_processQueuedAttestations`: Add 2 test cases: - Same target with different signing roots - Same target with same signing roots * `checkDoubleVotesOnDisk` ==> `checkDoubleVotes`. Before this commit, `checkDoubleVotes` did two tasks: - Checking if there are any slashable double votes in the input list of attestations with respect to each other. - Checking if there are any slashable double votes in the input list of attestations with respect to our database. However, `checkDoubleVotes` is called only in `checkSlashableAttestations`. And `checkSlashableAttestations` is called only in: - `processQueuedAttestations`, and in - `IsSlashableAttestation` Study of case `processQueuedAttestations`: --------------------------------------------- In `processQueuedAttestations`, `checkSlashableAttestations` is ALWAYS called after `Database.SaveAttestationRecordsForValidators`. It means that, when calling `checkSlashableAttestations`, `validAtts` are ALREADY stored in the DB. Each attestation of `validAtts` will be checked twice: - Against the other attestations of `validAtts` (the portion of deleted code) - Against the content of the database. One of those two checks is redundent. ==> We can remove the check against other attestations in `validAtts`. Study of case `Database.SaveAttestationRecordsForValidators`: ---------------------------------------------------------------- In `Database.SaveAttestationRecordsForValidators`, `checkSlashableAttestations` is ALWAYS called with a list of attestations containing only ONE attestation. This only attestaion will be checked twice: - Against itself, and an attestation cannot conflict with itself. - Against the content of the database. ==> We can remove the check against other attestations in `validAtts`. ========================= In both cases, we showed that we can remove the check of attestation against the content of `validAtts`, and the corresponding test `Test_checkDoubleVotes_SlashableInputAttestations`. * `Test_processQueuedBlocks_DetectsDoubleProposals`: Wrap proposals. So we can add new proposals later. * Fix slasher multiple proposals false negative. If a first batch of blocks is sent with: - validator 1 - slot 4 - signing root 1 - validator 1 - slot 5 - signing root 1 Then, if a second batch of blocks is sent with: - validator 1 - slot 4 - signing root 2 Because we have two blocks proposed by the same validator (1) and for the same slot (4), but with two different signing roots (1 and 2), the validator 1 should be slashed. This is not the case before this commit. A new test case has been added as well to check this. Fixes #13551 * `params.go`: Change comments. - NFC * `CheckSlashable`: Keep the happy path without indentation. * `detectAllAttesterSlashings` => `checkSurrounds`. * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * `CheckAttesterDoubleVotes`: Keep happy path without indentation. Well, even if, in our case, "happy path" mean slashing. * 'SaveAttestationRecordsForValidators': Save the first attestation. In case of multiple votes, arbitrarily save the first attestation. Saving the first one in particular has no functional impact, since in any case all attestations will be tested against the content of the database. So all but the first one will be detected as slashable. However, saving the first one and not an other one let us not to modify the end to end tests, since they expect the first one to be saved in the database. * Rename `min` => `minimum`. Not to conflict with the new `min` built-in function. * `couldNotSaveSlashableAtt` ==> `couldNotCheckSlashableAtt` --------- Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
2024-01-31 09:49:14 +00:00
valid = make([]*slashertypes.IndexedAttestationWrapper, 0, len(attWrappers))
validInFuture = make([]*slashertypes.IndexedAttestationWrapper, 0, len(attWrappers))
Beacon node slasher improvement (#13549) * Slasher: Ensure all gorouting are stopped before running `Stop` actions. Fixes #13550. In tests, `exitChan` are now useless since waitgroup are used to wait for all goroutines to be stopped. * `slasher.go`: Add comments and rename some variables. - NFC * `detect_blocks.go`: Improve. - NFC - Rename some variables. - Add comments. - Use second element of `range` when possible. * `chunks.go`: Remove `_`receivers. - NFC * `validateAttestationIntegrity`: Improve documentation. - NFC * `filterAttestations`: Avoid `else`and rename variable. - NFC * `slasher.go`: Fix and add comments. * `SaveAttestationRecordsForValidators`: Remove unused code. * `LastEpochWrittenForValidators`: Name variables consistently. - NFC Avoid mixes between `indice(s)`and `index(es)`. * `SaveLastEpochsWrittenForValidators`: Name variables consistently. - NFC * `CheckAttesterDoubleVotes`: Rename variables and add comments. - NFC * `schema.go`: Add comments. - NFC * `processQueuedAttestations`: Add comments. - NFC * `checkDoubleVotes`: Rename variable. - NFC * `Test_processQueuedAttestations`: Ensure there is no error log. * `shouldNotBeSlashable` => `shouldBeSlashable` * `Test_processQueuedAttestations`: Add 2 test cases: - Same target with different signing roots - Same target with same signing roots * `checkDoubleVotesOnDisk` ==> `checkDoubleVotes`. Before this commit, `checkDoubleVotes` did two tasks: - Checking if there are any slashable double votes in the input list of attestations with respect to each other. - Checking if there are any slashable double votes in the input list of attestations with respect to our database. However, `checkDoubleVotes` is called only in `checkSlashableAttestations`. And `checkSlashableAttestations` is called only in: - `processQueuedAttestations`, and in - `IsSlashableAttestation` Study of case `processQueuedAttestations`: --------------------------------------------- In `processQueuedAttestations`, `checkSlashableAttestations` is ALWAYS called after `Database.SaveAttestationRecordsForValidators`. It means that, when calling `checkSlashableAttestations`, `validAtts` are ALREADY stored in the DB. Each attestation of `validAtts` will be checked twice: - Against the other attestations of `validAtts` (the portion of deleted code) - Against the content of the database. One of those two checks is redundent. ==> We can remove the check against other attestations in `validAtts`. Study of case `Database.SaveAttestationRecordsForValidators`: ---------------------------------------------------------------- In `Database.SaveAttestationRecordsForValidators`, `checkSlashableAttestations` is ALWAYS called with a list of attestations containing only ONE attestation. This only attestaion will be checked twice: - Against itself, and an attestation cannot conflict with itself. - Against the content of the database. ==> We can remove the check against other attestations in `validAtts`. ========================= In both cases, we showed that we can remove the check of attestation against the content of `validAtts`, and the corresponding test `Test_checkDoubleVotes_SlashableInputAttestations`. * `Test_processQueuedBlocks_DetectsDoubleProposals`: Wrap proposals. So we can add new proposals later. * Fix slasher multiple proposals false negative. If a first batch of blocks is sent with: - validator 1 - slot 4 - signing root 1 - validator 1 - slot 5 - signing root 1 Then, if a second batch of blocks is sent with: - validator 1 - slot 4 - signing root 2 Because we have two blocks proposed by the same validator (1) and for the same slot (4), but with two different signing roots (1 and 2), the validator 1 should be slashed. This is not the case before this commit. A new test case has been added as well to check this. Fixes #13551 * `params.go`: Change comments. - NFC * `CheckSlashable`: Keep the happy path without indentation. * `detectAllAttesterSlashings` => `checkSurrounds`. * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * `CheckAttesterDoubleVotes`: Keep happy path without indentation. Well, even if, in our case, "happy path" mean slashing. * 'SaveAttestationRecordsForValidators': Save the first attestation. In case of multiple votes, arbitrarily save the first attestation. Saving the first one in particular has no functional impact, since in any case all attestations will be tested against the content of the database. So all but the first one will be detected as slashable. However, saving the first one and not an other one let us not to modify the end to end tests, since they expect the first one to be saved in the database. * Rename `min` => `minimum`. Not to conflict with the new `min` built-in function. * `couldNotSaveSlashableAtt` ==> `couldNotCheckSlashableAtt` --------- Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
2024-01-31 09:49:14 +00:00
for _, attWrapper := range attWrappers {
if attWrapper == nil || !validateAttestationIntegrity(attWrapper.IndexedAttestation) {
numDropped++
continue
}
// If an attestation's source is epoch is older than the max history length
// we keep track of for slashing detection, we drop it.
if attWrapper.IndexedAttestation.Data.Source.Epoch+s.params.historyLength <= currentEpoch {
numDropped++
continue
}
// If an attestations's target epoch is in the future, we defer processing for later.
if attWrapper.IndexedAttestation.Data.Target.Epoch > currentEpoch {
validInFuture = append(validInFuture, attWrapper)
Beacon node slasher improvement (#13549) * Slasher: Ensure all gorouting are stopped before running `Stop` actions. Fixes #13550. In tests, `exitChan` are now useless since waitgroup are used to wait for all goroutines to be stopped. * `slasher.go`: Add comments and rename some variables. - NFC * `detect_blocks.go`: Improve. - NFC - Rename some variables. - Add comments. - Use second element of `range` when possible. * `chunks.go`: Remove `_`receivers. - NFC * `validateAttestationIntegrity`: Improve documentation. - NFC * `filterAttestations`: Avoid `else`and rename variable. - NFC * `slasher.go`: Fix and add comments. * `SaveAttestationRecordsForValidators`: Remove unused code. * `LastEpochWrittenForValidators`: Name variables consistently. - NFC Avoid mixes between `indice(s)`and `index(es)`. * `SaveLastEpochsWrittenForValidators`: Name variables consistently. - NFC * `CheckAttesterDoubleVotes`: Rename variables and add comments. - NFC * `schema.go`: Add comments. - NFC * `processQueuedAttestations`: Add comments. - NFC * `checkDoubleVotes`: Rename variable. - NFC * `Test_processQueuedAttestations`: Ensure there is no error log. * `shouldNotBeSlashable` => `shouldBeSlashable` * `Test_processQueuedAttestations`: Add 2 test cases: - Same target with different signing roots - Same target with same signing roots * `checkDoubleVotesOnDisk` ==> `checkDoubleVotes`. Before this commit, `checkDoubleVotes` did two tasks: - Checking if there are any slashable double votes in the input list of attestations with respect to each other. - Checking if there are any slashable double votes in the input list of attestations with respect to our database. However, `checkDoubleVotes` is called only in `checkSlashableAttestations`. And `checkSlashableAttestations` is called only in: - `processQueuedAttestations`, and in - `IsSlashableAttestation` Study of case `processQueuedAttestations`: --------------------------------------------- In `processQueuedAttestations`, `checkSlashableAttestations` is ALWAYS called after `Database.SaveAttestationRecordsForValidators`. It means that, when calling `checkSlashableAttestations`, `validAtts` are ALREADY stored in the DB. Each attestation of `validAtts` will be checked twice: - Against the other attestations of `validAtts` (the portion of deleted code) - Against the content of the database. One of those two checks is redundent. ==> We can remove the check against other attestations in `validAtts`. Study of case `Database.SaveAttestationRecordsForValidators`: ---------------------------------------------------------------- In `Database.SaveAttestationRecordsForValidators`, `checkSlashableAttestations` is ALWAYS called with a list of attestations containing only ONE attestation. This only attestaion will be checked twice: - Against itself, and an attestation cannot conflict with itself. - Against the content of the database. ==> We can remove the check against other attestations in `validAtts`. ========================= In both cases, we showed that we can remove the check of attestation against the content of `validAtts`, and the corresponding test `Test_checkDoubleVotes_SlashableInputAttestations`. * `Test_processQueuedBlocks_DetectsDoubleProposals`: Wrap proposals. So we can add new proposals later. * Fix slasher multiple proposals false negative. If a first batch of blocks is sent with: - validator 1 - slot 4 - signing root 1 - validator 1 - slot 5 - signing root 1 Then, if a second batch of blocks is sent with: - validator 1 - slot 4 - signing root 2 Because we have two blocks proposed by the same validator (1) and for the same slot (4), but with two different signing roots (1 and 2), the validator 1 should be slashed. This is not the case before this commit. A new test case has been added as well to check this. Fixes #13551 * `params.go`: Change comments. - NFC * `CheckSlashable`: Keep the happy path without indentation. * `detectAllAttesterSlashings` => `checkSurrounds`. * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * `CheckAttesterDoubleVotes`: Keep happy path without indentation. Well, even if, in our case, "happy path" mean slashing. * 'SaveAttestationRecordsForValidators': Save the first attestation. In case of multiple votes, arbitrarily save the first attestation. Saving the first one in particular has no functional impact, since in any case all attestations will be tested against the content of the database. So all but the first one will be detected as slashable. However, saving the first one and not an other one let us not to modify the end to end tests, since they expect the first one to be saved in the database. * Rename `min` => `minimum`. Not to conflict with the new `min` built-in function. * `couldNotSaveSlashableAtt` ==> `couldNotCheckSlashableAtt` --------- Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
2024-01-31 09:49:14 +00:00
continue
}
Beacon node slasher improvement (#13549) * Slasher: Ensure all gorouting are stopped before running `Stop` actions. Fixes #13550. In tests, `exitChan` are now useless since waitgroup are used to wait for all goroutines to be stopped. * `slasher.go`: Add comments and rename some variables. - NFC * `detect_blocks.go`: Improve. - NFC - Rename some variables. - Add comments. - Use second element of `range` when possible. * `chunks.go`: Remove `_`receivers. - NFC * `validateAttestationIntegrity`: Improve documentation. - NFC * `filterAttestations`: Avoid `else`and rename variable. - NFC * `slasher.go`: Fix and add comments. * `SaveAttestationRecordsForValidators`: Remove unused code. * `LastEpochWrittenForValidators`: Name variables consistently. - NFC Avoid mixes between `indice(s)`and `index(es)`. * `SaveLastEpochsWrittenForValidators`: Name variables consistently. - NFC * `CheckAttesterDoubleVotes`: Rename variables and add comments. - NFC * `schema.go`: Add comments. - NFC * `processQueuedAttestations`: Add comments. - NFC * `checkDoubleVotes`: Rename variable. - NFC * `Test_processQueuedAttestations`: Ensure there is no error log. * `shouldNotBeSlashable` => `shouldBeSlashable` * `Test_processQueuedAttestations`: Add 2 test cases: - Same target with different signing roots - Same target with same signing roots * `checkDoubleVotesOnDisk` ==> `checkDoubleVotes`. Before this commit, `checkDoubleVotes` did two tasks: - Checking if there are any slashable double votes in the input list of attestations with respect to each other. - Checking if there are any slashable double votes in the input list of attestations with respect to our database. However, `checkDoubleVotes` is called only in `checkSlashableAttestations`. And `checkSlashableAttestations` is called only in: - `processQueuedAttestations`, and in - `IsSlashableAttestation` Study of case `processQueuedAttestations`: --------------------------------------------- In `processQueuedAttestations`, `checkSlashableAttestations` is ALWAYS called after `Database.SaveAttestationRecordsForValidators`. It means that, when calling `checkSlashableAttestations`, `validAtts` are ALREADY stored in the DB. Each attestation of `validAtts` will be checked twice: - Against the other attestations of `validAtts` (the portion of deleted code) - Against the content of the database. One of those two checks is redundent. ==> We can remove the check against other attestations in `validAtts`. Study of case `Database.SaveAttestationRecordsForValidators`: ---------------------------------------------------------------- In `Database.SaveAttestationRecordsForValidators`, `checkSlashableAttestations` is ALWAYS called with a list of attestations containing only ONE attestation. This only attestaion will be checked twice: - Against itself, and an attestation cannot conflict with itself. - Against the content of the database. ==> We can remove the check against other attestations in `validAtts`. ========================= In both cases, we showed that we can remove the check of attestation against the content of `validAtts`, and the corresponding test `Test_checkDoubleVotes_SlashableInputAttestations`. * `Test_processQueuedBlocks_DetectsDoubleProposals`: Wrap proposals. So we can add new proposals later. * Fix slasher multiple proposals false negative. If a first batch of blocks is sent with: - validator 1 - slot 4 - signing root 1 - validator 1 - slot 5 - signing root 1 Then, if a second batch of blocks is sent with: - validator 1 - slot 4 - signing root 2 Because we have two blocks proposed by the same validator (1) and for the same slot (4), but with two different signing roots (1 and 2), the validator 1 should be slashed. This is not the case before this commit. A new test case has been added as well to check this. Fixes #13551 * `params.go`: Change comments. - NFC * `CheckSlashable`: Keep the happy path without indentation. * `detectAllAttesterSlashings` => `checkSurrounds`. * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * `CheckAttesterDoubleVotes`: Keep happy path without indentation. Well, even if, in our case, "happy path" mean slashing. * 'SaveAttestationRecordsForValidators': Save the first attestation. In case of multiple votes, arbitrarily save the first attestation. Saving the first one in particular has no functional impact, since in any case all attestations will be tested against the content of the database. So all but the first one will be detected as slashable. However, saving the first one and not an other one let us not to modify the end to end tests, since they expect the first one to be saved in the database. * Rename `min` => `minimum`. Not to conflict with the new `min` built-in function. * `couldNotSaveSlashableAtt` ==> `couldNotCheckSlashableAtt` --------- Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
2024-01-31 09:49:14 +00:00
// The attestation is valid.
valid = append(valid, attWrapper)
}
return
}
// Validates the attestation data integrity, ensuring we have no nil values for
Beacon node slasher improvement (#13549) * Slasher: Ensure all gorouting are stopped before running `Stop` actions. Fixes #13550. In tests, `exitChan` are now useless since waitgroup are used to wait for all goroutines to be stopped. * `slasher.go`: Add comments and rename some variables. - NFC * `detect_blocks.go`: Improve. - NFC - Rename some variables. - Add comments. - Use second element of `range` when possible. * `chunks.go`: Remove `_`receivers. - NFC * `validateAttestationIntegrity`: Improve documentation. - NFC * `filterAttestations`: Avoid `else`and rename variable. - NFC * `slasher.go`: Fix and add comments. * `SaveAttestationRecordsForValidators`: Remove unused code. * `LastEpochWrittenForValidators`: Name variables consistently. - NFC Avoid mixes between `indice(s)`and `index(es)`. * `SaveLastEpochsWrittenForValidators`: Name variables consistently. - NFC * `CheckAttesterDoubleVotes`: Rename variables and add comments. - NFC * `schema.go`: Add comments. - NFC * `processQueuedAttestations`: Add comments. - NFC * `checkDoubleVotes`: Rename variable. - NFC * `Test_processQueuedAttestations`: Ensure there is no error log. * `shouldNotBeSlashable` => `shouldBeSlashable` * `Test_processQueuedAttestations`: Add 2 test cases: - Same target with different signing roots - Same target with same signing roots * `checkDoubleVotesOnDisk` ==> `checkDoubleVotes`. Before this commit, `checkDoubleVotes` did two tasks: - Checking if there are any slashable double votes in the input list of attestations with respect to each other. - Checking if there are any slashable double votes in the input list of attestations with respect to our database. However, `checkDoubleVotes` is called only in `checkSlashableAttestations`. And `checkSlashableAttestations` is called only in: - `processQueuedAttestations`, and in - `IsSlashableAttestation` Study of case `processQueuedAttestations`: --------------------------------------------- In `processQueuedAttestations`, `checkSlashableAttestations` is ALWAYS called after `Database.SaveAttestationRecordsForValidators`. It means that, when calling `checkSlashableAttestations`, `validAtts` are ALREADY stored in the DB. Each attestation of `validAtts` will be checked twice: - Against the other attestations of `validAtts` (the portion of deleted code) - Against the content of the database. One of those two checks is redundent. ==> We can remove the check against other attestations in `validAtts`. Study of case `Database.SaveAttestationRecordsForValidators`: ---------------------------------------------------------------- In `Database.SaveAttestationRecordsForValidators`, `checkSlashableAttestations` is ALWAYS called with a list of attestations containing only ONE attestation. This only attestaion will be checked twice: - Against itself, and an attestation cannot conflict with itself. - Against the content of the database. ==> We can remove the check against other attestations in `validAtts`. ========================= In both cases, we showed that we can remove the check of attestation against the content of `validAtts`, and the corresponding test `Test_checkDoubleVotes_SlashableInputAttestations`. * `Test_processQueuedBlocks_DetectsDoubleProposals`: Wrap proposals. So we can add new proposals later. * Fix slasher multiple proposals false negative. If a first batch of blocks is sent with: - validator 1 - slot 4 - signing root 1 - validator 1 - slot 5 - signing root 1 Then, if a second batch of blocks is sent with: - validator 1 - slot 4 - signing root 2 Because we have two blocks proposed by the same validator (1) and for the same slot (4), but with two different signing roots (1 and 2), the validator 1 should be slashed. This is not the case before this commit. A new test case has been added as well to check this. Fixes #13551 * `params.go`: Change comments. - NFC * `CheckSlashable`: Keep the happy path without indentation. * `detectAllAttesterSlashings` => `checkSurrounds`. * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * Update beacon-chain/db/slasherkv/slasher.go Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> * `CheckAttesterDoubleVotes`: Keep happy path without indentation. Well, even if, in our case, "happy path" mean slashing. * 'SaveAttestationRecordsForValidators': Save the first attestation. In case of multiple votes, arbitrarily save the first attestation. Saving the first one in particular has no functional impact, since in any case all attestations will be tested against the content of the database. So all but the first one will be detected as slashable. However, saving the first one and not an other one let us not to modify the end to end tests, since they expect the first one to be saved in the database. * Rename `min` => `minimum`. Not to conflict with the new `min` built-in function. * `couldNotSaveSlashableAtt` ==> `couldNotCheckSlashableAtt` --------- Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
2024-01-31 09:49:14 +00:00
// source and target epochs, and that the source epoch of the attestation must
// be less than the target epoch, which is a precondition for performing slashing
// detection (except for the genesis epoch).
func validateAttestationIntegrity(att *ethpb.IndexedAttestation) bool {
// If an attestation is malformed, we drop it.
if att == nil ||
att.Data == nil ||
att.Data.Source == nil ||
att.Data.Target == nil {
return false
}
sourceEpoch := att.Data.Source.Epoch
targetEpoch := att.Data.Target.Epoch
// The genesis epoch is a special case, since all attestations formed in it
// will have source and target 0, and they should be considered valid.
if sourceEpoch == 0 && targetEpoch == 0 {
return true
}
// All valid attestations must have source epoch < target epoch.
return sourceEpoch < targetEpoch
}
// Validates the signed beacon block header integrity, ensuring we have no nil values.
func validateBlockHeaderIntegrity(header *ethpb.SignedBeaconBlockHeader) bool {
// If a signed block header is malformed, we drop it.
if header == nil ||
header.Header == nil ||
len(header.Signature) != fieldparams.BLSSignatureLength ||
bytes.Equal(header.Signature, make([]byte, fieldparams.BLSSignatureLength)) {
return false
}
return true
}
func logAttesterSlashing(slashing *ethpb.AttesterSlashing) {
indices := slice.IntersectionUint64(slashing.Attestation_1.AttestingIndices, slashing.Attestation_2.AttestingIndices)
log.WithFields(logrus.Fields{
"validatorIndex": indices,
"prevSourceEpoch": slashing.Attestation_1.Data.Source.Epoch,
"prevTargetEpoch": slashing.Attestation_1.Data.Target.Epoch,
"sourceEpoch": slashing.Attestation_2.Data.Source.Epoch,
"targetEpoch": slashing.Attestation_2.Data.Target.Epoch,
}).Info("Attester slashing detected")
}
func logProposerSlashing(slashing *ethpb.ProposerSlashing) {
log.WithFields(logrus.Fields{
"validatorIndex": slashing.Header_1.Header.ProposerIndex,
"slot": slashing.Header_1.Header.Slot,
}).Info("Proposer slashing detected")
}
// Turns a uint64 value to a string representation.
func uintToString(val uint64) string {
return strconv.FormatUint(val, 10)
}
// If an existing signing root does not match an incoming proposal signing root,
// we then have a double block proposer slashing event.
func isDoubleProposal(incomingSigningRoot, existingSigningRoot [32]byte) bool {
// If the existing signing root is the zero hash, we do not consider
// this a double proposal.
if existingSigningRoot == params.BeaconConfig().ZeroHash {
return false
}
return incomingSigningRoot != existingSigningRoot
}