undo changes in Get() func

aditya1702 · Jan 18, 2024 · 532a6ae · 532a6ae
1 parent cc27f39
commit 532a6ae
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 102 deletions.
diff --git a/cmd/soroban-rpc/internal/config/config_option_test.go b/cmd/soroban-rpc/internal/config/config_option_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"math"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -115,6 +116,16 @@ func TestUnassignableField(t *testing.T) {
 	require.Contains(t, err.Error(), co.Name)
 }
 
+func TestNoParserForFlag(t *testing.T) {
+	var co ConfigOption
+	var invalidKey []time.Duration
+	co.Name = "mykey"
+	co.ConfigKey = &invalidKey
+	err := co.setValue("abc")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "no parser for flag mykey")
+}
+
 func TestSetValue(t *testing.T) {
 	var b bool
 	var i int

diff --git a/cmd/soroban-rpc/internal/events/events.go b/cmd/soroban-rpc/internal/events/events.go
@@ -114,17 +114,10 @@ func (m *MemoryStore) Scan(eventRange Range, f func(xdr.DiagnosticEvent, Cursor,
 	}
 
 	firstLedgerInRange := eventRange.Start.Ledger
-	firstBucket, err := m.eventsByLedger.Get(0)
-	if err != nil {
-		return 0, err
-	}
-	firstLedgerInWindow := firstBucket.LedgerSeq
+	firstLedgerInWindow := m.eventsByLedger.Get(0).LedgerSeq
 	lastLedgerInWindow := firstLedgerInWindow + (m.eventsByLedger.Len() - 1)
 	for i := firstLedgerInRange - firstLedgerInWindow; i < m.eventsByLedger.Len(); i++ {
-		bucket, err := m.eventsByLedger.Get(i)
-		if err != nil {
-			return 0, err
-		}
+		bucket := m.eventsByLedger.Get(i)
 		events := bucket.BucketContent
 		if bucket.LedgerSeq == firstLedgerInRange {
 			// we need to seek for the beginning of the events in the first bucket in the range
@@ -153,10 +146,7 @@ func (m *MemoryStore) validateRange(eventRange *Range) error {
 	if m.eventsByLedger.Len() == 0 {
 		return errors.New("event store is empty")
 	}
-	firstBucket, err := m.eventsByLedger.Get(0)
-	if err != nil {
-		return err
-	}
+	firstBucket := m.eventsByLedger.Get(0)
 	min := Cursor{Ledger: firstBucket.LedgerSeq}
 	if eventRange.Start.Cmp(min) < 0 {
 		if eventRange.ClampStart {

diff --git a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go
@@ -66,11 +66,11 @@ func (w *LedgerBucketWindow[T]) Len() uint32 {
 }
 
 // Get obtains a bucket from the window
-func (w *LedgerBucketWindow[T]) Get(i uint32) (*LedgerBucket[T], error) {
+func (w *LedgerBucketWindow[T]) Get(i uint32) *LedgerBucket[T] {
 	length := w.Len()
 	if i >= length {
-		return nil, fmt.Errorf("index out of range [%d] with length %d", i, length)
+		panic(fmt.Errorf("index out of range [%d] with length %d", i, length))
 	}
 	index := (w.start + i) % length
-	return &w.buckets[index], nil
+	return &w.buckets[index]
 }
diff --git a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go
@@ -23,10 +23,7 @@ func TestAppend(t *testing.T) {
 	require.NoError(t, err)
 	require.Nil(t, evicted)
 	require.Equal(t, uint32(1), m.Len())
-
-	firstBucket, err := m.Get(0)
-	require.NoError(t, err)
-	require.Equal(t, bucket(5), *firstBucket)
+	require.Equal(t, bucket(5), *m.Get(0))
 
 	// The next bucket must follow the previous bucket (ledger 5)
 	_, err = m.Append(LedgerBucket[uint32]{
@@ -43,84 +40,52 @@ func TestAppend(t *testing.T) {
 	})
 	require.Errorf(t, err, "ledgers not contiguous: expected ledger sequence 6 but received 4")
 
-	// Check that none of the calls above modified our buckets
+	// check that none of the calls above modified our buckets
 	require.Equal(t, uint32(1), m.Len())
-	firstBucket, err = m.Get(0)
-	require.NoError(t, err)
-	require.Equal(t, bucket(5), *firstBucket)
+	require.Equal(t, bucket(5), *m.Get(0))
 
 	// Append ledger 6 bucket, now we have two buckets filled
 	evicted, err = m.Append(bucket(6))
 	require.NoError(t, err)
 	require.Nil(t, evicted)
 	require.Equal(t, uint32(2), m.Len())
-
-	secondBucket, err := m.Get(1)
-	require.NoError(t, err)
-	require.Equal(t, bucket(6), *secondBucket)
+	require.Equal(t, bucket(5), *m.Get(0))
+	require.Equal(t, bucket(6), *m.Get(1))
 
 	// Append ledger 7, now we have all three buckets filled
 	evicted, err = m.Append(bucket(7))
 	require.NoError(t, err)
 	require.Nil(t, evicted)
 	require.Equal(t, uint32(3), m.Len())
-
-	thirdBucket, err := m.Get(2)
-	require.NoError(t, err)
-	require.Equal(t, bucket(7), *thirdBucket)
+	require.Equal(t, bucket(5), *m.Get(0))
+	require.Equal(t, bucket(6), *m.Get(1))
+	require.Equal(t, bucket(7), *m.Get(2))
 
 	// Append ledger 8, but all buckets are full, so we need to evict ledger 5
 	evicted, err = m.Append(bucket(8))
 	require.NoError(t, err)
 	require.Equal(t, bucket(5), *evicted)
 	require.Equal(t, uint32(3), m.Len())
-
-	firstBucket, err = m.Get(0)
-	require.NoError(t, err)
-	require.Equal(t, bucket(6), *firstBucket)
-
-	secondBucket, err = m.Get(1)
-	require.NoError(t, err)
-	require.Equal(t, bucket(7), *secondBucket)
-
-	thirdBucket, err = m.Get(2)
-	require.NoError(t, err)
-	require.Equal(t, bucket(8), *thirdBucket)
+	require.Equal(t, bucket(6), *m.Get(0))
+	require.Equal(t, bucket(7), *m.Get(1))
+	require.Equal(t, bucket(8), *m.Get(2))
 
 	// Append ledger 9 events, but all buckets are full, so we need to evict ledger 6
 	evicted, err = m.Append(bucket(9))
 	require.NoError(t, err)
 	require.Equal(t, bucket(6), *evicted)
 	require.Equal(t, uint32(3), m.Len())
-
-	firstBucket, err = m.Get(0)
-	require.NoError(t, err)
-	require.Equal(t, bucket(7), *firstBucket)
-
-	secondBucket, err = m.Get(1)
-	require.NoError(t, err)
-	require.Equal(t, bucket(8), *secondBucket)
-
-	thirdBucket, err = m.Get(2)
-	require.NoError(t, err)
-	require.Equal(t, bucket(9), *thirdBucket)
+	require.Equal(t, bucket(7), *m.Get(0))
+	require.Equal(t, bucket(8), *m.Get(1))
+	require.Equal(t, bucket(9), *m.Get(2))
 
 	// Append ledger 10, but all buckets are full, so we need to evict ledger 7.
 	// The start index must have wrapped around
 	evicted, err = m.Append(bucket(10))
 	require.NoError(t, err)
 	require.Equal(t, bucket(7), *evicted)
 	require.Equal(t, uint32(3), m.Len())
-
-	firstBucket, err = m.Get(0)
-	require.NoError(t, err)
-	require.Equal(t, bucket(8), *firstBucket)
-
-	secondBucket, err = m.Get(1)
-	require.NoError(t, err)
-	require.Equal(t, bucket(9), *secondBucket)
-
-	thirdBucket, err = m.Get(2)
-	require.NoError(t, err)
-	require.Equal(t, bucket(10), *thirdBucket)
+	require.Equal(t, bucket(8), *m.Get(0))
+	require.Equal(t, bucket(9), *m.Get(1))
+	require.Equal(t, bucket(10), *m.Get(2))
 }
diff --git a/cmd/soroban-rpc/internal/methods/health.go b/cmd/soroban-rpc/internal/methods/health.go
@@ -18,10 +18,7 @@ type HealthCheckResult struct {
 // NewHealthCheck returns a health check json rpc handler
 func NewHealthCheck(txStore *transactions.MemoryStore, maxHealthyLedgerLatency time.Duration) jrpc2.Handler {
 	return handler.New(func(ctx context.Context) (HealthCheckResult, error) {
-		ledgerInfo, err := txStore.GetLatestLedger()
-		if err != nil {
-			return HealthCheckResult{}, err
-		}
+		ledgerInfo := txStore.GetLatestLedger()
 		if ledgerInfo.Sequence < 1 {
 			return HealthCheckResult{}, jrpc2.Error{
 				Code:    jrpc2.InternalError,

diff --git a/cmd/soroban-rpc/internal/transactions/transactions.go b/cmd/soroban-rpc/internal/transactions/transactions.go
@@ -161,37 +161,28 @@ type StoreRange struct {
 }
 
 // GetLatestLedger returns the latest ledger available in the store.
-func (m *MemoryStore) GetLatestLedger() (LedgerInfo, error) {
+func (m *MemoryStore) GetLatestLedger() LedgerInfo {
 	m.lock.RLock()
 	defer m.lock.RUnlock()
 	if m.transactionsByLedger.Len() > 0 {
-		lastBucket, err := m.transactionsByLedger.Get(m.transactionsByLedger.Len() - 1)
-		if err != nil {
-			return LedgerInfo{}, err
-		}
+		lastBucket := m.transactionsByLedger.Get(m.transactionsByLedger.Len() - 1)
 		return LedgerInfo{
 			Sequence:  lastBucket.LedgerSeq,
 			CloseTime: lastBucket.LedgerCloseTimestamp,
-		}, nil
+		}
 	}
-	return LedgerInfo{}, nil
+	return LedgerInfo{}
 }
 
 // GetTransaction obtains a transaction from the store and whether it's present and the current store range
-func (m *MemoryStore) GetTransaction(hash xdr.Hash) (Transaction, bool, StoreRange, error) {
+func (m *MemoryStore) GetTransaction(hash xdr.Hash) (Transaction, bool, StoreRange) {
 	startTime := time.Now()
 	m.lock.RLock()
 	defer m.lock.RUnlock()
 	var storeRange StoreRange
 	if m.transactionsByLedger.Len() > 0 {
-		firstBucket, err := m.transactionsByLedger.Get(0)
-		if err != nil {
-			return Transaction{}, false, StoreRange{}, err
-		}
-		lastBucket, err := m.transactionsByLedger.Get(m.transactionsByLedger.Len() - 1)
-		if err != nil {
-			return Transaction{}, false, StoreRange{}, err
-		}
+		firstBucket := m.transactionsByLedger.Get(0)
+		lastBucket := m.transactionsByLedger.Get(m.transactionsByLedger.Len() - 1)
 		storeRange = StoreRange{
 			FirstLedger: LedgerInfo{
 				Sequence:  firstBucket.LedgerSeq,
@@ -205,7 +196,7 @@ func (m *MemoryStore) GetTransaction(hash xdr.Hash) (Transaction, bool, StoreRan
 	}
 	internalTx, ok := m.transactions[hash]
 	if !ok {
-		return Transaction{}, false, storeRange, nil
+		return Transaction{}, false, storeRange
 	}
 	tx := Transaction{
 		Result:           internalTx.result,
@@ -221,5 +212,5 @@ func (m *MemoryStore) GetTransaction(hash xdr.Hash) (Transaction, bool, StoreRan
 	}
 
 	m.transactionDurationMetric.With(prometheus.Labels{"operation": "get"}).Observe(time.Since(startTime).Seconds())
-	return tx, true, storeRange, nil
+	return tx, true, storeRange
 }
diff --git a/cmd/soroban-rpc/internal/transactions/transactions_test.go b/cmd/soroban-rpc/internal/transactions/transactions_test.go
@@ -240,14 +240,12 @@ func txEnvelope(ledgerSequence uint32, feeBump bool) xdr.TransactionEnvelope {
 }
 
 func requirePresent(t *testing.T, store *MemoryStore, feeBump bool, ledgerSequence, firstSequence, lastSequence uint32) {
-	tx, ok, storeRange, err := store.GetTransaction(txHash(ledgerSequence, false))
-	require.NoError(t, err)
+	tx, ok, storeRange := store.GetTransaction(txHash(ledgerSequence, false))
 	require.True(t, ok)
 	require.Equal(t, expectedTransaction(t, ledgerSequence, feeBump), tx)
 	require.Equal(t, expectedStoreRange(firstSequence, lastSequence), storeRange)
 	if feeBump {
-		tx, ok, storeRange, err = store.GetTransaction(txHash(ledgerSequence, true))
-		require.NoError(t, err)
+		tx, ok, storeRange = store.GetTransaction(txHash(ledgerSequence, true))
 		require.True(t, ok)
 		require.Equal(t, expectedTransaction(t, ledgerSequence, feeBump), tx)
 		require.Equal(t, expectedStoreRange(firstSequence, lastSequence), storeRange)
@@ -258,8 +256,7 @@ func TestIngestTransactions(t *testing.T) {
 	// Use a small retention window to test eviction
 	store := NewMemoryStore(interfaces.MakeNoOpDeamon(), "passphrase", 3)
 
-	_, ok, storeRange, err := store.GetTransaction(txHash(1, false))
-	require.NoError(t, err)
+	_, ok, storeRange := store.GetTransaction(txHash(1, false))
 	require.False(t, ok)
 	require.Equal(t, StoreRange{}, storeRange)
 
@@ -289,8 +286,7 @@ func TestIngestTransactions(t *testing.T) {
 	requirePresent(t, store, false, 3, 2, 4)
 	requirePresent(t, store, false, 4, 2, 4)
 
-	_, ok, storeRange, err = store.GetTransaction(txHash(1, false))
-	require.NoError(t, err)
+	_, ok, storeRange = store.GetTransaction(txHash(1, false))
 	require.False(t, ok)
 	require.Equal(t, expectedStoreRange(2, 4), storeRange)
 	require.Equal(t, uint32(3), store.transactionsByLedger.Len())
@@ -302,15 +298,13 @@ func TestIngestTransactions(t *testing.T) {
 	requirePresent(t, store, false, 4, 3, 5)
 	requirePresent(t, store, false, 5, 3, 5)
 
-	_, ok, storeRange, err = store.GetTransaction(txHash(2, false))
-	require.NoError(t, err)
+	_, ok, storeRange = store.GetTransaction(txHash(2, false))
 	require.False(t, ok)
 	require.Equal(t, expectedStoreRange(3, 5), storeRange)
 	require.Equal(t, uint32(3), store.transactionsByLedger.Len())
 	require.Len(t, store.transactions, 3)
 
-	_, ok, storeRange, err = store.GetTransaction(txHash(2, true))
-	require.NoError(t, err)
+	_, ok, storeRange = store.GetTransaction(txHash(2, true))
 	require.False(t, ok)
 	require.Equal(t, expectedStoreRange(3, 5), storeRange)
 	require.Equal(t, uint32(3), store.transactionsByLedger.Len())