From 30cd6c02420650cabf57cfce980d647445b8c191 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 11 Jan 2024 14:26:32 -0500 Subject: [PATCH 1/9] Remove panic - 1 --- cmd/soroban-rpc/internal/config/config_option.go | 4 +--- cmd/soroban-rpc/internal/config/log_format.go | 13 ++++++++----- cmd/soroban-rpc/internal/config/options.go | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cmd/soroban-rpc/internal/config/config_option.go b/cmd/soroban-rpc/internal/config/config_option.go index 86eab8e7b..3c8ca87a4 100644 --- a/cmd/soroban-rpc/internal/config/config_option.go +++ b/cmd/soroban-rpc/internal/config/config_option.go @@ -88,8 +88,6 @@ func (o *ConfigOption) setValue(i interface{}) (err error) { if o.CustomSetValue != nil { return o.CustomSetValue(o, i) } - // it's unfortunate that Set below panics when it cannot set the value.. - // we'll want to catch this so that we can alert the user nicely. defer func() { if recoverRes := recover(); recoverRes != nil { var ok bool @@ -101,7 +99,7 @@ func (o *ConfigOption) setValue(i interface{}) (err error) { } }() parser := func(option *ConfigOption, i interface{}) error { - panic(fmt.Sprintf("no parser for flag %s", o.Name)) + return errors.Errorf("no parser for flag %s", o.Name) } switch o.ConfigKey.(type) { case *bool: diff --git a/cmd/soroban-rpc/internal/config/log_format.go b/cmd/soroban-rpc/internal/config/log_format.go index 076e43e68..aca6f8048 100644 --- a/cmd/soroban-rpc/internal/config/log_format.go +++ b/cmd/soroban-rpc/internal/config/log_format.go @@ -1,6 +1,9 @@ package config -import "fmt" +import ( + "fmt" + "github.com/stellar/go/support/errors" +) type LogFormat int @@ -47,13 +50,13 @@ func (f *LogFormat) UnmarshalTOML(i interface{}) error { } } -func (f LogFormat) String() string { +func (f LogFormat) String() (string, error) { switch f { case LogFormatText: - return "text" + return "text", nil case LogFormatJSON: - return "json" + return "json", nil default: - panic(fmt.Sprintf("unknown log format: %d", f)) + return "", errors.Errorf("unknown log format: %d", f) } } diff --git a/cmd/soroban-rpc/internal/config/options.go b/cmd/soroban-rpc/internal/config/options.go index cecfb2e7d..d38dd7cd5 100644 --- a/cmd/soroban-rpc/internal/config/options.go +++ b/cmd/soroban-rpc/internal/config/options.go @@ -130,7 +130,7 @@ func (cfg *Config) options() ConfigOptions { return nil }, MarshalTOML: func(option *ConfigOption) (interface{}, error) { - return cfg.LogFormat.String(), nil + return cfg.LogFormat.String() }, }, { From 93ab0fe110cb81263c77cedf8b5c17cfdf828a4b Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 11 Jan 2024 14:47:26 -0500 Subject: [PATCH 2/9] Remove panic - 2 --- cmd/soroban-rpc/internal/events/events.go | 16 +++++++++++++--- .../ledgerbucketwindow/ledgerbucketwindow.go | 7 ++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/cmd/soroban-rpc/internal/events/events.go b/cmd/soroban-rpc/internal/events/events.go index 0c5fdc839..aa4bd2448 100644 --- a/cmd/soroban-rpc/internal/events/events.go +++ b/cmd/soroban-rpc/internal/events/events.go @@ -114,10 +114,17 @@ func (m *MemoryStore) Scan(eventRange Range, f func(xdr.DiagnosticEvent, Cursor, } firstLedgerInRange := eventRange.Start.Ledger - firstLedgerInWindow := m.eventsByLedger.Get(0).LedgerSeq + firstBucket, err := m.eventsByLedger.Get(0) + if err != nil { + return 0, err + } + firstLedgerInWindow := firstBucket.LedgerSeq lastLedgerInWindow := firstLedgerInWindow + (m.eventsByLedger.Len() - 1) for i := firstLedgerInRange - firstLedgerInWindow; i < m.eventsByLedger.Len(); i++ { - bucket := m.eventsByLedger.Get(i) + bucket, err := m.eventsByLedger.Get(i) + if err != nil { + return 0, err + } events := bucket.BucketContent if bucket.LedgerSeq == firstLedgerInRange { // we need to seek for the beginning of the events in the first bucket in the range @@ -146,7 +153,10 @@ func (m *MemoryStore) validateRange(eventRange *Range) error { if m.eventsByLedger.Len() == 0 { return errors.New("event store is empty") } - firstBucket := m.eventsByLedger.Get(0) + firstBucket, err := m.eventsByLedger.Get(0) + if err != nil { + return err + } 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 index 0d447e716..2f0f3a54f 100644 --- a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go +++ b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go @@ -2,6 +2,7 @@ package ledgerbucketwindow import ( "fmt" + "github.com/stellar/go/support/errors" ) // LedgerBucketWindow is a sequence of buckets associated to a ledger window. @@ -66,11 +67,11 @@ func (w *LedgerBucketWindow[T]) Len() uint32 { } // Get obtains a bucket from the window -func (w *LedgerBucketWindow[T]) Get(i uint32) *LedgerBucket[T] { +func (w *LedgerBucketWindow[T]) Get(i uint32) (*LedgerBucket[T], error) { length := w.Len() if i >= length { - panic(fmt.Errorf("index out of range [%d] with length %d", i, length)) + return nil, errors.Errorf("index out of range [%d] with length %d", i, length) } index := (w.start + i) % length - return &w.buckets[index] + return &w.buckets[index], nil } From 868de8929d520a1597a7dbf7c81e93728ab877cd Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Fri, 12 Jan 2024 10:13:09 -0500 Subject: [PATCH 3/9] Remove panic - 3 --- .../ledgerbucketwindow/ledgerbucketwindow.go | 7 +- .../ledgerbucketwindow_test.go | 183 +++++++++--------- .../internal/methods/get_transaction.go | 7 +- cmd/soroban-rpc/internal/methods/health.go | 5 +- .../internal/transactions/transactions.go | 32 ++- .../transactions/transactions_test.go | 18 +- 6 files changed, 133 insertions(+), 119 deletions(-) diff --git a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go index 2f0f3a54f..32e6e97d3 100644 --- a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go +++ b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go @@ -1,7 +1,6 @@ package ledgerbucketwindow import ( - "fmt" "github.com/stellar/go/support/errors" ) @@ -36,12 +35,12 @@ func NewLedgerBucketWindow[T any](retentionWindow uint32) *LedgerBucketWindow[T] } // Append adds a new bucket to the window. If the window is full a bucket will be evicted and returned. -func (w *LedgerBucketWindow[T]) Append(bucket LedgerBucket[T]) *LedgerBucket[T] { +func (w *LedgerBucketWindow[T]) Append(bucket LedgerBucket[T]) (*LedgerBucket[T], error) { length := w.Len() if length > 0 { expectedLedgerSequence := w.buckets[w.start].LedgerSeq + length if expectedLedgerSequence != bucket.LedgerSeq { - panic(fmt.Errorf("ledgers not contiguous: expected ledger sequence %v but received %v", expectedLedgerSequence, bucket.LedgerSeq)) + return &LedgerBucket[T]{}, errors.Errorf("ledgers not contiguous: expected ledger sequence %v but received %v", expectedLedgerSequence, bucket.LedgerSeq) } } @@ -58,7 +57,7 @@ func (w *LedgerBucketWindow[T]) Append(bucket LedgerBucket[T]) *LedgerBucket[T] w.start = (w.start + 1) % length } - return evicted + return evicted, nil } // Len returns the length (number of buckets in the window) diff --git a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go index b472af0b4..75b9a850f 100644 --- a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go +++ b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go @@ -18,118 +18,109 @@ func TestAppend(t *testing.T) { m := NewLedgerBucketWindow[uint32](3) require.Equal(t, uint32(0), m.Len()) - // test appending first bucket of events - evicted := m.Append(bucket(5)) + // Test appending first bucket of events + evicted, err := m.Append(bucket(5)) + require.NoError(t, err) require.Nil(t, evicted) require.Equal(t, uint32(1), m.Len()) - require.Equal(t, bucket(5), *m.Get(0)) - - // the next bucket must follow the previous bucket (ledger 5) - - require.PanicsWithError( - t, "ledgers not contiguous: expected ledger sequence 6 but received 10", - func() { - m.Append(LedgerBucket[uint32]{ - LedgerSeq: 10, - LedgerCloseTimestamp: 100, - BucketContent: 10, - }) - }, - ) - require.PanicsWithError( - t, "ledgers not contiguous: expected ledger sequence 6 but received 4", - func() { - m.Append(LedgerBucket[uint32]{ - LedgerSeq: 4, - LedgerCloseTimestamp: 100, - BucketContent: 4, - }) - }, - ) - require.PanicsWithError( - t, "ledgers not contiguous: expected ledger sequence 6 but received 5", - func() { - m.Append(LedgerBucket[uint32]{ - LedgerSeq: 5, - LedgerCloseTimestamp: 100, - BucketContent: 5, - }) - }, - ) - // check that none of the calls above modified our buckets + + firstBucket, err := m.Get(0) + require.NoError(t, err) + require.Equal(t, bucket(5), *firstBucket) + + // The next bucket must follow the previous bucket (ledger 5) + _, err = m.Append(LedgerBucket[uint32]{ + LedgerSeq: 10, + LedgerCloseTimestamp: 100, + BucketContent: 10, + }) + require.Errorf(t, err, "ledgers not contiguous: expected ledger sequence 6 but received 10") + + _, err = m.Append(LedgerBucket[uint32]{ + LedgerSeq: 4, + LedgerCloseTimestamp: 100, + BucketContent: 4, + }) + require.Errorf(t, err, "ledgers not contiguous: expected ledger sequence 6 but received 4") + + // Check that none of the calls above modified our buckets require.Equal(t, uint32(1), m.Len()) - require.Equal(t, bucket(5), *m.Get(0)) + firstBucket, err = m.Get(0) + require.NoError(t, err) + require.Equal(t, bucket(5), *firstBucket) - // append ledger 6 bucket, now we have two buckets filled - evicted = m.Append(bucket(6)) + // 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()) - require.Equal(t, bucket(5), *m.Get(0)) - require.Equal(t, bucket(6), *m.Get(1)) - - // the next bucket of events must follow the previous bucket (ledger 6) - require.PanicsWithError( - t, "ledgers not contiguous: expected ledger sequence 7 but received 10", - func() { - m.Append(LedgerBucket[uint32]{ - LedgerSeq: 10, - LedgerCloseTimestamp: 100, - BucketContent: 10, - }) - }, - ) - require.PanicsWithError( - t, "ledgers not contiguous: expected ledger sequence 7 but received 4", - func() { - m.Append(LedgerBucket[uint32]{ - LedgerSeq: 4, - LedgerCloseTimestamp: 100, - BucketContent: 4, - }) - }, - ) - require.PanicsWithError( - t, "ledgers not contiguous: expected ledger sequence 7 but received 5", - func() { - m.Append(LedgerBucket[uint32]{ - LedgerSeq: 5, - LedgerCloseTimestamp: 100, - BucketContent: 5, - }) - }, - ) - - // append ledger 7, now we have all three buckets filled - evicted = m.Append(bucket(7)) - require.Nil(t, evicted) + + secondBucket, err := m.Get(1) + require.NoError(t, err) + require.Equal(t, bucket(6), *secondBucket) + + // 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()) - 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 = m.Append(bucket(8)) + thirdBucket, err := m.Get(2) + require.NoError(t, err) + require.Equal(t, bucket(7), *thirdBucket) + + // 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()) - 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 = m.Append(bucket(9)) + 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) + + // 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()) - 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. + 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) + + // Append ledger 10, but all buckets are full, so we need to evict ledger 7. // The start index must have wrapped around - evicted = m.Append(bucket(10)) + evicted, err = m.Append(bucket(10)) + require.NoError(t, err) require.Equal(t, bucket(7), *evicted) require.Equal(t, uint32(3), m.Len()) - require.Equal(t, bucket(8), *m.Get(0)) - require.Equal(t, bucket(9), *m.Get(1)) - require.Equal(t, bucket(10), *m.Get(2)) + + 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) } diff --git a/cmd/soroban-rpc/internal/methods/get_transaction.go b/cmd/soroban-rpc/internal/methods/get_transaction.go index 7a2fe6575..9367cf515 100644 --- a/cmd/soroban-rpc/internal/methods/get_transaction.go +++ b/cmd/soroban-rpc/internal/methods/get_transaction.go @@ -63,7 +63,7 @@ type GetTransactionRequest struct { } type transactionGetter interface { - GetTransaction(hash xdr.Hash) (transactions.Transaction, bool, transactions.StoreRange) + GetTransaction(hash xdr.Hash) (transactions.Transaction, bool, transactions.StoreRange, error) } func GetTransaction(getter transactionGetter, request GetTransactionRequest) (GetTransactionResponse, error) { @@ -84,7 +84,10 @@ func GetTransaction(getter transactionGetter, request GetTransactionRequest) (Ge } } - tx, found, storeRange := getter.GetTransaction(txHash) + tx, found, storeRange, err := getter.GetTransaction(txHash) + if err != nil { + return GetTransactionResponse{}, err + } response := GetTransactionResponse{ LatestLedger: storeRange.LastLedger.Sequence, LatestLedgerCloseTime: storeRange.LastLedger.CloseTime, diff --git a/cmd/soroban-rpc/internal/methods/health.go b/cmd/soroban-rpc/internal/methods/health.go index ab51cc789..8e58b2708 100644 --- a/cmd/soroban-rpc/internal/methods/health.go +++ b/cmd/soroban-rpc/internal/methods/health.go @@ -18,7 +18,10 @@ 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 := txStore.GetLatestLedger() + ledgerInfo, err := txStore.GetLatestLedger() + if err != nil { + return HealthCheckResult{}, err + } 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 index 8d58a035a..07f1299ff 100644 --- a/cmd/soroban-rpc/internal/transactions/transactions.go +++ b/cmd/soroban-rpc/internal/transactions/transactions.go @@ -122,7 +122,10 @@ func (m *MemoryStore) IngestTransactions(ledgerCloseMeta xdr.LedgerCloseMeta) er m.lock.Lock() defer m.lock.Unlock() - evicted := m.transactionsByLedger.Append(bucket) + evicted, err := m.transactionsByLedger.Append(bucket) + if err != nil { + return err + } if evicted != nil { // garbage-collect evicted entries for _, evictedTxHash := range evicted.BucketContent { @@ -158,28 +161,37 @@ type StoreRange struct { } // GetLatestLedger returns the latest ledger available in the store. -func (m *MemoryStore) GetLatestLedger() LedgerInfo { +func (m *MemoryStore) GetLatestLedger() (LedgerInfo, error) { m.lock.RLock() defer m.lock.RUnlock() if m.transactionsByLedger.Len() > 0 { - lastBucket := m.transactionsByLedger.Get(m.transactionsByLedger.Len() - 1) + lastBucket, err := m.transactionsByLedger.Get(m.transactionsByLedger.Len() - 1) + if err != nil { + return LedgerInfo{}, err + } return LedgerInfo{ Sequence: lastBucket.LedgerSeq, CloseTime: lastBucket.LedgerCloseTimestamp, - } + }, nil } - return LedgerInfo{} + return LedgerInfo{}, nil } // 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) { +func (m *MemoryStore) GetTransaction(hash xdr.Hash) (Transaction, bool, StoreRange, error) { startTime := time.Now() m.lock.RLock() defer m.lock.RUnlock() var storeRange StoreRange if m.transactionsByLedger.Len() > 0 { - firstBucket := m.transactionsByLedger.Get(0) - lastBucket := m.transactionsByLedger.Get(m.transactionsByLedger.Len() - 1) + 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 + } storeRange = StoreRange{ FirstLedger: LedgerInfo{ Sequence: firstBucket.LedgerSeq, @@ -193,7 +205,7 @@ func (m *MemoryStore) GetTransaction(hash xdr.Hash) (Transaction, bool, StoreRan } internalTx, ok := m.transactions[hash] if !ok { - return Transaction{}, false, storeRange + return Transaction{}, false, storeRange, nil } tx := Transaction{ Result: internalTx.result, @@ -209,5 +221,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 + return tx, true, storeRange, nil } diff --git a/cmd/soroban-rpc/internal/transactions/transactions_test.go b/cmd/soroban-rpc/internal/transactions/transactions_test.go index d32a62c6d..7d6108ad4 100644 --- a/cmd/soroban-rpc/internal/transactions/transactions_test.go +++ b/cmd/soroban-rpc/internal/transactions/transactions_test.go @@ -240,12 +240,14 @@ func txEnvelope(ledgerSequence uint32, feeBump bool) xdr.TransactionEnvelope { } func requirePresent(t *testing.T, store *MemoryStore, feeBump bool, ledgerSequence, firstSequence, lastSequence uint32) { - tx, ok, storeRange := store.GetTransaction(txHash(ledgerSequence, false)) + tx, ok, storeRange, err := store.GetTransaction(txHash(ledgerSequence, false)) + require.NoError(t, err) require.True(t, ok) require.Equal(t, expectedTransaction(t, ledgerSequence, feeBump), tx) require.Equal(t, expectedStoreRange(firstSequence, lastSequence), storeRange) if feeBump { - tx, ok, storeRange = store.GetTransaction(txHash(ledgerSequence, true)) + tx, ok, storeRange, err = store.GetTransaction(txHash(ledgerSequence, true)) + require.NoError(t, err) require.True(t, ok) require.Equal(t, expectedTransaction(t, ledgerSequence, feeBump), tx) require.Equal(t, expectedStoreRange(firstSequence, lastSequence), storeRange) @@ -256,7 +258,8 @@ func TestIngestTransactions(t *testing.T) { // Use a small retention window to test eviction store := NewMemoryStore(interfaces.MakeNoOpDeamon(), "passphrase", 3) - _, ok, storeRange := store.GetTransaction(txHash(1, false)) + _, ok, storeRange, err := store.GetTransaction(txHash(1, false)) + require.NoError(t, err) require.False(t, ok) require.Equal(t, StoreRange{}, storeRange) @@ -286,7 +289,8 @@ func TestIngestTransactions(t *testing.T) { requirePresent(t, store, false, 3, 2, 4) requirePresent(t, store, false, 4, 2, 4) - _, ok, storeRange = store.GetTransaction(txHash(1, false)) + _, ok, storeRange, err = store.GetTransaction(txHash(1, false)) + require.NoError(t, err) require.False(t, ok) require.Equal(t, expectedStoreRange(2, 4), storeRange) require.Equal(t, uint32(3), store.transactionsByLedger.Len()) @@ -298,13 +302,15 @@ func TestIngestTransactions(t *testing.T) { requirePresent(t, store, false, 4, 3, 5) requirePresent(t, store, false, 5, 3, 5) - _, ok, storeRange = store.GetTransaction(txHash(2, false)) + _, ok, storeRange, err = store.GetTransaction(txHash(2, false)) + require.NoError(t, err) 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 = store.GetTransaction(txHash(2, true)) + _, ok, storeRange, err = store.GetTransaction(txHash(2, true)) + require.NoError(t, err) require.False(t, ok) require.Equal(t, expectedStoreRange(3, 5), storeRange) require.Equal(t, uint32(3), store.transactionsByLedger.Len()) From 7b8b60fb688e0d42dc9051dfa8f4d43d25993049 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Fri, 12 Jan 2024 10:36:23 -0500 Subject: [PATCH 4/9] Remove panic - 4 --- cmd/soroban-rpc/internal/methods/send_transaction.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-rpc/internal/methods/send_transaction.go b/cmd/soroban-rpc/internal/methods/send_transaction.go index c8a0ff844..f24ded8f8 100644 --- a/cmd/soroban-rpc/internal/methods/send_transaction.go +++ b/cmd/soroban-rpc/internal/methods/send_transaction.go @@ -48,7 +48,7 @@ type SendTransactionRequest struct { // LatestLedgerStore is a store which returns the latest ingested ledger. type LatestLedgerStore interface { // GetLatestLedger returns the latest ingested ledger. - GetLatestLedger() transactions.LedgerInfo + GetLatestLedger() (transactions.LedgerInfo, error) } // NewSendTransactionHandler returns a submit transaction json rpc handler @@ -74,7 +74,11 @@ func NewSendTransactionHandler(daemon interfaces.Daemon, logger *log.Entry, stor } txHash := hex.EncodeToString(hash[:]) - ledgerInfo := store.GetLatestLedger() + ledgerInfo, err := store.GetLatestLedger() + if err != nil { + return SendTransactionResponse{}, err + } + resp, err := submitter.SubmitTransaction(ctx, request.Transaction) if err != nil { logger.WithError(err). From cc27f39c64755ac5d702f379dc2915b83b9d04b0 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Fri, 12 Jan 2024 11:18:22 -0500 Subject: [PATCH 5/9] Small changes - 1 --- cmd/soroban-rpc/internal/config/log_format.go | 3 +-- .../internal/ledgerbucketwindow/ledgerbucketwindow.go | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/cmd/soroban-rpc/internal/config/log_format.go b/cmd/soroban-rpc/internal/config/log_format.go index aca6f8048..1ab4c7fc6 100644 --- a/cmd/soroban-rpc/internal/config/log_format.go +++ b/cmd/soroban-rpc/internal/config/log_format.go @@ -2,7 +2,6 @@ package config import ( "fmt" - "github.com/stellar/go/support/errors" ) type LogFormat int @@ -57,6 +56,6 @@ func (f LogFormat) String() (string, error) { case LogFormatJSON: return "json", nil default: - return "", errors.Errorf("unknown log format: %d", f) + return "", fmt.Errorf("unknown log format: %d", f) } } diff --git a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go index 32e6e97d3..c0d73efc0 100644 --- a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go +++ b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go @@ -1,7 +1,7 @@ package ledgerbucketwindow import ( - "github.com/stellar/go/support/errors" + "fmt" ) // LedgerBucketWindow is a sequence of buckets associated to a ledger window. @@ -40,7 +40,7 @@ func (w *LedgerBucketWindow[T]) Append(bucket LedgerBucket[T]) (*LedgerBucket[T] if length > 0 { expectedLedgerSequence := w.buckets[w.start].LedgerSeq + length if expectedLedgerSequence != bucket.LedgerSeq { - return &LedgerBucket[T]{}, errors.Errorf("ledgers not contiguous: expected ledger sequence %v but received %v", expectedLedgerSequence, bucket.LedgerSeq) + return &LedgerBucket[T]{}, fmt.Errorf("ledgers not contiguous: expected ledger sequence %v but received %v", expectedLedgerSequence, bucket.LedgerSeq) } } @@ -69,7 +69,7 @@ func (w *LedgerBucketWindow[T]) Len() uint32 { func (w *LedgerBucketWindow[T]) Get(i uint32) (*LedgerBucket[T], error) { length := w.Len() if i >= length { - return nil, errors.Errorf("index out of range [%d] with length %d", i, length) + return nil, fmt.Errorf("index out of range [%d] with length %d", i, length) } index := (w.start + i) % length return &w.buckets[index], nil From 532a6ae530142e98dd2e3806d644dd8da4e539a5 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 18 Jan 2024 13:25:47 -0500 Subject: [PATCH 6/9] undo changes in Get() func --- .../internal/config/config_option_test.go | 11 +++ cmd/soroban-rpc/internal/events/events.go | 16 +---- .../ledgerbucketwindow/ledgerbucketwindow.go | 6 +- .../ledgerbucketwindow_test.go | 69 +++++-------------- cmd/soroban-rpc/internal/methods/health.go | 5 +- .../internal/transactions/transactions.go | 27 +++----- .../transactions/transactions_test.go | 18 ++--- 7 files changed, 50 insertions(+), 102 deletions(-) diff --git a/cmd/soroban-rpc/internal/config/config_option_test.go b/cmd/soroban-rpc/internal/config/config_option_test.go index 831c88656..a6309cb3e 100644 --- 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 index aa4bd2448..0c5fdc839 100644 --- 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 index c0d73efc0..c9ee24f0f 100644 --- 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 index 75b9a850f..9aa93f0ab 100644 --- 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,67 +40,44 @@ 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 @@ -111,16 +85,7 @@ func TestAppend(t *testing.T) { 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 index 8e58b2708..ab51cc789 100644 --- 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 index 07f1299ff..5ed719ade 100644 --- 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 index 7d6108ad4..d32a62c6d 100644 --- 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()) From 6063490a0a435df5c848fac898d748956bdaf6ed Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 18 Jan 2024 13:29:36 -0500 Subject: [PATCH 7/9] undo changes - 2 --- cmd/soroban-rpc/internal/methods/get_transaction.go | 4 ++-- cmd/soroban-rpc/internal/methods/send_transaction.go | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/cmd/soroban-rpc/internal/methods/get_transaction.go b/cmd/soroban-rpc/internal/methods/get_transaction.go index 9367cf515..52df47552 100644 --- a/cmd/soroban-rpc/internal/methods/get_transaction.go +++ b/cmd/soroban-rpc/internal/methods/get_transaction.go @@ -63,7 +63,7 @@ type GetTransactionRequest struct { } type transactionGetter interface { - GetTransaction(hash xdr.Hash) (transactions.Transaction, bool, transactions.StoreRange, error) + GetTransaction(hash xdr.Hash) (transactions.Transaction, bool, transactions.StoreRange) } func GetTransaction(getter transactionGetter, request GetTransactionRequest) (GetTransactionResponse, error) { @@ -84,7 +84,7 @@ func GetTransaction(getter transactionGetter, request GetTransactionRequest) (Ge } } - tx, found, storeRange, err := getter.GetTransaction(txHash) + tx, found, storeRange := getter.GetTransaction(txHash) if err != nil { return GetTransactionResponse{}, err } diff --git a/cmd/soroban-rpc/internal/methods/send_transaction.go b/cmd/soroban-rpc/internal/methods/send_transaction.go index f24ded8f8..9fe1ae2da 100644 --- a/cmd/soroban-rpc/internal/methods/send_transaction.go +++ b/cmd/soroban-rpc/internal/methods/send_transaction.go @@ -48,7 +48,7 @@ type SendTransactionRequest struct { // LatestLedgerStore is a store which returns the latest ingested ledger. type LatestLedgerStore interface { // GetLatestLedger returns the latest ingested ledger. - GetLatestLedger() (transactions.LedgerInfo, error) + GetLatestLedger() transactions.LedgerInfo } // NewSendTransactionHandler returns a submit transaction json rpc handler @@ -74,10 +74,7 @@ func NewSendTransactionHandler(daemon interfaces.Daemon, logger *log.Entry, stor } txHash := hex.EncodeToString(hash[:]) - ledgerInfo, err := store.GetLatestLedger() - if err != nil { - return SendTransactionResponse{}, err - } + ledgerInfo := store.GetLatestLedger() resp, err := submitter.SubmitTransaction(ctx, request.Transaction) if err != nil { From 9dbe116c3037c6d7acf93f20789c918db307558d Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 18 Jan 2024 13:30:41 -0500 Subject: [PATCH 8/9] undo changes - 3 --- cmd/soroban-rpc/internal/methods/get_transaction.go | 3 --- cmd/soroban-rpc/internal/methods/send_transaction.go | 1 - 2 files changed, 4 deletions(-) diff --git a/cmd/soroban-rpc/internal/methods/get_transaction.go b/cmd/soroban-rpc/internal/methods/get_transaction.go index 52df47552..7a2fe6575 100644 --- a/cmd/soroban-rpc/internal/methods/get_transaction.go +++ b/cmd/soroban-rpc/internal/methods/get_transaction.go @@ -85,9 +85,6 @@ func GetTransaction(getter transactionGetter, request GetTransactionRequest) (Ge } tx, found, storeRange := getter.GetTransaction(txHash) - if err != nil { - return GetTransactionResponse{}, err - } response := GetTransactionResponse{ LatestLedger: storeRange.LastLedger.Sequence, LatestLedgerCloseTime: storeRange.LastLedger.CloseTime, diff --git a/cmd/soroban-rpc/internal/methods/send_transaction.go b/cmd/soroban-rpc/internal/methods/send_transaction.go index 9fe1ae2da..c8a0ff844 100644 --- a/cmd/soroban-rpc/internal/methods/send_transaction.go +++ b/cmd/soroban-rpc/internal/methods/send_transaction.go @@ -75,7 +75,6 @@ func NewSendTransactionHandler(daemon interfaces.Daemon, logger *log.Entry, stor txHash := hex.EncodeToString(hash[:]) ledgerInfo := store.GetLatestLedger() - resp, err := submitter.SubmitTransaction(ctx, request.Transaction) if err != nil { logger.WithError(err). From d0a5ce3e72e88770411950077a95ffde6fa7be15 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 18 Jan 2024 14:49:30 -0500 Subject: [PATCH 9/9] add test for append error --- .../ledgerbucketwindow/ledgerbucketwindow.go | 2 +- .../ledgerbucketwindow/ledgerbucketwindow_test.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go index c9ee24f0f..8234b6076 100644 --- a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go +++ b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow.go @@ -40,7 +40,7 @@ func (w *LedgerBucketWindow[T]) Append(bucket LedgerBucket[T]) (*LedgerBucket[T] if length > 0 { expectedLedgerSequence := w.buckets[w.start].LedgerSeq + length if expectedLedgerSequence != bucket.LedgerSeq { - return &LedgerBucket[T]{}, fmt.Errorf("ledgers not contiguous: expected ledger sequence %v but received %v", expectedLedgerSequence, bucket.LedgerSeq) + return &LedgerBucket[T]{}, fmt.Errorf("error appending ledgers: ledgers not contiguous: expected ledger sequence %v but received %v", expectedLedgerSequence, bucket.LedgerSeq) } } diff --git a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go index 9aa93f0ab..2e50ed6d5 100644 --- a/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go +++ b/cmd/soroban-rpc/internal/ledgerbucketwindow/ledgerbucketwindow_test.go @@ -89,3 +89,16 @@ func TestAppend(t *testing.T) { require.Equal(t, bucket(9), *m.Get(1)) require.Equal(t, bucket(10), *m.Get(2)) } + +func TestAppendError(t *testing.T) { + m := NewLedgerBucketWindow[uint32](3) + require.Equal(t, uint32(0), m.Len()) + + evicted, err := m.Append(bucket(5)) + require.NoError(t, err) + require.Nil(t, evicted) + + evicted, err = m.Append(bucket(1)) + require.Error(t, err) + require.Contains(t, err.Error(), "error appending ledgers: ledgers not contiguous: expected ledger sequence 6 but received 1") +}