* 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>
* backfill service
* fix bug where origin state is never unlocked
* support mvslice states
* use renamed interface
* refactor db code to skip block cache for backfill
* lint
* add test for verifier.verify
* enable service in service init test
* cancellation cleanup
* adding nil checks to configset juggling
* assume blocks are available by default
As long as we're sure the AvailableBlocker is initialized correctly
during node startup, defaulting to assuming we aren't in a checkpoint
sync simplifies things greatly for tests.
* block saving path refactor and bugfix
* fix fillback test
* fix BackfillStatus init tests
---------
Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>
* Check init sync before getting payload attributes
This PR adds a helper to forkchoice to return the delay of the latest
imported block. It also adds a helper with an heuristic to check if the
node is during init sync. If the highest imported node was imported with
a delay of less than an epoch then the node is considered in regular
sync. If on the other hand, in addition the highest imported node is
more than two epochs old, then the node is considered in init Sync.
The helper to check this only uses forkchoice and therefore requires a
read lock. There are four paths to call this
1) During regular block processing, we defer a function to send the
second FCU call with attributes. This function may not be called at
all if we are not regularly syncing
2) During regular block processing, we check in the path
`postBlockProces->getFCUArgs->computePayloadAttributes` the payload
attributes if we are syncing a late block. In this case forkchoice is
already locked and we add a call in `getFCUArgs` to return early if not
regularly syncing
3) During handling of late blocks on `lateBlockTasks` we simply return
early if not in regular sync (This is the biggest change as it takes
a longer FC lock for lateBlockTasks)
4) On Attestation processing, in UpdateHead, we are already locked so we
just add a check to not update head on this path if not regularly
syncing.
* fix build
* Fix mocks
* do not check optimistic status if cached attestation
* Gazelle
* Gazelle again
* fix nil panics
* more nil checks
* more nil checks
---------
Co-authored-by: terence tsao <terence@prysmaticlabs.com>
* Defragment head state
* change log level
* change it to be more efficient
* add flag
* add tests and clean up
* fix it
* gosimple
* Update container/multi-value-slice/multi_value_slice.go
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
* radek's review
* unlock it
* remove from fc lock
---------
Co-authored-by: rkapka <rkapka@wp.pl>
