-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement EIP-3076 minimal slashing protection, using a filesystem database #13360
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nalepae
force-pushed
the
11616-minimal
branch
30 times, most recently
from
December 22, 2023 12:40
0bb4184
to
c11c02b
Compare
Options were: - `--source` (for source data directory), and - `--target` (for target data directory) However, since this command deals with slashing protection, which has source (epochs) and target (epochs), the initial option names may confuse the user. In this commit: `--source` ==> `--source-data-dir` `--target` ==> `--target-data-dir`
And delete `CheckSlashableAttestation` from iface.
No functional change.
==> `filesystem` does not depend anymore on `kv`. ==> `iface` does not depend anymore on `kv`. ==> `slashing-protection` does not depend anymore on `kv`.
This way, we can: - Avoid any circular import for tests. - Implement once for all `iface.ValidatorDB` implementations the `ValidateMetadata`function. - Have tests (and coverage) of `ValidateMetadata`in its own package. The ideal solution would have been to implement `ValidateMetadata` as a method with the `iface.ValidatorDB`receiver. Unfortunately, golang does not allow that.
The whole purpose of this commit is to avoid the `switch validatorDB.(type)` in `ImportStandardProtectionJSON`.
Before, `Exists` was only able to detect if a file exists. Now, this function takes an extra `File` or `Directory` argument. It detects either if a file or a directory exists. Before, if an error was returned by `os.Stat`, the the file was considered as non existing. Now, it is treated as a real error.
To be consistent with `slashing-protection`.
rauljordan
approved these changes
Mar 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Feature
What does this PR do? Why is it needed?
This pull request implements EIP-3076 minimal slashing protection, using a filesystem database.
Which issues(s) does this PR fix?
Fixes #12475
Other notes for review
This PR is designed to be read commit by commit.
This PR implements the flag
--enable-minimal-slashing-protection
in the validator client.If this flag is used, then the former BoltDB
validator.db
database is not used any more.Instead, a pure filesystem, YAML database is used, using a EIP-3076 minimal slashing protection database.
The filesystem structure is the following:
configuration.yaml
contains data relative to:An example of
configuration.yaml
file is:In the
slashing-protection
directory, each file contains slashing protection data relative a given public key.The public key itself is the name of the file.
An example of a file in the
slashing-protection
directory is:The content of each field is self-describing.
Regarding anti-slashing strategy:
latestSignedBlockSlot
stored in the database.lastSignedAttestationSourceEpoch
stored in the database, or iflastSignedAttestationSourceEpoch
stored in the database.Strategy at VC start:
--enable-minimal-slashing-protection
is set AND a complete, BoltDB database is found, then a warning message is displayed indicating to the user how to convert the DB, and the VC continues with the complete, BoltDB database.--enable-minimal-slashing-protection
is NOT set AND a minimal, filesystem database is found, then the VC automatically convert the minimal, filesystem database into a complete, BoltDB one. (No data loss in this way)To convert a complete database into a minimal one, the user can do:
😱 This PR has 10k+ new lines! 😱
Actualy, a lot of modified lines in this PR result in tests transformed from something like:
to:
Git will consider all wrapped lines as new lines, even if only a few spaces had been added on the left.
For this reason, it is strongly advised to check the
Hide whitespace
option during the code review:Benchmarks:
We create
2000
public keys, and we call, in parallel,SaveAttestationForPubKey
on both complete, BoltDB and minimal, filesystem DBs.Benchmark results:
~130ms
~127ms
Comments:
For complete, BoltDB database,
SaveAttestationForPubKey
fills a queue which is dumped into the DB every 100 ms (or if the queue, containing up to2048
items, is full). It explains why the time needed is>= 100ms
. If we decide to save only10
attestations instead of2000
, the benchmark result is still~104ms
. Obviously, it cannot go under100ms
.