diff --git a/services/horizon/internal/actions/effects.go b/services/horizon/internal/actions/effects.go index e04997aca5..d8620fc11f 100644 --- a/services/horizon/internal/actions/effects.go +++ b/services/horizon/internal/actions/effects.go @@ -51,12 +51,12 @@ type GetEffectsHandler struct { } func (handler GetEffectsHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } diff --git a/services/horizon/internal/actions/helpers.go b/services/horizon/internal/actions/helpers.go index a2ce9675eb..8db4674991 100644 --- a/services/horizon/internal/actions/helpers.go +++ b/services/horizon/internal/actions/helpers.go @@ -45,8 +45,6 @@ type Opt int const ( // DisableCursorValidation disables cursor validation in GetPageQuery DisableCursorValidation Opt = iota - // DefaultTOID sets a default cursor value in GetPageQuery based on the ledger state - DefaultTOID Opt = iota ) // HeaderWriter is an interface for setting HTTP response headers @@ -185,14 +183,10 @@ func getLimit(r *http.Request, name string, def uint64, max uint64) (uint64, err // using the results from a call to GetPagingParams() func GetPageQuery(ledgerState *ledger.State, r *http.Request, opts ...Opt) (db2.PageQuery, error) { disableCursorValidation := false - defaultTOID := false for _, opt := range opts { if opt == DisableCursorValidation { disableCursorValidation = true } - if opt == DefaultTOID { - defaultTOID = true - } } cursor, err := getCursor(ledgerState, r, ParamCursor) @@ -222,13 +216,6 @@ func GetPageQuery(ledgerState *ledger.State, r *http.Request, opts ...Opt) (db2. return db2.PageQuery{}, err } - if defaultTOID && pageQuery.Order == db2.OrderAscending { - if cursor == "" || errors.Is(validateCursor(ledgerState, pageQuery), &hProblem.BeforeHistory) { - pageQuery.Cursor = toid.AfterLedger( - ordered.Max(0, ledgerState.CurrentStatus().HistoryElder-1), - ).String() - } - } return pageQuery, nil } @@ -553,23 +540,37 @@ func validateAssetParams(aType, code, issuer, prefix string) error { return nil } -// validateCursorWithinHistory compares the requested page of data against the +// validateAndAdjustCursor compares the requested page of data against the // ledger state of the history database. In the event that the cursor is // guaranteed to return no results, we return a 410 GONE http response. -func validateCursorWithinHistory(ledgerState *ledger.State, pq db2.PageQuery) error { - // an ascending query should never return a gone response: An ascending query - // prior to known history should return results at the beginning of history, - // and an ascending query beyond the end of history should not error out but - // rather return an empty page (allowing code that tracks the procession of - // some resource more easily). - if pq.Order != "desc" { - return nil +// For ascending queries, we adjust the cursor to ensure it starts at +// the oldest available ledger. +func validateAndAdjustCursor(ledgerState *ledger.State, pq *db2.PageQuery) error { + + if pq.Order == db2.OrderDescending { + return validateCursorWithinHistory(ledgerState, *pq) + } else if pq.Order == db2.OrderAscending { + // an ascending query should never return a gone response: An ascending query + // prior to known history should return results at the beginning of history, + // and an ascending query beyond the end of history should not error out but + // rather return an empty page (allowing code that tracks the procession of + // some resource more easily). + + // set/modify the cursor for ascending queries to start at the oldest available ledger if it + // precedes the oldest ledger. This avoids inefficient queries caused by index bloat from deleted rows + // that are removed as part of reaping to maintain the retention window. + if pq.Cursor == "" || errors.Is(validateCursorWithinHistory(ledgerState, *pq), &hProblem.BeforeHistory) { + pq.Cursor = toid.AfterLedger( + ordered.Max(0, ledgerState.CurrentStatus().HistoryElder-1), + ).String() + } } - - return validateCursor(ledgerState, pq) + return nil } -func validateCursor(ledgerState *ledger.State, pq db2.PageQuery) error { +// validateCursorWithinHistory checks if the cursor is within the known history range. +// If the cursor is before the oldest available ledger, it returns BeforeHistory error. +func validateCursorWithinHistory(ledgerState *ledger.State, pq db2.PageQuery) error { var cursor int64 var err error diff --git a/services/horizon/internal/actions/helpers_test.go b/services/horizon/internal/actions/helpers_test.go index b18a6b4b1f..97a5d92c8d 100644 --- a/services/horizon/internal/actions/helpers_test.go +++ b/services/horizon/internal/actions/helpers_test.go @@ -16,6 +16,7 @@ import ( horizonContext "github.com/stellar/go/services/horizon/internal/context" "github.com/stellar/go/services/horizon/internal/db2" "github.com/stellar/go/services/horizon/internal/ledger" + hProblem "github.com/stellar/go/services/horizon/internal/render/problem" "github.com/stellar/go/services/horizon/internal/test" "github.com/stellar/go/support/db" "github.com/stellar/go/support/errors" @@ -126,11 +127,21 @@ func TestValidateCursorWithinHistory(t *testing.T) { { cursor: "0", order: "asc", - valid: true, + valid: false, }, { cursor: "0-1234", order: "asc", + valid: false, + }, + { + cursor: "1", + order: "asc", + valid: true, + }, + { + cursor: "1-1234", + order: "asc", valid: true, }, } @@ -291,11 +302,10 @@ func TestGetPageQuery(t *testing.T) { tt.Assert.Error(err) } -func TestGetPageQueryCursorDefaultTOID(t *testing.T) { - ascReq := makeTestActionRequest("/foo-bar/blah?limit=2", testURLParams()) - descReq := makeTestActionRequest("/foo-bar/blah?limit=2&order=desc", testURLParams()) - +func TestPageQueryCursorDefaultOrder(t *testing.T) { ledgerState := &ledger.State{} + + // truncated history ledgerState.SetHorizonStatus(ledger.HorizonStatus{ HistoryLatest: 7000, HistoryLatestClosedAt: time.Now(), @@ -303,30 +313,30 @@ func TestGetPageQueryCursorDefaultTOID(t *testing.T) { ExpHistoryLatest: 7000, }) - pq, err := GetPageQuery(ledgerState, ascReq, DefaultTOID) + req := makeTestActionRequest("/foo-bar/blah?limit=2", testURLParams()) + + // default asc, w/o cursor + pq, err := GetPageQuery(ledgerState, req) assert.NoError(t, err) + assert.Empty(t, pq.Cursor) + assert.Error(t, &hProblem.BeforeHistory, validateAndAdjustCursor(ledgerState, &pq)) assert.Equal(t, toid.AfterLedger(299).String(), pq.Cursor) assert.Equal(t, uint64(2), pq.Limit) assert.Equal(t, "asc", pq.Order) - pq, err = GetPageQuery(ledgerState, descReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, "", pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) + cursor := toid.AfterLedger(200).String() + reqWithCursor := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", cursor), testURLParams()) - pq, err = GetPageQuery(ledgerState, ascReq) + // default asc, w/ cursor + pq, err = GetPageQuery(ledgerState, reqWithCursor) assert.NoError(t, err) - assert.Empty(t, pq.Cursor) + assert.Equal(t, cursor, pq.Cursor) + assert.Error(t, &hProblem.BeforeHistory, validateAndAdjustCursor(ledgerState, &pq)) + assert.Equal(t, toid.AfterLedger(299).String(), pq.Cursor) assert.Equal(t, uint64(2), pq.Limit) assert.Equal(t, "asc", pq.Order) - pq, err = GetPageQuery(ledgerState, descReq) - assert.NoError(t, err) - assert.Empty(t, pq.Cursor) - assert.Equal(t, "", pq.Cursor) - assert.Equal(t, "desc", pq.Order) - + // full history ledgerState.SetHorizonStatus(ledger.HorizonStatus{ HistoryLatest: 7000, HistoryLatestClosedAt: time.Now(), @@ -334,128 +344,108 @@ func TestGetPageQueryCursorDefaultTOID(t *testing.T) { ExpHistoryLatest: 7000, }) - pq, err = GetPageQuery(ledgerState, ascReq, DefaultTOID) + // default asc, w/o cursor + pq, err = GetPageQuery(ledgerState, req) assert.NoError(t, err) + assert.Empty(t, pq.Cursor) + assert.Error(t, &hProblem.BeforeHistory, validateAndAdjustCursor(ledgerState, &pq)) assert.Equal(t, toid.AfterLedger(0).String(), pq.Cursor) assert.Equal(t, uint64(2), pq.Limit) assert.Equal(t, "asc", pq.Order) - pq, err = GetPageQuery(ledgerState, descReq, DefaultTOID) + // default asc, w/ cursor + pq, err = GetPageQuery(ledgerState, reqWithCursor) assert.NoError(t, err) - assert.Equal(t, "", pq.Cursor) + assert.Equal(t, cursor, pq.Cursor) + assert.Error(t, &hProblem.BeforeHistory, validateAndAdjustCursor(ledgerState, &pq)) + assert.Equal(t, cursor, pq.Cursor) assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) + assert.Equal(t, "asc", pq.Order) + } -func TestPageQueryCursorFullHistory(t *testing.T) { +func TestGetPageQueryWithoutCursor(t *testing.T) { ledgerState := &ledger.State{} + + validateCursor := func(limit uint64, order string, expectedCursor string) { + req := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?limit=%d&order=%s", limit, order), testURLParams()) + pq, err := GetPageQuery(ledgerState, req) + assert.NoError(t, err) + assert.Empty(t, pq.Cursor) + assert.Error(t, &hProblem.BeforeHistory, validateAndAdjustCursor(ledgerState, &pq)) + assert.Equal(t, expectedCursor, pq.Cursor) + assert.Equal(t, limit, pq.Limit) + assert.Equal(t, order, pq.Order) + } + + // truncated history ledgerState.SetHorizonStatus(ledger.HorizonStatus{ HistoryLatest: 7000, HistoryLatestClosedAt: time.Now(), - HistoryElder: 0, + HistoryElder: 300, ExpHistoryLatest: 7000, }) - ascReq := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", toid.AfterLedger(200).String()), testURLParams()) - pq, err := GetPageQuery(ledgerState, ascReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(200).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "asc", pq.Order) - - ascReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", toid.AfterLedger(200).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, ascReq) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(200).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "asc", pq.Order) - - descReq := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2&order=desc", toid.AfterLedger(200).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, descReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(200).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) + validateCursor(2, "asc", toid.AfterLedger(299).String()) + validateCursor(2, "desc", "") - descReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2&order=desc", toid.AfterLedger(200).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, descReq) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(200).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) -} - -func TestPageQueryCursorLimitedHistory(t *testing.T) { - ledgerState := &ledger.State{} + // full history ledgerState.SetHorizonStatus(ledger.HorizonStatus{ HistoryLatest: 7000, HistoryLatestClosedAt: time.Now(), - HistoryElder: 300, + HistoryElder: 0, ExpHistoryLatest: 7000, }) - ascReq := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", toid.AfterLedger(200).String()), testURLParams()) - pq, err := GetPageQuery(ledgerState, ascReq) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(200).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "asc", pq.Order) + validateCursor(2, "asc", toid.AfterLedger(0).String()) + validateCursor(2, "desc", "") +} - ascReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", toid.AfterLedger(200).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, ascReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(299).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "asc", pq.Order) +func TestPageQueryWithCursor(t *testing.T) { + ledgerState := &ledger.State{} - ascReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", toid.AfterLedger(298).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, ascReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(299).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "asc", pq.Order) + validateCursor := func(cursor string, limit uint64, order string, expectedCursor string) { + req := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=%d&order=%s", cursor, limit, order), testURLParams()) + pq, err := GetPageQuery(ledgerState, req) + assert.NoError(t, err) + assert.Equal(t, cursor, pq.Cursor) + assert.Error(t, &hProblem.BeforeHistory, validateAndAdjustCursor(ledgerState, &pq)) + assert.Equal(t, expectedCursor, pq.Cursor) + assert.Equal(t, limit, pq.Limit) + assert.Equal(t, order, pq.Order) + } - ascReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", toid.AfterLedger(299).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, ascReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(299).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "asc", pq.Order) + // full history + ledgerState.SetHorizonStatus(ledger.HorizonStatus{ + HistoryLatest: 7000, + HistoryLatestClosedAt: time.Now(), + HistoryElder: 0, + ExpHistoryLatest: 7000, + }) - ascReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", toid.AfterLedger(300).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, ascReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(300).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "asc", pq.Order) + validateCursor(toid.AfterLedger(200).String(), 2, "asc", toid.AfterLedger(200).String()) + validateCursor(toid.AfterLedger(200).String(), 2, "desc", toid.AfterLedger(200).String()) - ascReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2", toid.AfterLedger(301).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, ascReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(301).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "asc", pq.Order) + // truncated history + ledgerState.SetHorizonStatus(ledger.HorizonStatus{ + HistoryLatest: 7000, + HistoryLatestClosedAt: time.Now(), + HistoryElder: 300, + ExpHistoryLatest: 7000, + }) - descReq := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2&order=desc", toid.AfterLedger(298).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, descReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(298).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) + // asc order + validateCursor(toid.AfterLedger(200).String(), 2, "asc", toid.AfterLedger(299).String()) + validateCursor(toid.AfterLedger(298).String(), 2, "asc", toid.AfterLedger(299).String()) + validateCursor(toid.AfterLedger(299).String(), 2, "asc", toid.AfterLedger(299).String()) + validateCursor(toid.AfterLedger(300).String(), 2, "asc", toid.AfterLedger(300).String()) + validateCursor(toid.AfterLedger(301).String(), 2, "asc", toid.AfterLedger(301).String()) - descReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2&order=desc", toid.AfterLedger(320).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, descReq, DefaultTOID) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(320).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) + // desc order + validateCursor(toid.AfterLedger(298).String(), 2, "desc", toid.AfterLedger(298).String()) + validateCursor(toid.AfterLedger(300).String(), 2, "desc", toid.AfterLedger(300).String()) + validateCursor(toid.AfterLedger(320).String(), 2, "desc", toid.AfterLedger(320).String()) - descReq = makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=2&order=desc", toid.AfterLedger(298).String()), testURLParams()) - pq, err = GetPageQuery(ledgerState, descReq) - assert.NoError(t, err) - assert.Equal(t, toid.AfterLedger(298).String(), pq.Cursor) - assert.Equal(t, uint64(2), pq.Limit) - assert.Equal(t, "desc", pq.Order) } func TestGetString(t *testing.T) { diff --git a/services/horizon/internal/actions/ledger.go b/services/horizon/internal/actions/ledger.go index 0b3d51b8e1..0836ba9b10 100644 --- a/services/horizon/internal/actions/ledger.go +++ b/services/horizon/internal/actions/ledger.go @@ -17,12 +17,12 @@ type GetLedgersHandler struct { } func (handler GetLedgersHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } diff --git a/services/horizon/internal/actions/operation.go b/services/horizon/internal/actions/operation.go index 9ccadb272e..6b200cba29 100644 --- a/services/horizon/internal/actions/operation.go +++ b/services/horizon/internal/actions/operation.go @@ -72,12 +72,12 @@ type GetOperationsHandler struct { func (handler GetOperationsHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { ctx := r.Context() - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } diff --git a/services/horizon/internal/actions/trade.go b/services/horizon/internal/actions/trade.go index b29ad64715..1e81dbb7a9 100644 --- a/services/horizon/internal/actions/trade.go +++ b/services/horizon/internal/actions/trade.go @@ -159,12 +159,12 @@ type GetTradesHandler struct { func (handler GetTradesHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { ctx := r.Context() - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } @@ -287,7 +287,7 @@ func (handler GetTradeAggregationsHandler) GetResource(w HeaderWriter, r *http.R if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err } diff --git a/services/horizon/internal/actions/transaction.go b/services/horizon/internal/actions/transaction.go index 6a2fd1c6e2..ed579f9d6f 100644 --- a/services/horizon/internal/actions/transaction.go +++ b/services/horizon/internal/actions/transaction.go @@ -98,12 +98,12 @@ type GetTransactionsHandler struct { func (handler GetTransactionsHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) { ctx := r.Context() - pq, err := GetPageQuery(handler.LedgerState, r, DefaultTOID) + pq, err := GetPageQuery(handler.LedgerState, r) if err != nil { return nil, err } - err = validateCursorWithinHistory(handler.LedgerState, pq) + err = validateAndAdjustCursor(handler.LedgerState, &pq) if err != nil { return nil, err }