Serve API to return poet registrations state #6266
Conversation
- fixed review issues
- removed unused params
- removed unused params2
# Conflicts: # api/grpcserver/admin_service.go # api/grpcserver/grpcserver_test.go # api/grpcserver/node_service_test.go
# Conflicts: # activation/nipost.go # activation/nipost_test.go # sql/localsql/nipost/poet_registration.go
# Conflicts: # activation/nipost.go
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6266 +/- ##
=========================================
- Coverage 79.9% 79.9% -0.1%
=========================================
Files 353 355 +2
Lines 46396 46614 +218
=========================================
+ Hits 37103 37246 +143
- Misses 7191 7255 +64
- Partials 2102 2113 +11 ☔ View full report in Codecov by Sentry. |
|
Some quite minor nits, otherwise looks good |
| tab := newTestBuilder(t, 1, WithPoetConfig(PoetConfig{PhaseShift: layerDuration})) | ||
| ctrl := gomock.NewController(t) | ||
| idStates := NewMockIdentityStates(ctrl) | ||
| idStates.EXPECT().Set(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | ||
|
|
||
| tab := newTestBuilder(t, 1, | ||
| WithPoetConfig(PoetConfig{PhaseShift: layerDuration}), | ||
| WithIdentityStates(idStates)) | ||
|
|
There was a problem hiding this comment.
What's the point of adding the "identitystates` in these tests? It doesn't verify anything additionally (compared to the original test) but it complicates the tests.
| ctrl := gomock.NewController(t) | ||
| idStates := NewMockIdentityStates(ctrl) | ||
| idStates.EXPECT().Set(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | ||
|
|
||
| tab := newTestBuilder(t, 1, | ||
| WithPoetConfig(PoetConfig{PhaseShift: layerDuration}), | ||
| WithIdentityStates(idStates), |
There was a problem hiding this comment.
Ditto
| ctrl := gomock.NewController(t) | |
| idStates := NewMockIdentityStates(ctrl) | |
| idStates.EXPECT().Set(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() | |
| tab := newTestBuilder(t, 1, | |
| WithPoetConfig(PoetConfig{PhaseShift: layerDuration}), | |
| WithIdentityStates(idStates), | |
| tab := newTestBuilder(t, 1, | |
| WithPoetConfig(PoetConfig{PhaseShift: layerDuration}), |
| func (b *Builder) IdentityStates() map[types.IdentityDescriptor]IdentityState { | ||
| states := b.identitiesStates.All() | ||
| res := make(map[types.IdentityDescriptor]IdentityState, len(states)) | ||
| b.smeshingMutex.Lock() | ||
| defer b.smeshingMutex.Unlock() | ||
| for id, state := range states { | ||
| if sig, exists := b.signers[id]; exists { | ||
| res[sig] = state | ||
| } | ||
| } | ||
| return res | ||
| } |
There was a problem hiding this comment.
How about not adding this to the ATX Builder? The builder doesn't need this at all. AFAIR, it's only used in the grpc service, which could do read directly from the "identities state", wdyt?
We don't support unregistering, so the GRPC service could just return all the state of all IDs.
| if len(r.RoundID) == 0 { | ||
| continue // skip failed registrations | ||
| } |
There was a problem hiding this comment.
It's incorrect as the first poet round has ID 0. IMHO, it's better not to give existing fields "extra meaning" but to have a dedicated field.
|
Note: this PR depends on spacemeshos/api#369 |
Motivation
Smesher needs to understand situation with poet registrations (success, failed, residual, no registrations and etc.)
Easiest way is providing API for that.
Description
Closes #6174
Connected PR:
spacemeshos/api#369
Test Plan
Unit tests
TODO