persist identity events, published proposals and calculated eligibilities #6577
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## node-split-poc #6577 +/- ##
================================================
- Coverage 77.2% 76.8% -0.4%
================================================
Files 342 346 +4
Lines 45839 46317 +478
================================================
+ Hits 35391 35607 +216
- Misses 8330 8585 +255
- Partials 2118 2125 +7 ☔ View full report in Codecov by Sentry. |
poszu
changed the title
Avoids duplicated: - PoetChallengeReady - PoetRegistered
poszu
force-pushed
the
node-split-persist-states
branch
2 times, most recently
from
0d3cc9c to
2e4734b
Compare
poszu
force-pushed
the
node-split-persist-states
branch
from
352e103 to
338ce11
Compare
poszu
requested review from
dshulyak,
fasmat,
ivan4th,
acud and
jellonek
as code owners
| @@ -101,7 +101,7 @@ func NewNIPostBuilder( | |||
| layerClock: layerClock, | |||
| postStates: NewPostStates(lg), | |||
| validator: validator, | |||
| identityStates: identity.NewIdentityStateStorage(), | |||
| identityStates: identity.NewIdentityStateStorage(db), | |||
There was a problem hiding this comment.
Every time I stumble upon this line I question why instead of a newly initialized repository/service we do not pass the one created in the activation.Builder or even node.go. And then I realise it's because we pass a mandatory dependency using a functional option 😅 Maybe that's worth changing?
| @@ -201,7 +201,7 @@ func NewBuilder( | |||
| logger: log, | |||
| poetRetryInterval: defaultPoetRetryInterval, | |||
| postStates: NewPostStates(log), | |||
| identitiesStates: identity.NewIdentityStateStorage(), | |||
| identitiesStates: identity.NewIdentityStateStorage(localDB), | |||
There was a problem hiding this comment.
same here - I don't think defaulting to this and passing the real identity service as functional parameter is correct 🤔
| // NOTE: The `require` doesn't support comparing time.Time after marhsaling | ||
| // (the monotonic counter is dropped in the process of serialization). | ||
| // We need to compare the values manually for some types. | ||
| // See: https://github.com/stretchr/testify/issues/502 |
There was a problem hiding this comment.
| // NOTE: The `require` doesn't support comparing time.Time after marhsaling | |
| // (the monotonic counter is dropped in the process of serialization). | |
| // We need to compare the values manually for some types. | |
| // See: https://github.com/stretchr/testify/issues/502 | |
| // NOTE: The `require` doesn't support comparing time.Time after marshaling | |
| // (the monotonic counter is dropped in the process of serialization). | |
| // We need to compare the values manually for some types. | |
| // See: https://github.com/stretchr/testify/issues/502 |
| s.identities[id] = append(s.identities[id], info) | ||
| stateBytes, err := marshalState(&info) | ||
| if err != nil { | ||
| panic(fmt.Sprintf("marhsaling state: %v", err)) |
There was a problem hiding this comment.
| panic(fmt.Sprintf("marhsaling state: %v", err)) | |
| panic(fmt.Sprintf("marshaling state: %v", err)) |
I'm not a big fan of just panicing here. How about adding a logger to StateStorage and at least log.Fatal or log.Panic instead? 🙂
| return nil | ||
| } | ||
|
|
||
| func InterateAllProposals( |
There was a problem hiding this comment.
| func InterateAllProposals( | |
| func IterateAllProposals( |
| return nil | ||
| } | ||
|
|
||
| func InterateAllEligibilities( |
There was a problem hiding this comment.
| func InterateAllEligibilities( | |
| func IterateAllEligibilities( |
Closes #6544
Persist identity events, published proposals and eligibilities in a local database.
The
StateStoragewas much simplified and is a simple proxy to the local DB.The events are persisted in a form of JSON. A tag is used to identify the right event type to deserialize to.
The proposal is serialized using scale to maintain as much information as possible but we could also just store: