From 966ac43bc96b3d9022ebdaefcc5075cdeaba4e38 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Thu, 13 Jun 2024 17:04:40 +0200 Subject: [PATCH] Remove JSONRPC client leaks --- .../network/requestdurationlimiter_test.go | 6 ++++ .../internal/test/get_fee_stats_test.go | 6 +--- .../internal/test/get_ledger_entries_test.go | 10 ++---- .../internal/test/get_ledger_entry_test.go | 10 ++---- .../internal/test/get_network_test.go | 5 +-- .../internal/test/get_transactions_test.go | 4 +-- .../internal/test/get_version_info_test.go | 8 ++--- cmd/soroban-rpc/internal/test/health_test.go | 5 +-- cmd/soroban-rpc/internal/test/integration.go | 16 +++++++--- cmd/soroban-rpc/internal/test/migrate_test.go | 5 +-- .../test/simulate_transaction_test.go | 32 ++++++------------- .../internal/test/transaction_test.go | 19 ++++------- cmd/soroban-rpc/internal/test/upgrade_test.go | 5 +-- 13 files changed, 48 insertions(+), 83 deletions(-) diff --git a/cmd/soroban-rpc/internal/network/requestdurationlimiter_test.go b/cmd/soroban-rpc/internal/network/requestdurationlimiter_test.go index 5be64cea..b815e6cd 100644 --- a/cmd/soroban-rpc/internal/network/requestdurationlimiter_test.go +++ b/cmd/soroban-rpc/internal/network/requestdurationlimiter_test.go @@ -206,7 +206,9 @@ func TestJRPCRequestDurationLimiter_Limiting(t *testing.T) { logCounter.Entry()).Handle ch := jhttp.NewChannel("http://"+addr+"/", nil) + defer ch.Close() client := jrpc2.NewClient(ch, nil) + defer client.Close() var res interface{} req := struct { @@ -250,7 +252,9 @@ func TestJRPCRequestDurationLimiter_NoLimiting(t *testing.T) { logCounter.Entry()).Handle ch := jhttp.NewChannel("http://"+addr+"/", nil) + defer ch.Close() client := jrpc2.NewClient(ch, nil) + defer client.Close() var res interface{} req := struct { @@ -291,7 +295,9 @@ func TestJRPCRequestDurationLimiter_NoLimiting_Warn(t *testing.T) { logCounter.Entry()).Handle ch := jhttp.NewChannel("http://"+addr+"/", nil) + defer ch.Close() client := jrpc2.NewClient(ch, nil) + defer client.Close() var res interface{} req := struct { diff --git a/cmd/soroban-rpc/internal/test/get_fee_stats_test.go b/cmd/soroban-rpc/internal/test/get_fee_stats_test.go index b1de24e7..15969aea 100644 --- a/cmd/soroban-rpc/internal/test/get_fee_stats_test.go +++ b/cmd/soroban-rpc/internal/test/get_fee_stats_test.go @@ -4,8 +4,6 @@ import ( "context" "testing" - "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stellar/go/keypair" "github.com/stellar/go/txnbuild" "github.com/stellar/go/xdr" @@ -18,9 +16,7 @@ import ( func TestGetFeeStats(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) - + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase) address := sourceAccount.Address() account := txnbuild.NewSimpleAccount(address, 0) diff --git a/cmd/soroban-rpc/internal/test/get_ledger_entries_test.go b/cmd/soroban-rpc/internal/test/get_ledger_entries_test.go index 50c0af9e..835f183f 100644 --- a/cmd/soroban-rpc/internal/test/get_ledger_entries_test.go +++ b/cmd/soroban-rpc/internal/test/get_ledger_entries_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -20,8 +19,7 @@ import ( func TestGetLedgerEntriesNotFound(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase).Address() contractID := getContractID(t, sourceAccount, testSalt, StandaloneNetworkPassphrase) @@ -58,8 +56,7 @@ func TestGetLedgerEntriesNotFound(t *testing.T) { func TestGetLedgerEntriesInvalidParams(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() var keys []string keys = append(keys, "<>@@#$") @@ -76,8 +73,7 @@ func TestGetLedgerEntriesInvalidParams(t *testing.T) { func TestGetLedgerEntriesSucceeds(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase) address := sourceAccount.Address() diff --git a/cmd/soroban-rpc/internal/test/get_ledger_entry_test.go b/cmd/soroban-rpc/internal/test/get_ledger_entry_test.go index df606bfc..007dd0f2 100644 --- a/cmd/soroban-rpc/internal/test/get_ledger_entry_test.go +++ b/cmd/soroban-rpc/internal/test/get_ledger_entry_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stellar/go/txnbuild" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -20,8 +19,7 @@ import ( func TestGetLedgerEntryNotFound(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase).Address() contractID := getContractID(t, sourceAccount, testSalt, StandaloneNetworkPassphrase) @@ -53,8 +51,7 @@ func TestGetLedgerEntryNotFound(t *testing.T) { func TestGetLedgerEntryInvalidParams(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() request := methods.GetLedgerEntryRequest{ Key: "<>@@#$", @@ -69,8 +66,7 @@ func TestGetLedgerEntryInvalidParams(t *testing.T) { func TestGetLedgerEntrySucceeds(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() kp := keypair.Root(StandaloneNetworkPassphrase) account := txnbuild.NewSimpleAccount(kp.Address(), 0) diff --git a/cmd/soroban-rpc/internal/test/get_network_test.go b/cmd/soroban-rpc/internal/test/get_network_test.go index dad90771..777a48e4 100644 --- a/cmd/soroban-rpc/internal/test/get_network_test.go +++ b/cmd/soroban-rpc/internal/test/get_network_test.go @@ -4,8 +4,6 @@ import ( "context" "testing" - "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stretchr/testify/assert" "github.com/stellar/soroban-rpc/cmd/soroban-rpc/internal/methods" @@ -14,8 +12,7 @@ import ( func TestGetNetworkSucceeds(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() request := methods.GetNetworkRequest{} diff --git a/cmd/soroban-rpc/internal/test/get_transactions_test.go b/cmd/soroban-rpc/internal/test/get_transactions_test.go index f3da92dd..48371cea 100644 --- a/cmd/soroban-rpc/internal/test/get_transactions_test.go +++ b/cmd/soroban-rpc/internal/test/get_transactions_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stellar/go/keypair" "github.com/stellar/go/txnbuild" "github.com/stretchr/testify/assert" @@ -57,8 +56,7 @@ func sendTransactions(t *testing.T, client *jrpc2.Client) []uint32 { func TestGetTransactions(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() ledgers := sendTransactions(t, client) diff --git a/cmd/soroban-rpc/internal/test/get_version_info_test.go b/cmd/soroban-rpc/internal/test/get_version_info_test.go index 9f406b8d..25a43f62 100644 --- a/cmd/soroban-rpc/internal/test/get_version_info_test.go +++ b/cmd/soroban-rpc/internal/test/get_version_info_test.go @@ -3,12 +3,11 @@ package test import ( "context" "fmt" - "github.com/stellar/soroban-rpc/cmd/soroban-rpc/internal/config" "os/exec" "testing" - "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" + "github.com/stellar/soroban-rpc/cmd/soroban-rpc/internal/config" + "github.com/stretchr/testify/assert" "github.com/stellar/soroban-rpc/cmd/soroban-rpc/internal/methods" @@ -28,8 +27,7 @@ func TestGetVersionInfoSucceeds(t *testing.T) { config.BuildTimestamp = buildTimeStamp }) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() var result methods.GetVersionInfoResponse err := client.CallResult(context.Background(), "getVersionInfo", nil, &result) diff --git a/cmd/soroban-rpc/internal/test/health_test.go b/cmd/soroban-rpc/internal/test/health_test.go index 0840959c..adae006c 100644 --- a/cmd/soroban-rpc/internal/test/health_test.go +++ b/cmd/soroban-rpc/internal/test/health_test.go @@ -4,8 +4,6 @@ import ( "context" "testing" - "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stretchr/testify/assert" "github.com/stellar/soroban-rpc/cmd/soroban-rpc/internal/ledgerbucketwindow" @@ -15,8 +13,7 @@ import ( func TestHealth(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() var result methods.HealthCheckResult if err := client.CallResult(context.Background(), "getHealth", nil, &result); err != nil { diff --git a/cmd/soroban-rpc/internal/test/integration.go b/cmd/soroban-rpc/internal/test/integration.go index 99d74f3f..5fa0eab1 100644 --- a/cmd/soroban-rpc/internal/test/integration.go +++ b/cmd/soroban-rpc/internal/test/integration.go @@ -81,6 +81,7 @@ type Test struct { masterAccount txnbuild.Account shutdownOnce sync.Once shutdownCalls []func() + rpcClient *jrpc2.Client } func NewTest(t *testing.T, cfg *TestConfig) *Test { @@ -119,6 +120,9 @@ func NewTest(t *testing.T, cfg *TestConfig) *Test { proxy.ServeHTTP(w, r) })) + ch := jhttp.NewChannel(i.sorobanRPCURL(), nil) + t.Cleanup(func() { ch.Close() }) + i.rpcClient = jrpc2.NewClient(ch, nil) rpcCfg := i.getRPConfig(sqlLitePath) if i.rpcContainerVersion != "" { i.rpcContainerConfigMountDir = i.createRPCContainerMountDir(rpcCfg) @@ -137,6 +141,9 @@ func NewTest(t *testing.T, cfg *TestConfig) *Test { return i } +func (i *Test) GetRPCLient() *jrpc2.Client { + return i.rpcClient +} func (i *Test) MasterKey() *keypair.Full { return keypair.Root(StandaloneNetworkPassphrase) } @@ -237,13 +244,9 @@ func (i *Test) getRPConfig(sqlitePath string) map[string]string { func (i *Test) waitForRPC() { i.t.Log("Waiting for RPC to be healthy...") - ch := jhttp.NewChannel(i.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) - defer client.Close() - var result methods.HealthCheckResult for t := 120; t >= 0; t-- { - err := client.CallResult(context.Background(), "getHealth", nil, &result) + err := i.rpcClient.CallResult(context.Background(), "getHealth", nil, &result) if err == nil { if result.Status == "healthy" { i.t.Log("RPC is healthy") @@ -349,6 +352,9 @@ func (i *Test) prepareShutdownHandlers() { if i.historyArchiveProxy != nil { i.historyArchiveProxy.Close() } + if i.rpcClient != nil { + i.rpcClient.Close() + } i.runComposeCommand("down", "-v") }, ) diff --git a/cmd/soroban-rpc/internal/test/migrate_test.go b/cmd/soroban-rpc/internal/test/migrate_test.go index eded0e61..c99f74d8 100644 --- a/cmd/soroban-rpc/internal/test/migrate_test.go +++ b/cmd/soroban-rpc/internal/test/migrate_test.go @@ -11,8 +11,6 @@ import ( "strings" "testing" - "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stellar/go/keypair" "github.com/stellar/go/txnbuild" "github.com/stretchr/testify/assert" @@ -53,8 +51,7 @@ func testMigrateFromVersion(t *testing.T, version string) { UseSQLitePath: sqliteFile, }) - ch := jhttp.NewChannel(it.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := it.GetRPCLient() // Submit an event-logging transaction in the version to migrate from kp := keypair.Root(StandaloneNetworkPassphrase) diff --git a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go index 318142e1..642c7937 100644 --- a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go +++ b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go @@ -11,7 +11,6 @@ import ( "time" "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -191,9 +190,7 @@ func preflightTransactionParams(t *testing.T, client *jrpc2.Client, params txnbu func TestSimulateTransactionSucceeds(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) - + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase).Address() contractBinary := getHelloWorldContract(t) params := txnbuild.TransactionParams{ @@ -323,8 +320,7 @@ func TestSimulateTransactionSucceeds(t *testing.T) { func TestSimulateTransactionWithAuth(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase) address := sourceAccount.Address() @@ -381,8 +377,7 @@ func TestSimulateTransactionWithAuth(t *testing.T) { func TestSimulateInvokeContractTransactionSucceeds(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase) address := sourceAccount.Address() @@ -565,8 +560,7 @@ func TestSimulateInvokeContractTransactionSucceeds(t *testing.T) { func TestSimulateTransactionError(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase).Address() invokeHostOp := createInvokeHostOperation(sourceAccount, xdr.Hash{}, "noMethod") @@ -605,8 +599,7 @@ func TestSimulateTransactionError(t *testing.T) { func TestSimulateTransactionMultipleOperations(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase).Address() contractBinary := getHelloWorldContract(t) @@ -640,8 +633,7 @@ func TestSimulateTransactionMultipleOperations(t *testing.T) { func TestSimulateTransactionWithoutInvokeHostFunction(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() params := txnbuild.TransactionParams{ SourceAccount: &txnbuild.SimpleAccount{ @@ -671,8 +663,7 @@ func TestSimulateTransactionWithoutInvokeHostFunction(t *testing.T) { func TestSimulateTransactionUnmarshalError(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() request := methods.SimulateTransactionRequest{Transaction: "invalid"} var result methods.SimulateTransactionResponse @@ -688,8 +679,7 @@ func TestSimulateTransactionUnmarshalError(t *testing.T) { func TestSimulateTransactionExtendAndRestoreFootprint(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase) address := sourceAccount.Address() @@ -921,8 +911,7 @@ func waitUntilLedgerEntryTTL(t *testing.T, client *jrpc2.Client, ledgerKey xdr.L func TestSimulateInvokePrng_u64_in_range(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase) address := sourceAccount.Address() @@ -1032,8 +1021,7 @@ func TestSimulateInvokePrng_u64_in_range(t *testing.T) { func TestSimulateSystemEvent(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase) address := sourceAccount.Address() diff --git a/cmd/soroban-rpc/internal/test/transaction_test.go b/cmd/soroban-rpc/internal/test/transaction_test.go index f838ad6b..4a8461f6 100644 --- a/cmd/soroban-rpc/internal/test/transaction_test.go +++ b/cmd/soroban-rpc/internal/test/transaction_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -23,8 +22,7 @@ import ( func TestSendTransactionSucceedsWithoutResults(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() kp := keypair.Root(StandaloneNetworkPassphrase) address := kp.Address() @@ -48,8 +46,7 @@ func TestSendTransactionSucceedsWithoutResults(t *testing.T) { func TestSendTransactionSucceedsWithResults(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() kp := keypair.Root(StandaloneNetworkPassphrase) address := kp.Address() @@ -112,8 +109,7 @@ func TestSendTransactionSucceedsWithResults(t *testing.T) { func TestSendTransactionBadSequence(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() kp := keypair.Root(StandaloneNetworkPassphrase) address := kp.Address() @@ -154,8 +150,7 @@ func TestSendTransactionBadSequence(t *testing.T) { func TestSendTransactionFailedInsufficientResourceFee(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() kp := keypair.Root(StandaloneNetworkPassphrase) address := kp.Address() @@ -206,8 +201,7 @@ func TestSendTransactionFailedInsufficientResourceFee(t *testing.T) { func TestSendTransactionFailedInLedger(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() kp := keypair.Root(StandaloneNetworkPassphrase) address := kp.Address() @@ -268,8 +262,7 @@ func TestSendTransactionFailedInLedger(t *testing.T) { func TestSendTransactionFailedInvalidXDR(t *testing.T) { test := NewTest(t, nil) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() request := methods.SendTransactionRequest{Transaction: "abcdef"} var response methods.SendTransactionResponse diff --git a/cmd/soroban-rpc/internal/test/upgrade_test.go b/cmd/soroban-rpc/internal/test/upgrade_test.go index 6889e614..6a59cefd 100644 --- a/cmd/soroban-rpc/internal/test/upgrade_test.go +++ b/cmd/soroban-rpc/internal/test/upgrade_test.go @@ -5,8 +5,6 @@ import ( "testing" "time" - "github.com/creachadair/jrpc2" - "github.com/creachadair/jrpc2/jhttp" "github.com/stellar/go/keypair" "github.com/stellar/go/txnbuild" "github.com/stellar/go/xdr" @@ -24,8 +22,7 @@ func TestUpgradeFrom20To21(t *testing.T) { ProtocolVersion: 20, }) - ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) - client := jrpc2.NewClient(ch, nil) + client := test.GetRPCLient() sourceAccount := keypair.Root(StandaloneNetworkPassphrase) address := sourceAccount.Address()