From c8d37c9977d28fc394283fa5988a3501a2a7cdfe Mon Sep 17 00:00:00 2001 From: ConvallariaMaj Date: Thu, 29 Aug 2024 05:10:06 +0000 Subject: [PATCH] Cache the full response of the v1/info (#6274) ## Motivation Cache full response of the v1/info to decrease amount of requests to poet server --- activation/activation.go | 10 +-- activation/e2e/poet_client.go | 6 +- activation/e2e/poet_test.go | 38 ++-------- activation/interface.go | 2 +- activation/poet.go | 57 ++++++--------- activation/poet_client_test.go | 122 +++++++++++++++++++++------------ activation/poet_mocks.go | 39 ----------- config/mainnet.go | 2 +- config/presets/fastnet.go | 2 +- config/presets/standalone.go | 2 +- config/presets/testnet.go | 4 +- 11 files changed, 118 insertions(+), 166 deletions(-) diff --git a/activation/activation.go b/activation/activation.go index 59006a203a..8c1188f70f 100644 --- a/activation/activation.go +++ b/activation/activation.go @@ -51,17 +51,17 @@ type PoetConfig struct { RequestRetryDelay time.Duration `mapstructure:"retry-delay"` // Period to find positioning ATX. Must be less, than GracePeriod PositioningATXSelectionTimeout time.Duration `mapstructure:"positioning-atx-selection-timeout"` - CertifierInfoCacheTTL time.Duration `mapstructure:"certifier-info-cache-ttl"` + InfoCacheTTL time.Duration `mapstructure:"info-cache-ttl"` PowParamsCacheTTL time.Duration `mapstructure:"pow-params-cache-ttl"` MaxRequestRetries int `mapstructure:"retry-max"` } func DefaultPoetConfig() PoetConfig { return PoetConfig{ - RequestRetryDelay: 400 * time.Millisecond, - MaxRequestRetries: 10, - CertifierInfoCacheTTL: 5 * time.Minute, - PowParamsCacheTTL: 5 * time.Minute, + RequestRetryDelay: 400 * time.Millisecond, + MaxRequestRetries: 10, + InfoCacheTTL: 5 * time.Minute, + PowParamsCacheTTL: 5 * time.Minute, } } diff --git a/activation/e2e/poet_client.go b/activation/e2e/poet_client.go index 01fee564ec..97cd37d3f4 100644 --- a/activation/e2e/poet_client.go +++ b/activation/e2e/poet_client.go @@ -67,11 +67,7 @@ func (p *TestPoet) Submit( return &types.PoetRound{ID: strconv.Itoa(round), End: time.Now()}, nil } -func (p *TestPoet) CertifierInfo(ctx context.Context) (*types.CertifierInfo, error) { - return nil, errors.New("CertifierInfo: not supported") -} - -func (p *TestPoet) Info(ctx context.Context) (*types.PoetInfo, error) { +func (p *TestPoet) Info(_ context.Context) (*types.PoetInfo, error) { return &types.PoetInfo{ PhaseShift: p.poetCfg.PhaseShift, CycleGap: p.poetCfg.CycleGap, diff --git a/activation/e2e/poet_test.go b/activation/e2e/poet_test.go index 27d50f18e0..a7b4b80442 100644 --- a/activation/e2e/poet_test.go +++ b/activation/e2e/poet_test.go @@ -230,7 +230,7 @@ func TestSubmitTooLate(t *testing.T) { r.ErrorIs(err, activation.ErrInvalidRequest) } -func TestCertifierInfo(t *testing.T) { +func TestInfoWithCertifierInfo(t *testing.T) { t.Parallel() r := require.New(t) @@ -259,38 +259,8 @@ func TestCertifierInfo(t *testing.T) { ) require.NoError(t, err) - certInfo, err := client.CertifierInfo(context.Background()) + info, err := client.Info(context.Background()) r.NoError(err) - r.Equal("http://localhost:8080", certInfo.Url.String()) - r.Equal([]byte("pubkey"), certInfo.Pubkey) -} - -func TestNoCertifierInfo(t *testing.T) { - t.Parallel() - r := require.New(t) - - var eg errgroup.Group - poetDir := t.TempDir() - t.Cleanup(func() { r.NoError(eg.Wait()) }) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - c, err := NewHTTPPoetTestHarness(ctx, poetDir) - r.NoError(err) - r.NotNil(c) - - eg.Go(func() error { - err := c.Service.Start(ctx) - return errors.Join(err, c.Service.Close()) - }) - - client, err := activation.NewHTTPPoetClient( - types.PoetServer{Address: c.RestURL().String()}, - activation.DefaultPoetConfig(), - activation.WithLogger(zaptest.NewLogger(t)), - ) - require.NoError(t, err) - - _, err = client.CertifierInfo(context.Background()) - r.ErrorContains(err, "poet doesn't support certificates") + r.Equal("http://localhost:8080", info.Certifier.Url.String()) + r.Equal([]byte("pubkey"), info.Certifier.Pubkey) } diff --git a/activation/interface.go b/activation/interface.go index 21c414e9c5..5bd649f5ae 100644 --- a/activation/interface.go +++ b/activation/interface.go @@ -167,7 +167,7 @@ type certifierClient interface { // certifierService is used to certify nodeID for registering in the poet. // It holds the certificates and can recertify if needed. type certifierService interface { - // Acquire a certificate for the ID in the given certifier. + // Certificate acquires a certificate for the ID in the given certifier. // The certificate confirms that the ID is verified and it can be later used to submit in poet. Certificate( ctx context.Context, diff --git a/activation/poet.go b/activation/poet.go index 844cd0038b..2b723da477 100644 --- a/activation/poet.go +++ b/activation/poet.go @@ -34,6 +34,7 @@ var ( ErrUnauthorized = errors.New("unauthorized") ErrCertificatesNotSupported = errors.New("poet doesn't support certificates") ErrIncompatiblePhaseShift = errors.New("fetched poet phase_shift is incompatible with configured phase_shift") + ErrCertifierNotConfigured = errors.New("certifier service not configured") ) type PoetPowParams struct { @@ -56,7 +57,6 @@ type PoetClient interface { Address() string PowParams(ctx context.Context) (*PoetPowParams, error) - CertifierInfo(ctx context.Context) (*types.CertifierInfo, error) Submit( ctx context.Context, deadline time.Time, @@ -206,17 +206,6 @@ func (c *HTTPPoetClient) PowParams(ctx context.Context) (*PoetPowParams, error) }, nil } -func (c *HTTPPoetClient) CertifierInfo(ctx context.Context) (*types.CertifierInfo, error) { - info, err := c.Info(ctx) - if err != nil { - return nil, err - } - if info.Certifier == nil { - return nil, ErrCertificatesNotSupported - } - return info.Certifier, nil -} - // Submit registers a challenge in the proving service current open round. func (c *HTTPPoetClient) Submit( ctx context.Context, @@ -408,10 +397,8 @@ type poetService struct { certifier certifierService - certifierInfoCache cachedData[*types.CertifierInfo] - mtx sync.Mutex expectedPhaseShift time.Duration - fetchedPhaseShift time.Duration + infoCache cachedData[*types.PoetInfo] powParamsCache cachedData[*PoetPowParams] } @@ -455,7 +442,7 @@ func NewPoetServiceWithClient( logger: logger, client: client, requestTimeout: cfg.RequestTimeout, - certifierInfoCache: cachedData[*types.CertifierInfo]{ttl: cfg.CertifierInfoCacheTTL}, + infoCache: cachedData[*types.PoetInfo]{ttl: cfg.InfoCacheTTL}, powParamsCache: cachedData[*PoetPowParams]{ttl: cfg.PowParamsCacheTTL}, proofMembers: make(map[string][]types.Hash32, 1), expectedPhaseShift: cfg.PhaseShift, @@ -479,20 +466,15 @@ func NewPoetServiceWithClient( } func (c *poetService) verifyPhaseShiftConfiguration(ctx context.Context) error { - c.mtx.Lock() - defer c.mtx.Unlock() - - if c.fetchedPhaseShift != 0 { - return nil - } - resp, err := c.client.Info(ctx) + info, err := c.getInfo(ctx) if err != nil { return err - } else if resp.PhaseShift != c.expectedPhaseShift { + } + + if info.PhaseShift != c.expectedPhaseShift { return ErrIncompatiblePhaseShift } - c.fetchedPhaseShift = resp.PhaseShift return nil } @@ -553,8 +535,8 @@ func (c *poetService) reauthorize( challenge []byte, ) (*PoetAuth, error) { if c.certifier != nil { - if info, err := c.getCertifierInfo(ctx); err == nil { - if err := c.certifier.DeleteCertificate(id, info.Pubkey); err != nil { + if info, err := c.getInfo(ctx); err == nil { + if err := c.certifier.DeleteCertificate(id, info.Certifier.Pubkey); err != nil { return nil, fmt.Errorf("deleting cert: %w", err) } } @@ -640,22 +622,27 @@ func (c *poetService) Proof(ctx context.Context, roundID string) (*types.PoetPro func (c *poetService) Certify(ctx context.Context, id types.NodeID) (*certifier.PoetCert, error) { if c.certifier == nil { - return nil, errors.New("certifier not configured") + return nil, ErrCertifierNotConfigured } - info, err := c.getCertifierInfo(ctx) + + info, err := c.getInfo(ctx) if err != nil { return nil, err } - return c.certifier.Certificate(ctx, id, info.Url, info.Pubkey) + + if info.Certifier == nil { + return nil, ErrCertificatesNotSupported + } + return c.certifier.Certificate(ctx, id, info.Certifier.Url, info.Certifier.Pubkey) } -func (c *poetService) getCertifierInfo(ctx context.Context) (*types.CertifierInfo, error) { - info, err := c.certifierInfoCache.get(func() (*types.CertifierInfo, error) { - certifierInfo, err := c.client.CertifierInfo(ctx) +func (c *poetService) getInfo(ctx context.Context) (*types.PoetInfo, error) { + info, err := c.infoCache.get(func() (*types.PoetInfo, error) { + info, err := c.client.Info(ctx) if err != nil { - return nil, fmt.Errorf("getting certifier info: %w", err) + return nil, fmt.Errorf("getting poet info: %w", err) } - return certifierInfo, nil + return info, nil }) if err != nil { return nil, err diff --git a/activation/poet_client_test.go b/activation/poet_client_test.go index 2b106cbcb4..d721d83612 100644 --- a/activation/poet_client_test.go +++ b/activation/poet_client_test.go @@ -259,43 +259,79 @@ func TestPoetClient_QueryProofTimeout(t *testing.T) { } func TestPoetClient_Certify(t *testing.T) { + t.Parallel() + sig, err := signing.NewEdSigner() require.NoError(t, err) - certifierAddress := &url.URL{Scheme: "http", Host: "certifier"} - certifierPubKey := []byte("certifier-pubkey") - mux := http.NewServeMux() - infoResp, err := protojson.Marshal(&rpcapi.InfoResponse{ - ServicePubkey: []byte("pubkey"), - Certifier: &rpcapi.InfoResponse_Cerifier{ - Url: certifierAddress.String(), - Pubkey: certifierPubKey, - }, + cfg := PoetConfig{RequestTimeout: time.Millisecond * 100} + + t.Run("poet supports certificate", func(t *testing.T) { + certifierAddress := &url.URL{Scheme: "http", Host: "certifier"} + certifierPubKey := []byte("certifier-pubkey") + + infoResp, err := protojson.Marshal(&rpcapi.InfoResponse{ + ServicePubkey: []byte("pubkey"), + Certifier: &rpcapi.InfoResponse_Cerifier{ + Url: certifierAddress.String(), + Pubkey: certifierPubKey, + }, + }) + require.NoError(t, err) + + mux := http.NewServeMux() + mux.HandleFunc("GET /v1/info", func(w http.ResponseWriter, r *http.Request) { w.Write(infoResp) }) + ts := httptest.NewServer(mux) + defer ts.Close() + + server := types.PoetServer{ + Address: ts.URL, + Pubkey: types.NewBase64Enc([]byte("pubkey")), + } + + cert := certifier.PoetCert{Data: []byte("abc")} + + ctrl := gomock.NewController(t) + mCertifier := NewMockcertifierService(ctrl) + mCertifier.EXPECT(). + Certificate(gomock.Any(), sig.NodeID(), certifierAddress, certifierPubKey). + Return(&cert, nil) + + client, err := NewHTTPPoetClient(server, cfg, withCustomHttpClient(ts.Client())) + require.NoError(t, err) + poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t), WithCertifier(mCertifier)) + + got, err := poet.Certify(context.Background(), sig.NodeID()) + require.NoError(t, err) + require.Equal(t, cert, *got) }) - require.NoError(t, err) - mux.HandleFunc("GET /v1/info", func(w http.ResponseWriter, r *http.Request) { w.Write(infoResp) }) - ts := httptest.NewServer(mux) - defer ts.Close() - server := types.PoetServer{ - Address: ts.URL, - Pubkey: types.NewBase64Enc([]byte("pubkey")), - } - cfg := PoetConfig{RequestTimeout: time.Millisecond * 100} - cert := certifier.PoetCert{Data: []byte("abc")} - ctrl := gomock.NewController(t) - mCertifier := NewMockcertifierService(ctrl) - mCertifier.EXPECT(). - Certificate(gomock.Any(), sig.NodeID(), certifierAddress, certifierPubKey). - Return(&cert, nil) + t.Run("poet does not support certificate", func(t *testing.T) { + infoResp, err := protojson.Marshal(&rpcapi.InfoResponse{ + ServicePubkey: []byte("pubkey"), + }) + require.NoError(t, err) - client, err := NewHTTPPoetClient(server, cfg, withCustomHttpClient(ts.Client())) - require.NoError(t, err) - poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t), WithCertifier(mCertifier)) + mux := http.NewServeMux() + mux.HandleFunc("GET /v1/info", func(w http.ResponseWriter, r *http.Request) { w.Write(infoResp) }) + ts := httptest.NewServer(mux) + defer ts.Close() - got, err := poet.Certify(context.Background(), sig.NodeID()) - require.NoError(t, err) - require.Equal(t, cert, *got) + server := types.PoetServer{ + Address: ts.URL, + Pubkey: types.NewBase64Enc([]byte("pubkey")), + } + + ctrl := gomock.NewController(t) + mCertifier := NewMockcertifierService(ctrl) + + client, err := NewHTTPPoetClient(server, cfg, withCustomHttpClient(ts.Client())) + require.NoError(t, err) + + poet := NewPoetServiceWithClient(nil, client, cfg, zaptest.NewLogger(t), WithCertifier(mCertifier)) + _, err = poet.Certify(context.Background(), sig.NodeID()) + require.ErrorIs(t, err, ErrCertificatesNotSupported) + }) } func TestPoetClient_ObtainsCertOnSubmit(t *testing.T) { @@ -456,7 +492,7 @@ func TestPoetClient_FallbacksToPowWhenCannotRecertify(t *testing.T) { Address: ts.URL, Pubkey: types.NewBase64Enc([]byte("pubkey")), } - cfg := PoetConfig{CertifierInfoCacheTTL: time.Hour} + cfg := PoetConfig{InfoCacheTTL: time.Hour} ctrl := gomock.NewController(t) mCertifier := NewMockcertifierService(ctrl) @@ -491,28 +527,30 @@ func TestPoetService_CachesCertifierInfo(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Parallel() + cfg := DefaultPoetConfig() - cfg.CertifierInfoCacheTTL = tc.ttl - client := NewMockPoetClient(gomock.NewController(t)) + cfg.InfoCacheTTL = tc.ttl db := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t)) + url := &url.URL{Host: "certifier.hello"} + pubkey := []byte("pubkey") + poetInfoResp := &types.PoetInfo{Certifier: &types.CertifierInfo{Url: url, Pubkey: pubkey}} + + client := NewMockPoetClient(gomock.NewController(t)) client.EXPECT().Address().Return("some_addr").AnyTimes() - client.EXPECT().Info(gomock.Any()).Return(&types.PoetInfo{}, nil) + client.EXPECT().Info(gomock.Any()).Return(poetInfoResp, nil) poet := NewPoetServiceWithClient(db, client, cfg, zaptest.NewLogger(t)) - url := &url.URL{Host: "certifier.hello"} - pubkey := []byte("pubkey") - exp := client.EXPECT().CertifierInfo(gomock.Any()). - Return(&types.CertifierInfo{Url: url, Pubkey: pubkey}, nil) if tc.ttl == 0 { - exp.Times(5) + client.EXPECT().Info(gomock.Any()).Times(5).Return(poetInfoResp, nil) } + for range 5 { - info, err := poet.getCertifierInfo(context.Background()) + info, err := poet.getInfo(context.Background()) require.NoError(t, err) - require.Equal(t, url, info.Url) - require.Equal(t, pubkey, info.Pubkey) + require.Equal(t, url, info.Certifier.Url) + require.Equal(t, pubkey, info.Certifier.Pubkey) } }) } diff --git a/activation/poet_mocks.go b/activation/poet_mocks.go index 885105fef3..8681039090 100644 --- a/activation/poet_mocks.go +++ b/activation/poet_mocks.go @@ -79,45 +79,6 @@ func (c *MockPoetClientAddressCall) DoAndReturn(f func() string) *MockPoetClient return c } -// CertifierInfo mocks base method. -func (m *MockPoetClient) CertifierInfo(ctx context.Context) (*types.CertifierInfo, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CertifierInfo", ctx) - ret0, _ := ret[0].(*types.CertifierInfo) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// CertifierInfo indicates an expected call of CertifierInfo. -func (mr *MockPoetClientMockRecorder) CertifierInfo(ctx any) *MockPoetClientCertifierInfoCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CertifierInfo", reflect.TypeOf((*MockPoetClient)(nil).CertifierInfo), ctx) - return &MockPoetClientCertifierInfoCall{Call: call} -} - -// MockPoetClientCertifierInfoCall wrap *gomock.Call -type MockPoetClientCertifierInfoCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockPoetClientCertifierInfoCall) Return(arg0 *types.CertifierInfo, arg1 error) *MockPoetClientCertifierInfoCall { - c.Call = c.Call.Return(arg0, arg1) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockPoetClientCertifierInfoCall) Do(f func(context.Context) (*types.CertifierInfo, error)) *MockPoetClientCertifierInfoCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockPoetClientCertifierInfoCall) DoAndReturn(f func(context.Context) (*types.CertifierInfo, error)) *MockPoetClientCertifierInfoCall { - c.Call = c.Call.DoAndReturn(f) - return c -} - // Id mocks base method. func (m *MockPoetClient) Id() []byte { m.ctrl.T.Helper() diff --git a/config/mainnet.go b/config/mainnet.go index f970c851cc..3b1f8920f1 100644 --- a/config/mainnet.go +++ b/config/mainnet.go @@ -167,7 +167,7 @@ func MainnetConfig() Config { CycleGap: 12 * time.Hour, GracePeriod: 1 * time.Hour, PositioningATXSelectionTimeout: 50 * time.Minute, - CertifierInfoCacheTTL: 5 * time.Minute, + InfoCacheTTL: 5 * time.Minute, PowParamsCacheTTL: 5 * time.Minute, // RequestTimeout = RequestRetryDelay * 2 * MaxRequestRetries*(MaxRequestRetries+1)/2 RequestTimeout: 1100 * time.Second, diff --git a/config/presets/fastnet.go b/config/presets/fastnet.go index 2b9fe78417..6d0f7e57e2 100644 --- a/config/presets/fastnet.go +++ b/config/presets/fastnet.go @@ -101,7 +101,7 @@ func fastnet() config.Config { conf.POET.RequestTimeout = 12 * time.Second conf.POET.RequestRetryDelay = 1 * time.Second conf.POET.MaxRequestRetries = 3 - conf.POET.CertifierInfoCacheTTL = time.Minute + conf.POET.InfoCacheTTL = time.Minute conf.POET.PowParamsCacheTTL = 10 * time.Second return conf diff --git a/config/presets/standalone.go b/config/presets/standalone.go index 937f3c8f4c..b49b3860a8 100644 --- a/config/presets/standalone.go +++ b/config/presets/standalone.go @@ -85,7 +85,7 @@ func standalone() config.Config { conf.POET.RequestTimeout = 12 * time.Second conf.POET.RequestRetryDelay = 1 * time.Second conf.POET.MaxRequestRetries = 3 - conf.POET.CertifierInfoCacheTTL = time.Minute + conf.POET.InfoCacheTTL = time.Minute conf.POET.PowParamsCacheTTL = 10 * time.Second conf.P2P.DisableNatPort = true diff --git a/config/presets/testnet.go b/config/presets/testnet.go index d6d80530af..af070e1068 100644 --- a/config/presets/testnet.go +++ b/config/presets/testnet.go @@ -124,8 +124,8 @@ func testnet() config.Config { RequestRetryDelay: 5 * time.Second, MaxRequestRetries: 10, - CertifierInfoCacheTTL: 5 * time.Minute, - PowParamsCacheTTL: 5 * time.Minute, + InfoCacheTTL: 5 * time.Minute, + PowParamsCacheTTL: 5 * time.Minute, }, POST: activation.PostConfig{ MinNumUnits: 2,