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

client(consensus/grandpa): implement grandpaDb and environment types #3383

Merged
merged 17 commits into from
Oct 18, 2023

Conversation

jimjbrettj
Copy link
Contributor

@jimjbrettj jimjbrettj commented Jul 12, 2023

Changes

  • add an interface to represent grandpa DB interactions. After reviewing their auxDB impl, I don't think we need to implement all of it right now and believe we are ok having simple db interactions.
  • add logic in auxDB that is relevant to our implementation as well as tests
  • create all the environment types needed for our db impl and test them
  • add ability to decode custom vdts within maps and custom vdts to scale package

Tests

go test -tags integration github.com/ChainSafe/gossamer

Issues

Primary Reviewer

@EclesioMeloJunior

Copy link
Member

@EclesioMeloJunior EclesioMeloJunior left a comment

Choose a reason for hiding this comment

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

The keys and values are always bytes, I see that you are receiving strings, what I would suggest is to:

type databaseKeyValue [2][]byte

type AuxStore interface {
    Insert(toInsert []databaseKeyValue, toDelete [][]byte)
    Get(key []byte) []byte
}

ex: https://go.dev/play/p/C6oQRS_-Ett

client/consensus/grandpa/interfaces.go Outdated Show resolved Hide resolved
client/consensus/grandpa/interfaces.go Outdated Show resolved Hide resolved
@jimjbrettj jimjbrettj changed the title client(consensus/grandpa): add database interface client(consensus/grandpa): implement grandpaDb and environment types Aug 2, 2023
@jimjbrettj jimjbrettj force-pushed the jimmy/auxDbInterface branch from 48e8752 to 8e248c6 Compare August 8, 2023 17:48
@jimjbrettj jimjbrettj force-pushed the jimmy/auxDbInterface branch 2 times, most recently from 93a7265 to 04441f2 Compare August 8, 2023 20:32
client/consensus/grandpa/authorities.go Outdated Show resolved Hide resolved
client/consensus/grandpa/authorities.go Outdated Show resolved Hide resolved
client/consensus/grandpa/auxSchema.go Outdated Show resolved Hide resolved
client/consensus/grandpa/auxSchema.go Outdated Show resolved Hide resolved
client/consensus/grandpa/auxSchema.go Outdated Show resolved Hide resolved
client/consensus/grandpa/interfaces.go Outdated Show resolved Hide resolved
client/consensus/grandpa/interfaces.go Outdated Show resolved Hide resolved
client/consensus/grandpa/interfaces.go Outdated Show resolved Hide resolved
client/consensus/grandpa/lib.go Show resolved Hide resolved
pkg/scale/decode.go Outdated Show resolved Hide resolved
@timwu20 timwu20 force-pushed the jimmy/authorityChangeTree branch from a06ea09 to 293683a Compare August 16, 2023 16:53
@jimjbrettj jimjbrettj force-pushed the jimmy/auxDbInterface branch from 66a31fa to fd96db7 Compare August 16, 2023 20:48
client/consensus/grandpa/changeTree.go Outdated Show resolved Hide resolved
client/consensus/grandpa/aux_schema.go Outdated Show resolved Hide resolved
client/consensus/grandpa/aux_schema.go Outdated Show resolved Hide resolved
client/consensus/grandpa/interfaces.go Outdated Show resolved Hide resolved
client/consensus/grandpa/aux_schema.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
client/api/backend.go Outdated Show resolved Hide resolved
client/consensus/grandpa/authorities.go Outdated Show resolved Hide resolved
client/consensus/grandpa/aux_schema.go Outdated Show resolved Hide resolved
client/consensus/grandpa/aux_schema.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
client/consensus/grandpa/environment_test.go Show resolved Hide resolved
client/consensus/grandpa/environment.go Show resolved Hide resolved
client/consensus/grandpa/environment.go Outdated Show resolved Hide resolved
Base automatically changed from jimmy/authorityChangeTree to feat/grandpa October 10, 2023 16:48
@jimjbrettj jimjbrettj force-pushed the jimmy/auxDbInterface branch from 20bef8a to db5219f Compare October 10, 2023 17:49
@jimjbrettj jimjbrettj requested a review from timwu20 October 16, 2023 22:48
Copy link
Contributor

@timwu20 timwu20 left a comment

Choose a reason for hiding this comment

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

great work!

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.

3 participants