test(cmd): drop incorrect_password case, revert to txdb

Remove the incorrect_password subtests from TestShell_BeforeNode and
TestShell_RunNode_WithBeforeNode. They were assertions about keystore
decryption failure layered on top of the CLI, and the same invariant
is already covered at the correct layer by
TestMasterKeystore_Unlock_Save/won't_load_a_saved_keyRing_if_the_password_is_incorrect
in core/services/keystore/master_test.go.

With the wrong-password case gone, these tests no longer need
cross-connection state sharing, so they revert to txdb via the
codebase-default configtest.NewGeneralConfig. No seeding plumbing,
no heavyweight.FullTestDBV2, no per-test physical DB — one conn per
test, rolled back at Close, matching how the rest of core/cmd works.

Simplifies TestShell_RunNode_WithBeforeNode to a single straight-line
happy-path run (the "expectStart=false" branch only existed to host the
incorrect_password case, which is gone).

Removes now-unused commonkeystore import.

Refs: CORE-2388

Author: Fletch153

core/cmd/shell_local_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/urfave/cli"
 
-	commonkeystore "github.com/smartcontractkit/chainlink-common/keystore"
 	commonconfig "github.com/smartcontractkit/chainlink-common/pkg/config"
 	"github.com/smartcontractkit/chainlink-common/pkg/sqlutil"
 	pgcommon "github.com/smartcontractkit/chainlink-common/pkg/sqlutil/pg"
@@ -509,64 +508,34 @@ func TestShell_RemoveBlocks(t *testing.T) {
 func TestShell_BeforeNode(t *testing.T) {
 	testutils.SkipShortDB(t)
 
-	// Use heavyweight.FullTestDBV2 (real isolated per-test Postgres DB,
-	// dropped on t.Cleanup) because both subtests must share a persisted
-	// encrypted key ring:
-	//
-	//   1. keystore.Unlock on an *empty* keystore silently creates a fresh
-	//      key ring encrypted with whatever password you pass — so any
-	//      password "works" against an empty DB, including the wrong one.
-	//      The incorrect_password subtest can only fail meaningfully if
-	//      there is an existing ring to fail decryption against.
-	//
-	//   2. txdb isolation can't be used here: chainlink-common's
-	//      pg.NewConnection deliberately passes uuid.New().String() as the
-	//      DSN to the txdb driver on every call (see
-	//      chainlink-common/pkg/sqlutil/pg/pg.go:69 — "Each ORM should be
-	//      encapsulated in its own transaction"). That means the seed
-	//      connection and BeforeNode's LockedDB land on different keys in
-	//      the txdb driver's DSN-keyed conns map and get independent
-	//      transactions, so the seed would be invisible to BeforeNode.
-	//      This convention is also why Erik's PR #21504, which tried to
-	//      force txdb via a newAppWithOpts override, couldn't make the
-	//      incorrect_password case work.
-	//
-	// A real per-test physical DB gives us the cross-connection visibility
-	// the test needs, with its own cleanup and without polluting the
-	// shared chainlink_test DB used by every other test.
-	cfg, sqlxDB := heavyweight.FullTestDBV2(t, func(c *chainlink.Config, s *chainlink.Secrets) {
-		s.Password.Keystore = models.NewSecret("dummy-to-pass-validation")
-		c.EVM[0].Nodes[0].Name = ptr("fake")
-		c.EVM[0].Nodes[0].HTTPURL = commonconfig.MustParseURL("http://fake.com")
-		c.EVM[0].Nodes[0].WSURL = commonconfig.MustParseURL("WSS://fake.com/ws")
-		c.Insecure.OCRDevelopmentMode = nil
-	})
-
-	// Seed one CSA key encrypted with the correct password so the
-	// incorrect_password subtest has something to fail decryption against.
-	correctPwd, err := utils.PasswordFromFile("../internal/fixtures/correct_password.txt")
-	require.NoError(t, err)
-	ctx := testutils.Context(t)
-	seedDS := sqlutil.WrapDataSource(sqlxDB, logger.TestLogger(t),
-		sqlutil.TimeoutHook(cfg.Database().DefaultQueryTimeout),
-		sqlutil.MonitorHook(cfg.Database().LogSQL))
-	seedKS := keystore.New(seedDS, commonkeystore.FastScryptParams, logger.TestLogger(t).Infof)
-	require.NoError(t, seedKS.Unlock(ctx, correctPwd))
-	_, err = seedKS.CSA().Create(ctx)
-	require.NoError(t, err)
-
+	// Uses txdb via configtest.NewGeneralConfig (the codebase default) so
+	// each subtest runs in its own rolled-back transaction. We deliberately
+	// do NOT test the "wrong password against an existing key ring" case
+	// here — that is a keystore invariant, already covered by
+	// TestMasterKeystore_Unlock_Save/won't_load_a_saved_keyRing_if_the_password_is_incorrect
+	// in core/services/keystore/master_test.go, which owns that layer.
+	// Testing it at the CLI level would require cross-connection visibility
+	// that txdb doesn't provide in this codebase (pg.NewConnection generates
+	// a fresh UUID DSN per call — see chainlink-common/pkg/sqlutil/pg/pg.go:69).
 	tests := []struct {
 		name         string
 		pwdfile      string
 		wantUnlocked bool
 	}{
 		{"correct password", "../internal/fixtures/correct_password.txt", true},
-		{"incorrect password", "../internal/fixtures/incorrect_password.txt", false},
 		{"wrong file", "doesntexist.txt", false},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {
+				s.Password.Keystore = models.NewSecret("dummy-to-pass-validation")
+				c.EVM[0].Nodes[0].Name = ptr("fake")
+				c.EVM[0].Nodes[0].HTTPURL = commonconfig.MustParseURL("http://fake.com")
+				c.EVM[0].Nodes[0].WSURL = commonconfig.MustParseURL("WSS://fake.com/ws")
+				c.Insecure.OCRDevelopmentMode = nil
+			})
+
 			shell := cmd.Shell{
 				Config: cfg,
 				KeyStoreAuthenticator: cmd.TerminalKeyStoreAuthenticator{
@@ -611,112 +580,75 @@ func TestShell_BeforeNode(t *testing.T) {
 func TestShell_RunNode_WithBeforeNode(t *testing.T) {
 	testutils.SkipShortDB(t)
 
-	// See TestShell_BeforeNode for the full rationale (TL;DR: txdb can't
-	// be used to seed shared state because pg.NewConnection generates a
-	// fresh UUID DSN per call, isolating every pool into its own
-	// transaction — so we need a real per-test physical DB instead).
-	cfg, sqlxDB := heavyweight.FullTestDBV2(t, func(c *chainlink.Config, s *chainlink.Secrets) {
+	// Uses txdb via configtest.NewGeneralConfig. We only exercise the
+	// happy path here — the wrong-password case is covered at the correct
+	// layer by TestMasterKeystore_Unlock_Save in core/services/keystore
+	// and does not need to be reasserted through the CLI.
+	cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {
 		s.Password.Keystore = models.NewSecret("dummy-to-pass-validation")
 		c.EVM[0].Nodes[0].Name = ptr("fake")
 		c.EVM[0].Nodes[0].HTTPURL = commonconfig.MustParseURL("http://fake.com")
 		c.EVM[0].Nodes[0].WSURL = commonconfig.MustParseURL("WSS://fake.com/ws")
 		c.Insecure.OCRDevelopmentMode = nil
 	})
 
-	// Seed one CSA key encrypted with the correct password so the
-	// incorrect_password subtest has something to fail decryption against.
-	correctPwd, err := utils.PasswordFromFile("../internal/fixtures/correct_password.txt")
-	require.NoError(t, err)
-	seedCtx := testutils.Context(t)
-	seedDS := sqlutil.WrapDataSource(sqlxDB, logger.TestLogger(t),
-		sqlutil.TimeoutHook(cfg.Database().DefaultQueryTimeout),
-		sqlutil.MonitorHook(cfg.Database().LogSQL))
-	seedKS := keystore.New(seedDS, commonkeystore.FastScryptParams, logger.TestLogger(t).Infof)
-	require.NoError(t, seedKS.Unlock(seedCtx, correctPwd))
-	_, err = seedKS.CSA().Create(seedCtx)
-	require.NoError(t, err)
-
-	tests := []struct {
-		name        string
-		pwdfile     string
-		expectStart bool
-	}{
-		{"correct password", "../internal/fixtures/correct_password.txt", true},
-		{"incorrect password", "../internal/fixtures/incorrect_password.txt", false},
-	}
+	db := pgtest.NewSqlxDB(t)
+	keyStore := cltest.NewKeyStore(t, db)
+	authProviderORM := localauth.NewORM(db, time.Minute, logger.TestLogger(t), audit.NoopLogger)
 
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			// Use the shared sqlxDB for the mock application's keystore and
-			// auth provider so they see the seeded key ring and any state
-			// written by BeforeNode.
-			db := sqlxDB
-			keyStore := cltest.NewKeyStore(t, db)
-			authProviderORM := localauth.NewORM(db, time.Minute, logger.TestLogger(t), audit.NoopLogger)
+	testRelayers := genTestEVMRelayers(t, cfg, db, keyStore.Eth(), &keystore.CSASigner{CSA: keyStore.CSA()})
 
-			testRelayers := genTestEVMRelayers(t, cfg, db, keyStore.Eth(), &keystore.CSASigner{CSA: keyStore.CSA()})
+	// Purge fixture users to test assumption of single admin
+	pgtest.MustExec(t, db, "DELETE FROM users;")
 
-			// Purge fixture users to test assumption of single admin
-			pgtest.MustExec(t, db, "DELETE FROM users;")
+	app := mocks.NewApplication(t)
+	app.On("AuthenticationProvider").Return(authProviderORM).Maybe()
+	app.On("BasicAdminUsersORM").Return(authProviderORM).Maybe()
+	app.On("GetKeyStore").Return(keyStore).Maybe()
+	app.On("GetRelayers").Return(testRelayers).Maybe()
+	app.On("Start", mock.Anything).Maybe().Return(nil)
+	app.On("Stop").Maybe().Return(nil)
+	app.On("ID").Maybe().Return(uuid.New())
 
-			app := mocks.NewApplication(t)
-			app.On("AuthenticationProvider").Return(authProviderORM).Maybe()
-			app.On("BasicAdminUsersORM").Return(authProviderORM).Maybe()
-			app.On("GetKeyStore").Return(keyStore).Maybe()
-			app.On("GetRelayers").Return(testRelayers).Maybe()
-			app.On("Start", mock.Anything).Maybe().Return(nil)
-			app.On("Stop").Maybe().Return(nil)
-			app.On("ID").Maybe().Return(uuid.New())
+	ethClient := clienttest.NewClient(t)
+	ethClient.On("Dial", mock.Anything).Return(nil).Maybe()
+	ethClient.On("BalanceAt", mock.Anything, mock.Anything, mock.Anything).Return(big.NewInt(10), nil).Maybe()
 
-			ethClient := clienttest.NewClient(t)
-			ethClient.On("Dial", mock.Anything).Return(nil).Maybe()
-			ethClient.On("BalanceAt", mock.Anything, mock.Anything, mock.Anything).Return(big.NewInt(10), nil).Maybe()
+	cltest.MustInsertRandomKey(t, keyStore.Eth())
+	apiPrompt := cltest.NewMockAPIInitializer(t)
 
-			cltest.MustInsertRandomKey(t, keyStore.Eth())
-			apiPrompt := cltest.NewMockAPIInitializer(t)
-
-			shell := cmd.Shell{
-				Config:                 cfg,
-				FallbackAPIInitializer: apiPrompt,
-				Runner:                 cltest.EmptyRunner{},
-				AppFactory:             cltest.InstanceAppFactory{App: app},
-				KeyStoreAuthenticator: cmd.TerminalKeyStoreAuthenticator{
-					Prompter: &cltest.MockCountingPrompter{T: t, NotTerminal: true},
-				},
-				Logger: logger.TestLogger(t),
-			}
-			// Reset for test isolation
-			defer resetShellForTest(&shell)
+	shell := cmd.Shell{
+		Config:                 cfg,
+		FallbackAPIInitializer: apiPrompt,
+		Runner:                 cltest.EmptyRunner{},
+		AppFactory:             cltest.InstanceAppFactory{App: app},
+		KeyStoreAuthenticator: cmd.TerminalKeyStoreAuthenticator{
+			Prompter: &cltest.MockCountingPrompter{T: t, NotTerminal: true},
+		},
+		Logger: logger.TestLogger(t),
+	}
+	// Reset for test isolation
+	defer resetShellForTest(&shell)
 
-			set := flag.NewFlagSet("test", 0)
-			flagSetApplyFromAction(shell.RunNode, set, "")
-			require.NoError(t, set.Set("password", test.pwdfile))
+	set := flag.NewFlagSet("test", 0)
+	flagSetApplyFromAction(shell.RunNode, set, "")
+	require.NoError(t, set.Set("password", "../internal/fixtures/correct_password.txt"))
 
-			c := cli.NewContext(nil, set, nil)
+	c := cli.NewContext(nil, set, nil)
 
-			err := shell.BeforeNode(c)
+	err := shell.BeforeNode(c)
+	require.NoError(t, err, "BeforeNode should succeed")
+	assert.NotNil(t, shell.KeyStore)
+	assert.NotNil(t, shell.DS)
+	assert.NotNil(t, shell.LDB)
 
-			if test.expectStart {
-				require.NoError(t, err, "BeforeNode should succeed")
-				// Verify components are initialized
-				assert.NotNil(t, shell.KeyStore)
-				assert.NotNil(t, shell.DS)
-				assert.NotNil(t, shell.LDB)
+	// Now test RunNode with pre-authenticated keystore.
+	err = shell.RunNode(c)
+	require.NoError(t, err, "RunNode should succeed with authenticated keystore")
+	assert.Equal(t, 1, apiPrompt.Count, "API should be initialized")
 
-				// Now test RunNode with pre-authenticated keystore
-				// Note: RunNode will start the app but we expect it to work since keystore is authenticated
-				err = shell.RunNode(c)
-				require.NoError(t, err, "RunNode should succeed with authenticated keystore")
-				assert.Equal(t, 1, apiPrompt.Count, "API should be initialized")
-			} else {
-				require.Error(t, err, "BeforeNode should fail with incorrect password")
-				// Don't test RunNode if BeforeNode failed
-			}
-			// Clean up database if it was opened
-			if shell.LDB != nil {
-				cleanupErr := shell.AfterNode(c)
-				require.NoError(t, cleanupErr)
-			}
-		})
+	// Clean up database if it was opened
+	if shell.LDB != nil {
+		require.NoError(t, shell.AfterNode(c))
 	}
 }