From b8ddb2e42c81394069196d1caae6c8c8a931f722 Mon Sep 17 00:00:00 2001 From: George Date: Mon, 19 Aug 2024 13:46:28 -0700 Subject: [PATCH] Add basic test on POST, JSON, error checks: It'd be nice if we had POST param unit tests but we don't :( --- clients/stellarcore/client.go | 60 +++++++++++++++++-------- clients/stellarcore/client_test.go | 41 +++++++++++++++++ services/horizon/internal/ingest/fsm.go | 5 --- 3 files changed, 82 insertions(+), 24 deletions(-) diff --git a/clients/stellarcore/client.go b/clients/stellarcore/client.go index fafc3f4f21..ca3d098521 100644 --- a/clients/stellarcore/client.go +++ b/clients/stellarcore/client.go @@ -1,11 +1,11 @@ package stellarcore import ( + "bytes" "context" "encoding/json" "fmt" "io" - "io/ioutil" "net/http" "net/url" "path" @@ -21,7 +21,7 @@ import ( // Client represents a client that is capable of communicating with a // stellar-core server using HTTP type Client struct { - // HTTP is the client to use when communicating with stellar-core. If nil, + // HTTP is the client to use when communicating with stellar-core. If nil, // http.DefaultClient will be used. HTTP HTTP @@ -36,7 +36,7 @@ type Client struct { // in case an error was encountered during either the draining or closing of the // stream, that error would be returned. func drainReponse(hresp *http.Response, close bool, err *error) (outerror error) { - _, err2 := io.Copy(ioutil.Discard, hresp.Body) + _, err2 := io.Copy(io.Discard, hresp.Body) if err2 != nil { if err != nil && *err == nil { *err = errors.Wrap(err2, "unable to read excess data from response") @@ -152,9 +152,9 @@ func (c *Client) SetCursor(ctx context.Context, id string, cursor int32) (err er return nil } -func (c *Client) GetLedgerEntries(ctx context.Context, ledgerSeq uint32, keys ...xdr.LedgerKey) (*proto.GetLedgerEntriesResponse, error) { - var resp *proto.GetLedgerEntriesResponse - return resp, c.makeLedgerKeyRequest(ctx, resp, "getledgerentries", ledgerSeq, keys...) +func (c *Client) GetLedgerEntries(ctx context.Context, ledgerSeq uint32, keys ...xdr.LedgerKey) (proto.GetLedgerEntriesResponse, error) { + var resp proto.GetLedgerEntriesResponse + return resp, c.makeLedgerKeyRequest(ctx, &resp, "getledgerentry", ledgerSeq, keys...) } //lint:ignore U1000 Ignore unused function until it's supported in Core @@ -270,19 +270,33 @@ func (c *Client) simpleGet( newPath string, query url.Values, ) (*http.Request, error) { - q := "" + u, err := url.Parse(c.URL) + if err != nil { + return nil, errors.Wrap(err, "unparseable url") + } + + u.Path = path.Join(u.Path, newPath) if query != nil { - q = query.Encode() + u.RawQuery = query.Encode() } - return c.rawGet(ctx, newPath, q) + newURL := u.String() + + var req *http.Request + req, err = http.NewRequestWithContext(ctx, http.MethodGet, newURL, nil) + if err != nil { + return nil, errors.Wrap(err, "failed to create request") + } + + return req, nil } -// rawGet returns a new GET request to the connected stellar-core using the -// provided path and a raw query string to construct the result. -func (c *Client) rawGet( +// rawPost returns a new POST request to the connected stellar-core using the +// provided path and the params values encoded as the request body to construct +// the result. +func (c *Client) rawPost( ctx context.Context, newPath string, - query string, + params string, ) (*http.Request, error) { u, err := url.Parse(c.URL) if err != nil { @@ -290,11 +304,14 @@ func (c *Client) rawGet( } u.Path = path.Join(u.Path, newPath) - u.RawQuery = query newURL := u.String() var req *http.Request - req, err = http.NewRequestWithContext(ctx, http.MethodGet, newURL, nil) + req, err = http.NewRequestWithContext( + ctx, + http.MethodPost, + newURL, + bytes.NewBuffer([]byte(params))) if err != nil { return nil, errors.Wrap(err, "failed to create request") } @@ -304,7 +321,7 @@ func (c *Client) rawGet( // makeLedgerKeyRequest is a generic method to perform a request in the form // `key=...&key=...&ledgerSeq=...` which is useful because three Stellar Core -// endpoints all use this request format. +// endpoints all use this request format. Be sure to pass `target` by reference. func (c *Client) makeLedgerKeyRequest( ctx context.Context, target interface{}, @@ -319,7 +336,7 @@ func (c *Client) makeLedgerKeyRequest( } var req *http.Request - req, err = c.rawGet(ctx, endpoint, q) + req, err = c.rawPost(ctx, endpoint, q) if err != nil { return err } @@ -329,7 +346,7 @@ func (c *Client) makeLedgerKeyRequest( return err } - // returns nil if the error is nil + // wrap returns nil if the inner error is nil return errors.Wrap(json.NewDecoder(hresp.Body).Decode(&target), "json decode failed") } @@ -356,10 +373,15 @@ func (c *Client) getResponse(req *http.Request) (*http.Response, error) { func buildMultiKeyRequest(keys ...xdr.LedgerKey) (string, error) { // The average ledger key length, according to a simple // - // SELECT AVG(LENGTH(HEX(key))) / 2 FROM ledger_entries; + // SELECT AVG(LENGTH(HEX(key))) / 2 FROM ledger_entries; // // on a pubnet RPC instance is ~57.6. We can use this to preallocate a // string buffer for performance. + // + // We know that these endpoints will almost exclusively be used for + // ContractData and the like, so we could optimize the buffer further for + // that, but that data is harder to query since it'd involve parsing the XDR + // from the DB to check the key type. q := strings.Builder{} q.Grow(50 * len(keys)) diff --git a/clients/stellarcore/client_test.go b/clients/stellarcore/client_test.go index 6cfd01b210..aa068610b1 100644 --- a/clients/stellarcore/client_test.go +++ b/clients/stellarcore/client_test.go @@ -6,9 +6,12 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stellar/go/keypair" proto "github.com/stellar/go/protocols/stellarcore" "github.com/stellar/go/support/http/httptest" + "github.com/stellar/go/xdr" ) func TestSubmitTransaction(t *testing.T) { @@ -75,3 +78,41 @@ func TestManualClose_NotAvailable(t *testing.T) { assert.EqualError(t, err, "exception in response: Set MANUAL_CLOSE=true") } + +func TestGetLedgerEntries(t *testing.T) { + hmock := httptest.NewClient() + c := &Client{HTTP: hmock, URL: "http://localhost:11626"} + + // build a fake response body + mockResp := proto.GetLedgerEntriesResponse{ + Ledger: 1215, // checkpoint align on expected request + Entries: []proto.LedgerEntryResponse{ + { + Entry: "pretend this is XDR lol", + State: proto.DeadState, + }, + { + Entry: "pretend this is another XDR lol", + State: proto.ArchivedStateNoProof, + }, + }, + } + + // happy path - fetch an entry + hmock.On("POST", "http://localhost:11626/getledgerentry"). + ReturnJSON(http.StatusOK, &mockResp) + + var key xdr.LedgerKey + acc, err := xdr.AddressToAccountId(keypair.MustRandom().Address()) + require.NoError(t, err) + key.SetAccount(acc) + + resp, err := c.GetLedgerEntries(context.Background(), 1234, key) + require.NoError(t, err) + require.NotNil(t, resp) + + require.EqualValues(t, 1215, resp.Ledger) + require.Len(t, resp.Entries, 2) + require.Equal(t, resp.Entries[0].State, proto.DeadState) + require.Equal(t, resp.Entries[1].State, proto.ArchivedStateNoProof) +} diff --git a/services/horizon/internal/ingest/fsm.go b/services/horizon/internal/ingest/fsm.go index 8220942e73..c165faf371 100644 --- a/services/horizon/internal/ingest/fsm.go +++ b/services/horizon/internal/ingest/fsm.go @@ -765,11 +765,6 @@ func (v verifyRangeState) run(s *system) (transition, error) { "ledger": true, "commit": true, }).Info("Processing ledger") - - if sequence == 52885867 || sequence == 52885868 { - log.Error("We are in the problem zone.") - } - startTime := time.Now() if err = s.historyQ.Begin(s.ctx); err != nil {