Merge pull request #1156 from ViktorT-11/2025-10-action-mig-handle-empty-RPCParamsJson

[sql-55] firewalldb: actions mig handle empty RPCParamsJson

Author: ViktorT-11

firewalldb/sql_migration.go

@@ -1131,6 +1131,11 @@ func validateMigratedAction(ctx context.Context, sqlTx SQLQueries,
 	// the SQL action has the full 8 bytes set.
 	overrideMacRootKeyID(migAction)
 
+	// If there are no RPCParamsJson set for the actions, that's represented
+	// differently in KVDB vs SQL. We therefore override the RPCParamsJson
+	// so that both actions represent them in the same way.
+	overrideRPCParamsJson(kvAction, migAction)
+
 	// Now that we have overridden the fields that are expected to differ
 	// between the original KVDB action and the migrated SQL action, we can
 	// compare the two actions to ensure that they match.
@@ -1606,3 +1611,14 @@ func overrideMacRootKeyID(action *Action) {
 		action.MacaroonRootKeyID = fn.Some(last32)
 	})
 }
+
+// overrideRPCParamsJson overrides the SQL action's RPCParamsJson in case they
+// are empty for the kvAction. In the SQL DB, empty RPCParamsJson are
+// represented as nil, while they are represented as an empty array in the
+// KVDB version. Therefore, this function overrides the SQL action's
+// RPCParamsJson to an empty array if they are nil.
+func overrideRPCParamsJson(kvAction *Action, sqlAction *Action) {
+	if len(kvAction.RPCParamsJson) == 0 && sqlAction.RPCParamsJson == nil {
+		sqlAction.RPCParamsJson = []byte{}
+	}
+}

firewalldb/sql_migration_test.go

@@ -474,6 +474,10 @@ func TestFirewallDBMigration(t *testing.T) {
 			name:       "action with no session or account",
 			populateDB: actionNoSessionOrAccount,
 		},
+		{
+			name:       "action with empty RPCParamsJson",
+			populateDB: actionEmptyRPCParamsJson,
+		},
 		{
 			name:       "action with session but no account",
 			populateDB: actionWithSessionNoAccount,
@@ -1268,6 +1272,32 @@ func actionNoSessionOrAccount(t *testing.T, ctx context.Context,
 	}
 }
 
+// actionEmptyRPCParamsJson adds an action which has no RPCParamsJson set.
+func actionEmptyRPCParamsJson(t *testing.T, ctx context.Context,
+	boltDB *BoltDB, _ session.Store, _ accounts.Store,
+	rStore *rootKeyMockStore) *expectedResult {
+
+	// As the action is not linked to any session, we add a random root
+	// key which we use as the macaroon identifier for the action.
+	// This simulates how similar actions would have been created in
+	// production.
+	rootKey := rStore.addRandomRootKey()
+
+	actionReq := testActionReq
+	actionReq.MacaroonRootKeyID = fn.Some(rootKey)
+	actionReq.SessionID = fn.None[session.ID]()
+	actionReq.AccountID = fn.None[accounts.AccountID]()
+	actionReq.RPCParamsJson = []byte{}
+
+	action := addAction(t, ctx, boltDB, &actionReq)
+
+	return &expectedResult{
+		kvEntries: []*kvEntry{},
+		privPairs: make(privacyPairs),
+		actions:   []*Action{action},
+	}
+}
+
 // actionWithSessionNoAccount adds an action which is linked a session but no
 // account.
 func actionWithSessionNoAccount(t *testing.T, ctx context.Context,
@@ -1967,6 +1997,15 @@ func addAction(t *testing.T, ctx context.Context, boltDB *BoltDB,
 	action.AccountID = actionReq.AccountID
 	action.MacaroonRootKeyID = actionReq.MacaroonRootKeyID
 
+	// In case the actionReq's RPCParamsJson wasn't set, we need to set the
+	// expected action's RPCParamsJson to nil for Sqlite tests as that's
+	// how such RPCParamsJson are represented in the Sqlite database, which
+	// the returned expected action should reflect. Note that for Postgres
+	// dbs, this param is stored as an empty array and not nil.
+	if len(actionReq.RPCParamsJson) == 0 && isSqlite {
+		action.RPCParamsJson = nil
+	}
+
 	return action
 }
 
@@ -2155,6 +2194,11 @@ func randomBytes(n int) []byte {
 // Keys are random strings like "key1", "key2"...
 // Values are random ints, floats, or strings.
 func randomJSON(n int) (string, error) {
+	// When 0 pairs are requested, we can return immediately.
+	if n <= 0 {
+		return "", nil
+	}
+
 	obj := make(map[string]any, n)
 	for i := 0; i < n; i++ {
 		key := fmt.Sprintf("key%d", i+1)

firewalldb/test_kvdb.go

@@ -12,6 +12,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// isSqlite is true if the test_db_sqlite build flag is set.
+var isSqlite = false
+
 // NewTestDB is a helper function that creates an BBolt database for testing.
 func NewTestDB(t *testing.T, clock clock.Clock) FirewallDBs {
 	return NewTestDBFromPath(t, t.TempDir(), clock)

firewalldb/test_postgres.go

@@ -9,6 +9,9 @@ import (
 	"github.com/lightningnetwork/lnd/clock"
 )
 
+// isSqlite is true if the test_db_sqlite build flag is set.
+var isSqlite = false
+
 // NewTestDB is a helper function that creates an BBolt database for testing.
 func NewTestDB(t *testing.T, clock clock.Clock) FirewallDBs {
 	return createStore(t, db.NewTestPostgresDB(t).BaseDB, clock)

firewalldb/test_sqlite.go

@@ -9,6 +9,9 @@ import (
 	"github.com/lightningnetwork/lnd/clock"
 )
 
+// isSqlite is true if the test_db_sqlite build flag is set.
+var isSqlite = true
+
 // NewTestDB is a helper function that creates an BBolt database for testing.
 func NewTestDB(t *testing.T, clock clock.Clock) FirewallDBs {
 	return createStore(t, db.NewTestSqliteDB(t).BaseDB, clock)