Skip to content
New issue

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

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

Change slasher DB structure to mirror beacon-chains #4848

Merged
merged 14 commits into from Feb 13, 2020

Conversation

0xKiwi
Copy link
Contributor

@0xKiwi 0xKiwi commented Feb 12, 2020

This PR changes the DB structure of the slasher to mirror the beacon chains DB, including a kv, testing, and iface package.

No functionality changes.

@0xKiwi 0xKiwi requested review from terencechain, shayzluf and rauljordan and removed request for terencechain February 13, 2020 00:02
)

// SlasherDB defines the necessary methods for a Prysm slasher DB.
type SlasherDB interface {
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to separate this to read only and read + write

Copy link
Member

Choose a reason for hiding this comment

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

+1 , define different interfaces for different access patterns

@@ -0,0 +1,54 @@
package types
Copy link
Member

Choose a reason for hiding this comment

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

Why is types under /slasher/db, do you think it should be under /slasher?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I move all types in there then? Types like Service, Config, Server?

Copy link
Contributor

Choose a reason for hiding this comment

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

These types are highly specific only to the db, I think we should just leave them as a subpackage under slasher/db/types @terencechain

@0xKiwi 0xKiwi changed the title Add interface and move slashing types to /types package Change slasher DB structure to mirror beacon-chains Feb 13, 2020
@codecov
Copy link

codecov bot commented Feb 13, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@0b75db6). Click here to learn what that means.
The diff coverage is 73.33%.

@@            Coverage Diff            @@
##             master    #4848   +/-   ##
=========================================
  Coverage          ?   24.52%           
=========================================
  Files             ?      168           
  Lines             ?    11749           
  Branches          ?        0           
=========================================
  Hits              ?     2881           
  Misses            ?     8373           
  Partials          ?      495

slasher/db/kv/kv.go Outdated Show resolved Hide resolved
slasher/db/testing/BUILD.bazel Outdated Show resolved Hide resolved
slasher/db/types/BUILD.bazel Outdated Show resolved Hide resolved
@@ -0,0 +1,54 @@
package types
Copy link
Contributor

Choose a reason for hiding this comment

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

These types are highly specific only to the db, I think we should just leave them as a subpackage under slasher/db/types @terencechain

slasher/db/types/types.go Outdated Show resolved Hide resolved
@rauljordan rauljordan merged commit c44a306 into prysmaticlabs:master Feb 13, 2020
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 24, 2020
* Add interface and move slashing types to /types package

* WIP restructure to match beacon chain DB

* Fix build

* Fix comment

* Fix comments

* fix comments for sure

* Use wrapper function for evict

* Remove unused

* Update slasher/db/kv/kv.go

* Update slasher/db/testing/BUILD.bazel

* Update slasher/db/types/BUILD.bazel

* Update slasher/db/types/types.go

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 28, 2020
* Add interface and move slashing types to /types package

* WIP restructure to match beacon chain DB

* Fix build

* Fix comment

* Fix comments

* fix comments for sure

* Use wrapper function for evict

* Remove unused

* Update slasher/db/kv/kv.go

* Update slasher/db/testing/BUILD.bazel

* Update slasher/db/types/BUILD.bazel

* Update slasher/db/types/types.go

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
@0xKiwi 0xKiwi deleted the slasher-add-interface branch March 15, 2020 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants