From 21f93fa15e1fecf61c5cb48d54d8789ae4702702 Mon Sep 17 00:00:00 2001 From: tycho garen Date: Fri, 4 Mar 2022 15:23:08 -0500 Subject: [PATCH] unwind client creator --- internal/blocksync/reactor_test.go | 2 +- internal/mempool/mempool_test.go | 13 +++++----- internal/proxy/app_conn_test.go | 36 +++++++++++++-------------- internal/proxy/multi_app_conn_test.go | 15 ++--------- internal/state/execution_test.go | 18 ++++++++------ internal/state/helpers_test.go | 9 ------- internal/state/validation_test.go | 16 +++++++----- node/node.go | 9 ++++--- node/node_test.go | 6 ++--- node/public.go | 2 +- rpc/test/helpers.go | 2 +- test/e2e/node/main.go | 2 +- test/fuzz/mempool/checktx.go | 13 +++------- 13 files changed, 63 insertions(+), 80 deletions(-) diff --git a/internal/blocksync/reactor_test.go b/internal/blocksync/reactor_test.go index 59e093675..d73a522fa 100644 --- a/internal/blocksync/reactor_test.go +++ b/internal/blocksync/reactor_test.go @@ -109,7 +109,7 @@ func (rts *reactorTestSuite) addNode( logger := log.TestingLogger() rts.nodes = append(rts.nodes, nodeID) - rts.app[nodeID] = proxy.New(abciclient.NewLocalCreator(&abci.BaseApplication{}), logger, proxy.NopMetrics()) + rts.app[nodeID] = proxy.New(abciclient.NewLocalClient(logger, &abci.BaseApplication{}), logger, proxy.NopMetrics()) require.NoError(t, rts.app[nodeID].Start(ctx)) blockDB := dbm.NewMemDB() diff --git a/internal/mempool/mempool_test.go b/internal/mempool/mempool_test.go index 68eb5731b..a29303d03 100644 --- a/internal/mempool/mempool_test.go +++ b/internal/mempool/mempool_test.go @@ -78,24 +78,23 @@ func setup(ctx context.Context, t testing.TB, cacheSize int, options ...TxMempoo var cancel context.CancelFunc ctx, cancel = context.WithCancel(ctx) - app := &application{kvstore.NewApplication()} - cc := abciclient.NewLocalCreator(app) logger := log.TestingLogger() + app := &application{kvstore.NewApplication()} + conn := abciclient.NewLocalClient(logger, app) + cfg, err := config.ResetTestRoot(t.TempDir(), strings.ReplaceAll(t.Name(), "/", "|")) require.NoError(t, err) cfg.Mempool.CacheSize = cacheSize - appConnMem, err := cc(logger) - require.NoError(t, err) - require.NoError(t, appConnMem.Start(ctx)) + require.NoError(t, conn.Start(ctx)) t.Cleanup(func() { os.RemoveAll(cfg.RootDir) cancel() - appConnMem.Wait() + conn.Wait() }) - return NewTxMempool(logger.With("test", t.Name()), cfg.Mempool, appConnMem, options...) + return NewTxMempool(logger.With("test", t.Name()), cfg.Mempool, conn, options...) } func checkTxs(ctx context.Context, t *testing.T, txmp *TxMempool, numTxs int, peerID uint16) []testTx { diff --git a/internal/proxy/app_conn_test.go b/internal/proxy/app_conn_test.go index 22f519657..526d5819f 100644 --- a/internal/proxy/app_conn_test.go +++ b/internal/proxy/app_conn_test.go @@ -51,7 +51,10 @@ var SOCKET = "socket" func TestEcho(t *testing.T) { sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6)) logger := log.TestingLogger() - clientCreator := abciclient.NewRemoteCreator(logger, sockPath, SOCKET, true) + client, err := abciclient.NewClient(logger, sockPath, SOCKET, true) + if err != nil { + t.Fatal(err) + } ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -62,12 +65,9 @@ func TestEcho(t *testing.T) { t.Cleanup(func() { cancel(); s.Wait() }) // Start client - cli, err := clientCreator(logger.With("module", "abci-client")) - require.NoError(t, err, "Error creating ABCI client:") - - require.NoError(t, cli.Start(ctx), "Error starting ABCI client") + require.NoError(t, client.Start(ctx), "Error starting ABCI client") - proxy := newAppConnTest(cli) + proxy := newAppConnTest(client) t.Log("Connected") for i := 0; i < 1000; i++ { @@ -91,7 +91,10 @@ func BenchmarkEcho(b *testing.B) { b.StopTimer() // Initialize sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6)) logger := log.TestingLogger() - clientCreator := abciclient.NewRemoteCreator(logger, sockPath, SOCKET, true) + client, err := abciclient.NewClient(logger, sockPath, SOCKET, true) + if err != nil { + b.Fatal(err) + } ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -102,12 +105,9 @@ func BenchmarkEcho(b *testing.B) { b.Cleanup(func() { cancel(); s.Wait() }) // Start client - cli, err := clientCreator(logger.With("module", "abci-client")) - require.NoError(b, err, "Error creating ABCI client") + require.NoError(b, client.Start(ctx), "Error starting ABCI client") - require.NoError(b, cli.Start(ctx), "Error starting ABCI client") - - proxy := newAppConnTest(cli) + proxy := newAppConnTest(client) b.Log("Connected") echoString := strings.Repeat(" ", 200) b.StartTimer() // Start benchmarking tests @@ -139,7 +139,10 @@ func TestInfo(t *testing.T) { sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6)) logger := log.TestingLogger() - clientCreator := abciclient.NewRemoteCreator(logger, sockPath, SOCKET, true) + client, err := abciclient.NewClient(logger, sockPath, SOCKET, true) + if err != nil { + t.Fatal(err) + } // Start server s := server.NewSocketServer(logger.With("module", "abci-server"), sockPath, kvstore.NewApplication()) @@ -147,12 +150,9 @@ func TestInfo(t *testing.T) { t.Cleanup(func() { cancel(); s.Wait() }) // Start client - cli, err := clientCreator(logger.With("module", "abci-client")) - require.NoError(t, err, "Error creating ABCI client") - - require.NoError(t, cli.Start(ctx), "Error starting ABCI client") + require.NoError(t, client.Start(ctx), "Error starting ABCI client") - proxy := newAppConnTest(cli) + proxy := newAppConnTest(client) t.Log("Connected") resInfo, err := proxy.Info(ctx, RequestInfo) diff --git a/internal/proxy/multi_app_conn_test.go b/internal/proxy/multi_app_conn_test.go index 07f9d0f87..f3f96e3ac 100644 --- a/internal/proxy/multi_app_conn_test.go +++ b/internal/proxy/multi_app_conn_test.go @@ -36,13 +36,7 @@ func TestAppConns_Start_Stop(t *testing.T) { clientMock.On("Wait").Return(nil).Times(1) cl := &noopStoppableClientImpl{Client: clientMock} - creatorCallCount := 0 - creator := func(logger log.Logger) (abciclient.Client, error) { - creatorCallCount++ - return cl, nil - } - - appConns := New(creator, log.TestingLogger(), NopMetrics()) + appConns := New(cl, log.TestingLogger(), NopMetrics()) err := appConns.Start(ctx) require.NoError(t, err) @@ -54,7 +48,6 @@ func TestAppConns_Start_Stop(t *testing.T) { clientMock.AssertExpectations(t) assert.Equal(t, 1, cl.count) - assert.Equal(t, 1, creatorCallCount) } // Upon failure, we call tmos.Kill @@ -80,11 +73,7 @@ func TestAppConns_Failure(t *testing.T) { clientMock.On("Error").Return(errors.New("EOF")) cl := &noopStoppableClientImpl{Client: clientMock} - creator := func(log.Logger) (abciclient.Client, error) { - return cl, nil - } - - appConns := New(creator, log.TestingLogger(), NopMetrics()) + appConns := New(cl, log.TestingLogger(), NopMetrics()) err := appConns.Start(ctx) require.NoError(t, err) diff --git a/internal/state/execution_test.go b/internal/state/execution_test.go index be2392a96..321c684ee 100644 --- a/internal/state/execution_test.go +++ b/internal/state/execution_test.go @@ -39,8 +39,8 @@ var ( func TestApplyBlock(t *testing.T) { app := &testApp{} - cc := abciclient.NewLocalCreator(app) logger := log.TestingLogger() + cc := abciclient.NewLocalClient(logger, app) proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) ctx, cancel := context.WithCancel(context.Background()) @@ -73,9 +73,10 @@ func TestBeginBlockValidators(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() + logger := log.TestingLogger() app := &testApp{} - cc := abciclient.NewLocalCreator(app) - proxyApp := proxy.New(cc, log.TestingLogger(), proxy.NopMetrics()) + cc := abciclient.NewLocalClient(logger, app) + proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) err := proxyApp.Start(ctx) require.NoError(t, err) @@ -145,8 +146,9 @@ func TestBeginBlockByzantineValidators(t *testing.T) { defer cancel() app := &testApp{} - cc := abciclient.NewLocalCreator(app) - proxyApp := proxy.New(cc, log.TestingLogger(), proxy.NopMetrics()) + logger := log.TestingLogger() + cc := abciclient.NewLocalClient(logger, app) + proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) err := proxyApp.Start(ctx) require.NoError(t, err) @@ -248,8 +250,8 @@ func TestProcessProposal(t *testing.T) { defer cancel() app := abcimocks.NewBaseMock() - cc := abciclient.NewLocalCreator(app) logger := log.TestingLogger() + cc := abciclient.NewLocalClient(logger, app) proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) err := proxyApp.Start(ctx) require.NoError(t, err) @@ -451,8 +453,8 @@ func TestEndBlockValidatorUpdates(t *testing.T) { defer cancel() app := &testApp{} - cc := abciclient.NewLocalCreator(app) logger := log.TestingLogger() + cc := abciclient.NewLocalClient(logger, app) proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) err := proxyApp.Start(ctx) require.NoError(t, err) @@ -526,8 +528,8 @@ func TestEndBlockValidatorUpdatesResultingInEmptySet(t *testing.T) { defer cancel() app := &testApp{} - cc := abciclient.NewLocalCreator(app) logger := log.TestingLogger() + cc := abciclient.NewLocalClient(logger, app) proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) err := proxyApp.Start(ctx) require.NoError(t, err) diff --git a/internal/state/helpers_test.go b/internal/state/helpers_test.go index af67a0232..da2d7a9c8 100644 --- a/internal/state/helpers_test.go +++ b/internal/state/helpers_test.go @@ -11,16 +11,13 @@ import ( "github.com/stretchr/testify/require" dbm "github.com/tendermint/tm-db" - abciclient "github.com/tendermint/tendermint/abci/client" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/encoding" - "github.com/tendermint/tendermint/internal/proxy" sm "github.com/tendermint/tendermint/internal/state" sf "github.com/tendermint/tendermint/internal/state/test/factory" "github.com/tendermint/tendermint/internal/test/factory" - "github.com/tendermint/tendermint/libs/log" tmrand "github.com/tendermint/tendermint/libs/rand" tmtime "github.com/tendermint/tendermint/libs/time" tmstate "github.com/tendermint/tendermint/proto/tendermint/state" @@ -33,12 +30,6 @@ type paramsChangeTestCase struct { params types.ConsensusParams } -func newTestApp() abciclient.Client { - app := &testApp{} - cc := abciclient.NewLocalCreator(app) - return proxy.New(cc, log.NewNopLogger(), proxy.NopMetrics()) -} - func makeAndCommitGoodBlock( ctx context.Context, t *testing.T, diff --git a/internal/state/validation_test.go b/internal/state/validation_test.go index b90f9b134..d24b5098a 100644 --- a/internal/state/validation_test.go +++ b/internal/state/validation_test.go @@ -10,10 +10,12 @@ import ( "github.com/stretchr/testify/require" dbm "github.com/tendermint/tm-db" + abciclient "github.com/tendermint/tendermint/abci/client" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/tmhash" memmock "github.com/tendermint/tendermint/internal/mempool/mock" + "github.com/tendermint/tendermint/internal/proxy" sm "github.com/tendermint/tendermint/internal/state" "github.com/tendermint/tendermint/internal/state/mocks" statefactory "github.com/tendermint/tendermint/internal/state/test/factory" @@ -30,8 +32,8 @@ const validationTestsStopHeight int64 = 10 func TestValidateBlockHeader(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - - proxyApp := newTestApp() + logger := log.TestingLogger() + proxyApp := proxy.New(abciclient.NewLocalClient(logger, &testApp{}), logger, proxy.NopMetrics()) require.NoError(t, proxyApp.Start(ctx)) state, stateDB, privVals := makeState(t, 3, 1) @@ -39,7 +41,7 @@ func TestValidateBlockHeader(t *testing.T) { blockStore := store.NewBlockStore(dbm.NewMemDB()) blockExec := sm.NewBlockExecutor( stateStore, - log.TestingLogger(), + logger, proxyApp, memmock.Mempool{}, sm.EmptyEvidencePool{}, @@ -119,7 +121,8 @@ func TestValidateBlockCommit(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - proxyApp := newTestApp() + logger := log.TestingLogger() + proxyApp := proxy.New(abciclient.NewLocalClient(logger, &testApp{}), logger, proxy.NopMetrics()) require.NoError(t, proxyApp.Start(ctx)) state, stateDB, privVals := makeState(t, 1, 1) @@ -127,7 +130,7 @@ func TestValidateBlockCommit(t *testing.T) { blockStore := store.NewBlockStore(dbm.NewMemDB()) blockExec := sm.NewBlockExecutor( stateStore, - log.TestingLogger(), + logger, proxyApp, memmock.Mempool{}, sm.EmptyEvidencePool{}, @@ -245,7 +248,8 @@ func TestValidateBlockEvidence(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - proxyApp := newTestApp() + logger := log.TestingLogger() + proxyApp := proxy.New(abciclient.NewLocalClient(logger, &testApp{}), logger, proxy.NopMetrics()) require.NoError(t, proxyApp.Start(ctx)) state, stateDB, privVals := makeState(t, 4, 1) diff --git a/node/node.go b/node/node.go index 13c37d3b5..ce3a77ace 100644 --- a/node/node.go +++ b/node/node.go @@ -100,7 +100,10 @@ func newDefaultNode( return nil, err } - appClient, _ := proxy.ClientFactory(logger, cfg.ProxyApp, cfg.ABCI, cfg.DBDir()) + appClient, _, err := proxy.ClientFactory(logger, cfg.ProxyApp, cfg.ABCI, cfg.DBDir()) + if err != nil { + return nil, err + } return makeNode( ctx, @@ -120,7 +123,7 @@ func makeNode( cfg *config.Config, filePrivval *privval.FilePV, nodeKey types.NodeKey, - clientCreator abciclient.Creator, + client abciclient.Client, genesisDocProvider genesisDocProvider, dbProvider config.DBProvider, logger log.Logger, @@ -155,7 +158,7 @@ func makeNode( nodeMetrics := defaultMetricsProvider(cfg.Instrumentation)(genDoc.ChainID) // Create the proxyApp and establish connections to the ABCI app (consensus, mempool, query). - proxyApp := proxy.New(clientCreator, logger.With("module", "proxy"), nodeMetrics.proxy) + proxyApp := proxy.New(client, logger.With("module", "proxy"), nodeMetrics.proxy) if err := proxyApp.Start(ctx); err != nil { return nil, fmt.Errorf("error starting proxy app connections: %w", err) } diff --git a/node/node_test.go b/node/node_test.go index f9fc8fb9b..690da1a4d 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -273,7 +273,7 @@ func TestCreateProposalBlock(t *testing.T) { logger := log.NewNopLogger() - cc := abciclient.NewLocalCreator(kvstore.NewApplication()) + cc := abciclient.NewLocalClient(logger, kvstore.NewApplication()) proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) err = proxyApp.Start(ctx) require.NoError(t, err) @@ -371,7 +371,7 @@ func TestMaxTxsProposalBlockSize(t *testing.T) { logger := log.NewNopLogger() - cc := abciclient.NewLocalCreator(kvstore.NewApplication()) + cc := abciclient.NewLocalClient(logger, kvstore.NewApplication()) proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) err = proxyApp.Start(ctx) require.NoError(t, err) @@ -438,7 +438,7 @@ func TestMaxProposalBlockSize(t *testing.T) { logger := log.NewNopLogger() - cc := abciclient.NewLocalCreator(kvstore.NewApplication()) + cc := abciclient.NewLocalClient(logger, kvstore.NewApplication()) proxyApp := proxy.New(cc, logger, proxy.NopMetrics()) err = proxyApp.Start(ctx) require.NoError(t, err) diff --git a/node/public.go b/node/public.go index 0d6f1d93e..af3aece8e 100644 --- a/node/public.go +++ b/node/public.go @@ -35,7 +35,7 @@ func New( ctx context.Context, conf *config.Config, logger log.Logger, - cf abciclient.Creator, + cf abciclient.Client, gen *types.GenesisDoc, ) (service.Service, error) { nodeKey, err := types.LoadOrGenNodeKey(conf.NodeKeyFile()) diff --git a/rpc/test/helpers.go b/rpc/test/helpers.go index 19deac607..4c13b322a 100644 --- a/rpc/test/helpers.go +++ b/rpc/test/helpers.go @@ -98,7 +98,7 @@ func StartTendermint( } } - papp := abciclient.NewLocalCreator(app) + papp := abciclient.NewLocalClient(logger, app) tmNode, err := node.New(ctx, conf, logger, papp, nil) if err != nil { return nil, func(_ context.Context) error { cancel(); return nil }, err diff --git a/test/e2e/node/main.go b/test/e2e/node/main.go index 704cc06bb..7f46eea89 100644 --- a/test/e2e/node/main.go +++ b/test/e2e/node/main.go @@ -136,7 +136,7 @@ func startNode(ctx context.Context, cfg *Config) error { ctx, tmcfg, nodeLogger, - abciclient.NewLocalCreator(app), + abciclient.NewLocalClient(nodeLogger, app), nil, ) if err != nil { diff --git a/test/fuzz/mempool/checktx.go b/test/fuzz/mempool/checktx.go index a6e7006d0..8be90f0c2 100644 --- a/test/fuzz/mempool/checktx.go +++ b/test/fuzz/mempool/checktx.go @@ -15,9 +15,9 @@ var getMp func() mempool.Mempool func init() { app := kvstore.NewApplication() - cc := abciclient.NewLocalCreator(app) - appConnMem, _ := cc(log.NewNopLogger()) - err := appConnMem.Start(context.TODO()) + logger := log.NewNopLogger() + conn := abciclient.NewLocalClient(logger, app) + err := conn.Start(context.TODO()) if err != nil { panic(err) } @@ -27,12 +27,7 @@ func init() { getMp = func() mempool.Mempool { if mp == nil { - mp = mempool.NewTxMempool( - log.NewNopLogger(), - cfg, - appConnMem, - ) - + mp = mempool.NewTxMempool(logger, cfg, conn) } return mp }