Skip to content

Commit

Permalink
services/horizon: Remove global ledger state (#3216)
Browse files Browse the repository at this point in the history
horizon/ledger.State is a global variable. This is a bad pattern and caused some issues (see #3143). We should make the state local to horizon.App and pass a pointer to it to other parts of the system that need it.
  • Loading branch information
tamirms authored Nov 12, 2020
1 parent f80da5f commit 5b28203
Show file tree
Hide file tree
Showing 43 changed files with 293 additions and 200 deletions.
7 changes: 5 additions & 2 deletions services/horizon/internal/actions/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
protocol "github.com/stellar/go/protocols/horizon"
horizonContext "github.com/stellar/go/services/horizon/internal/context"
"github.com/stellar/go/services/horizon/internal/db2/history"
"github.com/stellar/go/services/horizon/internal/ledger"
"github.com/stellar/go/services/horizon/internal/resourceadapter"
"github.com/stellar/go/support/errors"
"github.com/stellar/go/support/render/hal"
Expand Down Expand Up @@ -118,7 +119,9 @@ func (q AccountsQuery) Asset() *xdr.Asset {
}

// GetAccountsHandler is the action handler for the /accounts endpoint
type GetAccountsHandler struct{}
type GetAccountsHandler struct {
LedgerState *ledger.State
}

// GetResourcePage returns a page containing the account records that have
// `signer` as a signer or have a trustline to the given asset.
Expand All @@ -127,7 +130,7 @@ func (handler GetAccountsHandler) GetResourcePage(
r *http.Request,
) ([]hal.Pageable, error) {
ctx := r.Context()
pq, err := GetPageQuery(r, DisableCursorValidation)
pq, err := GetPageQuery(handler.LedgerState, r, DisableCursorValidation)
if err != nil {
return nil, err
}
Expand Down
4 changes: 3 additions & 1 deletion services/horizon/internal/actions/asset.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stellar/go/services/horizon/internal/context"
"github.com/stellar/go/services/horizon/internal/db2"
"github.com/stellar/go/services/horizon/internal/db2/history"
"github.com/stellar/go/services/horizon/internal/ledger"
"github.com/stellar/go/services/horizon/internal/resourceadapter"
"github.com/stellar/go/support/errors"
"github.com/stellar/go/support/render/hal"
Expand All @@ -18,6 +19,7 @@ import (

// AssetStatsHandler is the action handler for the /asset endpoint
type AssetStatsHandler struct {
LedgerState *ledger.State
}

func (handler AssetStatsHandler) validateAssetParams(code, issuer string, pq db2.PageQuery) error {
Expand Down Expand Up @@ -123,7 +125,7 @@ func (handler AssetStatsHandler) GetResourcePage(
return nil, err
}

pq, err := GetPageQuery(r, DisableCursorValidation)
pq, err := GetPageQuery(handler.LedgerState, r, DisableCursorValidation)
if err != nil {
return nil, err
}
Expand Down
4 changes: 3 additions & 1 deletion services/horizon/internal/actions/claimable_balance.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
protocol "github.com/stellar/go/protocols/horizon"
horizonContext "github.com/stellar/go/services/horizon/internal/context"
"github.com/stellar/go/services/horizon/internal/db2/history"
"github.com/stellar/go/services/horizon/internal/ledger"
"github.com/stellar/go/services/horizon/internal/resourceadapter"
"github.com/stellar/go/support/errors"
"github.com/stellar/go/support/render/hal"
Expand Down Expand Up @@ -129,6 +130,7 @@ func (q ClaimableBalancesQuery) URITemplate() string {
}

type GetClaimableBalancesHandler struct {
LedgerState *ledger.State
}

// GetResourcePage returns a page of claimable balances.
Expand All @@ -143,7 +145,7 @@ func (handler GetClaimableBalancesHandler) GetResourcePage(
return nil, err
}

pq, err := GetPageQuery(r, DisableCursorValidation)
pq, err := GetPageQuery(handler.LedgerState, r, DisableCursorValidation)
if err != nil {
return nil, err
}
Expand Down
9 changes: 6 additions & 3 deletions services/horizon/internal/actions/effects.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/stellar/go/services/horizon/internal/context"
"github.com/stellar/go/services/horizon/internal/db2"
"github.com/stellar/go/services/horizon/internal/db2/history"
"github.com/stellar/go/services/horizon/internal/ledger"
"github.com/stellar/go/services/horizon/internal/resourceadapter"
"github.com/stellar/go/support/errors"
"github.com/stellar/go/support/render/hal"
Expand Down Expand Up @@ -42,15 +43,17 @@ func (qp EffectsQuery) Validate() error {
return nil
}

type GetEffectsHandler struct{}
type GetEffectsHandler struct {
LedgerState *ledger.State
}

func (handler GetEffectsHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) {
pq, err := GetPageQuery(r)
pq, err := GetPageQuery(handler.LedgerState, r)
if err != nil {
return nil, err
}

err = validateCursorWithinHistory(pq)
err = validateCursorWithinHistory(handler.LedgerState, pq)
if err != nil {
return nil, err
}
Expand Down
12 changes: 6 additions & 6 deletions services/horizon/internal/actions/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ func SetLastLedgerHeader(w HeaderWriter, lastLedger uint32) {

// getCursor retrieves a string from either the URLParams, form or query string.
// This method uses the priority (URLParams, Form, Query).
func getCursor(r *http.Request, name string) (string, error) {
func getCursor(ledgerState *ledger.State, r *http.Request, name string) (string, error) {
cursor, err := getString(r, name)

if err != nil {
return "", err
}

if cursor == "now" {
tid := toid.AfterLedger(ledger.CurrentState().HistoryLatest)
tid := toid.AfterLedger(ledgerState.CurrentStatus().HistoryLatest)
cursor = tid.String()
}

Expand Down Expand Up @@ -180,15 +180,15 @@ func getLimit(r *http.Request, name string, def uint64, max uint64) (uint64, err

// GetPageQuery is a helper that returns a new db.PageQuery struct initialized
// using the results from a call to GetPagingParams()
func GetPageQuery(r *http.Request, opts ...Opt) (db2.PageQuery, error) {
func GetPageQuery(ledgerState *ledger.State, r *http.Request, opts ...Opt) (db2.PageQuery, error) {
disableCursorValidation := false
for _, opt := range opts {
if opt == DisableCursorValidation {
disableCursorValidation = true
}
}

cursor, err := getCursor(r, ParamCursor)
cursor, err := getCursor(ledgerState, r, ParamCursor)
if err != nil {
return db2.PageQuery{}, err
}
Expand Down Expand Up @@ -535,7 +535,7 @@ func validateAssetParams(aType, code, issuer, prefix string) error {
// validateCursorWithinHistory 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(pq db2.PageQuery) error {
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
Expand All @@ -560,7 +560,7 @@ func validateCursorWithinHistory(pq db2.PageQuery) error {
return problem.MakeInvalidFieldProblem("cursor", errors.New("invalid value"))
}

elder := toid.New(ledger.CurrentState().HistoryElder, 0, 0)
elder := toid.New(ledgerState.CurrentStatus().HistoryElder, 0, 0)

if cursor <= elder.ToInt64() {
return &hProblem.BeforeHistory
Expand Down
23 changes: 13 additions & 10 deletions services/horizon/internal/actions/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,19 @@ func TestGetCursor(t *testing.T) {
tt := test.Start(t)
defer tt.Finish()

ledgerState := &ledger.State{}
// now uses the ledger state
r := makeTestActionRequest("/?cursor=now", nil)
cursor, err := getCursor(r, "cursor")
cursor, err := getCursor(ledgerState, r, "cursor")
if tt.Assert.NoError(err) {
expected := toid.AfterLedger(ledger.CurrentState().HistoryLatest).String()
expected := toid.AfterLedger(ledgerState.CurrentStatus().HistoryLatest).String()
tt.Assert.Equal(expected, cursor)
}

//Last-Event-ID overrides cursor
r = makeFooBarTestActionRequest()
r.Header.Set("Last-Event-ID", "from_header")
cursor, err = getCursor(r, "cursor")
cursor, err = getCursor(ledgerState, r, "cursor")
if tt.Assert.NoError(err) {
tt.Assert.Equal("from_header", cursor)
}
Expand Down Expand Up @@ -135,7 +136,7 @@ func TestValidateCursorWithinHistory(t *testing.T) {
t.Run(fmt.Sprintf("cursor: %s", tc.cursor), func(t *testing.T) {
pq, err := db2.NewPageQuery(tc.cursor, false, tc.order, 10)
tt.NoError(err)
err = validateCursorWithinHistory(pq)
err = validateCursorWithinHistory(&ledger.State{}, pq)

if tc.valid {
tt.NoError(err)
Expand Down Expand Up @@ -238,9 +239,10 @@ func TestActionGetPageQuery(t *testing.T) {
tt := test.Start(t)
defer tt.Finish()
r := makeFooBarTestActionRequest()
ledgerState := &ledger.State{}

// happy path
pq, err := GetPageQuery(r)
pq, err := GetPageQuery(ledgerState, r)
tt.Assert.NoError(err)
tt.Assert.Equal("123456", pq.Cursor)
tt.Assert.Equal(uint64(2), pq.Limit)
Expand All @@ -250,23 +252,24 @@ func TestActionGetPageQuery(t *testing.T) {
r = makeTestActionRequest("/?limit=foo", nil)
_, err = getLimit(r, "limit", 1, 200)
tt.Assert.Error(err)
_, err = GetPageQuery(r)
_, err = GetPageQuery(ledgerState, r)
tt.Assert.Error(err)

// regression: https://github.com/stellar/go/services/horizon/internal/issues/372
// (limit of 0 turns into 10)
makeTestActionRequest("/?limit=0", nil)
_, err = GetPageQuery(r)
_, err = GetPageQuery(ledgerState, r)
tt.Assert.Error(err)
}

func TestGetPageQuery(t *testing.T) {
tt := test.Start(t)
defer tt.Finish()
r := makeFooBarTestActionRequest()
ledgerState := &ledger.State{}

// happy path
pq, err := GetPageQuery(r)
pq, err := GetPageQuery(ledgerState, r)
tt.Assert.NoError(err)
tt.Assert.Equal("123456", pq.Cursor)
tt.Assert.Equal(uint64(2), pq.Limit)
Expand All @@ -276,13 +279,13 @@ func TestGetPageQuery(t *testing.T) {
r = makeTestActionRequest("/?limit=foo", nil)
_, err = getLimit(r, "limit", 1, 200)
tt.Assert.Error(err)
_, err = GetPageQuery(r)
_, err = GetPageQuery(ledgerState, r)
tt.Assert.Error(err)

// regression: https://github.com/stellar/go/services/horizon/internal/issues/372
// (limit of 0 turns into 10)
r = makeTestActionRequest("/?limit=0", nil)
_, err = GetPageQuery(r)
_, err = GetPageQuery(ledgerState, r)
tt.Assert.Error(err)
}

Expand Down
14 changes: 9 additions & 5 deletions services/horizon/internal/actions/ledger.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ import (
"github.com/stellar/go/support/render/hal"
)

type GetLedgersHandler struct{}
type GetLedgersHandler struct {
LedgerState *ledger.State
}

func (handler GetLedgersHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) {
pq, err := GetPageQuery(r)
pq, err := GetPageQuery(handler.LedgerState, r)
if err != nil {
return nil, err
}

err = validateCursorWithinHistory(pq)
err = validateCursorWithinHistory(handler.LedgerState, pq)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -53,15 +55,17 @@ type LedgerByIDQuery struct {
LedgerID uint32 `schema:"ledger_id" valid:"-"`
}

type GetLedgerByIDHandler struct{}
type GetLedgerByIDHandler struct {
LedgerState *ledger.State
}

func (handler GetLedgerByIDHandler) GetResource(w HeaderWriter, r *http.Request) (interface{}, error) {
qp := LedgerByIDQuery{}
err := getParams(&qp, r)
if err != nil {
return nil, err
}
if int32(qp.LedgerID) < ledger.CurrentState().HistoryElder {
if int32(qp.LedgerID) < handler.LedgerState.CurrentStatus().HistoryElder {
return nil, problem.BeforeHistory
}
historyQ, err := context.HistoryQFromRequest(r)
Expand Down
7 changes: 5 additions & 2 deletions services/horizon/internal/actions/offer.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stellar/go/protocols/horizon"
horizonContext "github.com/stellar/go/services/horizon/internal/context"
"github.com/stellar/go/services/horizon/internal/db2/history"
"github.com/stellar/go/services/horizon/internal/ledger"
"github.com/stellar/go/services/horizon/internal/resourceadapter"
"github.com/stellar/go/support/errors"
"github.com/stellar/go/support/render/hal"
Expand Down Expand Up @@ -75,6 +76,7 @@ func (q OffersQuery) Validate() error {

// GetOffersHandler is the action handler for the /offers endpoint
type GetOffersHandler struct {
LedgerState *ledger.State
}

// GetResourcePage returns a page of offers.
Expand All @@ -89,7 +91,7 @@ func (handler GetOffersHandler) GetResourcePage(
return nil, err
}

pq, err := GetPageQuery(r)
pq, err := GetPageQuery(handler.LedgerState, r)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -132,10 +134,11 @@ type AccountOffersQuery struct {
// GetAccountOffersHandler is the action handler for the
// `/accounts/{account_id}/offers` endpoint when using experimental ingestion.
type GetAccountOffersHandler struct {
LedgerState *ledger.State
}

func (handler GetAccountOffersHandler) parseOffersQuery(r *http.Request) (history.OffersQuery, error) {
pq, err := GetPageQuery(r)
pq, err := GetPageQuery(handler.LedgerState, r)
if err != nil {
return history.OffersQuery{}, err
}
Expand Down
20 changes: 13 additions & 7 deletions services/horizon/internal/actions/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,20 @@ func (qp OperationsQuery) Validate() error {

// GetOperationsHandler is the action handler for all end-points returning a list of operations.
type GetOperationsHandler struct {
LedgerState *ledger.State
OnlyPayments bool
}

// GetResourcePage returns a page of operations.
func (handler GetOperationsHandler) GetResourcePage(w HeaderWriter, r *http.Request) ([]hal.Pageable, error) {
ctx := r.Context()

pq, err := GetPageQuery(r)
pq, err := GetPageQuery(handler.LedgerState, r)
if err != nil {
return nil, err
}

err = validateCursorWithinHistory(pq)
err = validateCursorWithinHistory(handler.LedgerState, pq)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -121,18 +122,21 @@ func (handler GetOperationsHandler) GetResourcePage(w HeaderWriter, r *http.Requ
}

// GetOperationByIDHandler is the action handler for all end-points returning a list of operations.
type GetOperationByIDHandler struct{}
type GetOperationByIDHandler struct {
LedgerState *ledger.State
}

// OperationQuery query struct for operation/id end-point
type OperationQuery struct {
Joinable `valid:"optional"`
ID uint64 `schema:"id" valid:"-"`
LedgerState *ledger.State `valid:"-"`
Joinable `valid:"optional"`
ID uint64 `schema:"id" valid:"-"`
}

// Validate runs extra validations on query parameters
func (qp OperationQuery) Validate() error {
parsed := toid.Parse(int64(qp.ID))
if parsed.LedgerSequence < ledger.CurrentState().HistoryElder {
if parsed.LedgerSequence < qp.LedgerState.CurrentStatus().HistoryElder {
return problem.BeforeHistory
}
return nil
Expand All @@ -141,7 +145,9 @@ func (qp OperationQuery) Validate() error {
// GetResource returns an operation page.
func (handler GetOperationByIDHandler) GetResource(w HeaderWriter, r *http.Request) (interface{}, error) {
ctx := r.Context()
qp := OperationQuery{}
qp := OperationQuery{
LedgerState: handler.LedgerState,
}
err := getParams(&qp, r)
if err != nil {
return nil, err
Expand Down
Loading

0 comments on commit 5b28203

Please sign in to comment.