From 1acc9af300cd8f60c24e14bdabe8fba0f1873eb0 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Wed, 18 Dec 2024 16:00:29 +0100 Subject: [PATCH] Improve bound and cache update checks --- cmd/stellar-rpc/internal/db/cursor.go | 4 ++-- cmd/stellar-rpc/internal/db/cursor_test.go | 7 +++++++ cmd/stellar-rpc/internal/db/db.go | 4 ++-- cmd/stellar-rpc/internal/db/ledger.go | 4 ++++ cmd/stellar-rpc/internal/methods/get_events.go | 13 ++++++++++--- .../internal/methods/get_transactions.go | 3 ++- 6 files changed, 27 insertions(+), 8 deletions(-) diff --git a/cmd/stellar-rpc/internal/db/cursor.go b/cmd/stellar-rpc/internal/db/cursor.go index 4102038d..6cdc1a8f 100644 --- a/cmd/stellar-rpc/internal/db/cursor.go +++ b/cmd/stellar-rpc/internal/db/cursor.go @@ -130,8 +130,8 @@ var ( //nolint:gochecknoglobals MaxCursor = Cursor{ Ledger: math.MaxInt32, - Tx: math.MaxInt32, - Op: math.MaxInt32, + Tx: toid.TransactionMask, + Op: toid.OperationMask, Event: math.MaxUint32, } ) diff --git a/cmd/stellar-rpc/internal/db/cursor_test.go b/cmd/stellar-rpc/internal/db/cursor_test.go index ab4c788d..9fcfcd74 100644 --- a/cmd/stellar-rpc/internal/db/cursor_test.go +++ b/cmd/stellar-rpc/internal/db/cursor_test.go @@ -108,3 +108,10 @@ func TestCursorCmp(t *testing.T) { } } } + +func TestMaxCursor(t *testing.T) { + str := MaxCursor.String() + cursor, err := ParseCursor(str) + require.NoError(t, err) + assert.Equal(t, MaxCursor, cursor) +} diff --git a/cmd/stellar-rpc/internal/db/db.go b/cmd/stellar-rpc/internal/db/db.go index 8cd1fa25..fa25f1d7 100644 --- a/cmd/stellar-rpc/internal/db/db.go +++ b/cmd/stellar-rpc/internal/db/db.go @@ -156,8 +156,8 @@ func getLatestLedgerSequence(ctx context.Context, ledgerReader LedgerReader, cac // Add missing ledger sequence and close time to the top cache. // Otherwise, the write-through cache won't get updated until the first ingestion commit cache.Lock() - if cache.latestLedgerSeq == 0 { - // Only update the cache if the value is missing (0), otherwise + if cache.latestLedgerSeq == 0 || cache.latestLedgerSeq < ledgerRange.LastLedger.Sequence { + // Only update the cache if the value is missing or lower, otherwise // we may end up overwriting the entry with an older version cache.latestLedgerSeq = ledgerRange.LastLedger.Sequence cache.latestLedgerCloseTime = ledgerRange.LastLedger.CloseTime diff --git a/cmd/stellar-rpc/internal/db/ledger.go b/cmd/stellar-rpc/internal/db/ledger.go index 7dd03611..cc14088c 100644 --- a/cmd/stellar-rpc/internal/db/ledger.go +++ b/cmd/stellar-rpc/internal/db/ledger.go @@ -3,6 +3,7 @@ package db import ( "context" "database/sql" + "errors" "fmt" sq "github.com/Masterminds/squirrel" @@ -63,6 +64,9 @@ func (l ledgerReaderTx) GetLedgerRange(ctx context.Context) (ledgerbucketwindow. func (l ledgerReaderTx) BatchGetLedgers(ctx context.Context, sequence uint32, batchSize uint, ) ([]xdr.LedgerCloseMeta, error) { + if batchSize < 1 { + return nil, errors.New("batch size must be greater than zero") + } sql := sq.Select("meta"). From(ledgerCloseMetaTableName). Where(sq.And{ diff --git a/cmd/stellar-rpc/internal/methods/get_events.go b/cmd/stellar-rpc/internal/methods/get_events.go index 8408ef95..ae317dca 100644 --- a/cmd/stellar-rpc/internal/methods/get_events.go +++ b/cmd/stellar-rpc/internal/methods/get_events.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "math" "strings" "time" @@ -124,10 +123,16 @@ func (g *GetEventsRequest) Valid(maxLimit uint) error { if g.StartLedger != 0 || g.EndLedger != 0 { return errors.New("ledger ranges and cursor cannot both be set") } - } else if g.StartLedger <= 0 { + } + + if g.StartLedger <= 0 { return errors.New("startLedger must be positive") } + if g.EndLedger < g.StartLedger { + return errors.New("startLedger must be >= endLedger") + } + if g.Pagination != nil && g.Pagination.Limit > maxLimit { return fmt.Errorf("limit must not exceed %d", maxLimit) } @@ -524,7 +529,9 @@ func (h eventsRPCHandler) getEvents(ctx context.Context, request GetEventsReques // cursor represents end of the search window if events does not reach limit // here endLedger is always exclusive when fetching events // so search window is max Cursor value with endLedger - 1 - cursor = db.Cursor{Ledger: endLedger - 1, Tx: math.MaxUint32, Event: math.MaxUint32 - 1}.String() + maxCursor := db.MaxCursor + maxCursor.Event = endLedger - 1 + cursor = maxCursor.String() } return GetEventsResponse{ diff --git a/cmd/stellar-rpc/internal/methods/get_transactions.go b/cmd/stellar-rpc/internal/methods/get_transactions.go index 20d4c6a2..b3c050b5 100644 --- a/cmd/stellar-rpc/internal/methods/get_transactions.go +++ b/cmd/stellar-rpc/internal/methods/get_transactions.go @@ -40,7 +40,8 @@ func (req GetTransactionsRequest) isValid(maxLimit uint, ledgerRange ledgerbucke if req.StartLedger != 0 { return errors.New("startLedger and cursor cannot both be set") } - } else if req.StartLedger < ledgerRange.FirstLedger.Sequence || req.StartLedger > ledgerRange.LastLedger.Sequence { + } + if req.StartLedger < ledgerRange.FirstLedger.Sequence || req.StartLedger > ledgerRange.LastLedger.Sequence { return fmt.Errorf( "start ledger must be between the oldest ledger: %d and the latest ledger: %d for this rpc instance", ledgerRange.FirstLedger.Sequence,