Skip to content

Commit 55c29c8

Browse files
parkanparkan
committed
exclude Actor.PrivateKey from GORM, read via raw SQL in export-keys
1 parent fb7b9cc commit 55c29c8

4 files changed

Lines changed: 117 additions & 71 deletions

File tree

‎handler/wallet/export_keys.go‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,14 @@ type ExportKeysResult struct {
1717
Errors []string // actors that failed (logged but don't abort)
1818
}
1919

20+
// actor row with the legacy private_key column, used only by export-keys.
21+
// Actor.PrivateKey is gorm:"-" in the model so we need a local type for raw SQL.
22+
type legacyActorRow struct {
23+
ID string
24+
Address string
25+
PrivateKey string
26+
}
27+
2028
// exports private keys from the legacy Actor.PrivateKey column into the
2129
// filesystem keystore, creating Wallet records where needed.
2230
// idempotent -- skips actors whose address already has a Wallet record.
@@ -28,8 +36,8 @@ func ExportKeysHandler(
2836
) (*ExportKeysResult, error) {
2937
db = db.WithContext(ctx)
3038

31-
var actors []model.Actor
32-
err := db.Where("private_key != '' AND private_key IS NOT NULL").Find(&actors).Error
39+
var actors []legacyActorRow
40+
err := db.Raw("SELECT id, address, private_key FROM actors WHERE private_key != '' AND private_key IS NOT NULL").Scan(&actors).Error
3341
if err != nil {
3442
return nil, errors.Wrap(err, "failed to query actors with private keys")
3543
}
@@ -54,7 +62,7 @@ func ExportKeysHandler(
5462

5563
// exports a single actor's key to keystore, returns (true, "") on success,
5664
// (false, "") on skip, ("", errMsg) on failure
57-
func exportOneKey(db *gorm.DB, ks keystore.KeyStore, actor model.Actor) (exported bool, errMsg string) {
65+
func exportOneKey(db *gorm.DB, ks keystore.KeyStore, actor legacyActorRow) (exported bool, errMsg string) {
5866
// derive address to check for existing wallet
5967
addr, err := keystore.AddressFromExport(actor.PrivateKey)
6068
if err != nil {
@@ -114,7 +122,17 @@ func exportOneKey(db *gorm.DB, ks keystore.KeyStore, actor model.Actor) (exporte
114122
}
115123

116124
func HasPrivateKeyColumn(db *gorm.DB) bool {
117-
return db.Migrator().HasColumn(&model.Actor{}, "private_key")
125+
// can't use db.Migrator().HasColumn(&model.Actor{}, "private_key") because
126+
// the field is gorm:"-" -- GORM won't resolve the column name. query directly.
127+
dialect := db.Dialector.Name()
128+
var count int64
129+
switch dialect {
130+
case "sqlite":
131+
db.Raw("SELECT COUNT(*) FROM pragma_table_info('actors') WHERE name = 'private_key'").Scan(&count)
132+
default:
133+
db.Raw("SELECT COUNT(*) FROM information_schema.columns WHERE table_name = 'actors' AND column_name = 'private_key'").Scan(&count)
134+
}
135+
return count > 0
118136
}
119137

120138
// counts actors that still have a non-empty private_key in the database.

‎handler/wallet/export_keys_test.go‎

Lines changed: 56 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,38 @@ import (
1212
"gorm.io/gorm"
1313
)
1414

15+
// model without -:migration, used to simulate a legacy database where
16+
// AutoMigrate created the private_key column
17+
type legacyActor struct {
18+
ID string `gorm:"primaryKey;size:15"`
19+
Address string `gorm:"index"`
20+
PrivateKey string
21+
}
22+
23+
func (legacyActor) TableName() string { return "actors" }
24+
25+
// simulates a legacy database by running AutoMigrate with the old model
26+
// that includes private_key as a normal column
27+
func addLegacyColumn(t *testing.T, db *gorm.DB) {
28+
t.Helper()
29+
require.NoError(t, db.AutoMigrate(&legacyActor{}))
30+
}
31+
32+
// inserts an actor with a legacy private_key via the legacy model
33+
func createLegacyActor(t *testing.T, db *gorm.DB, id, address, privateKey string) {
34+
t.Helper()
35+
require.NoError(t, db.Create(&legacyActor{
36+
ID: id, Address: address, PrivateKey: privateKey,
37+
}).Error)
38+
}
39+
1540
func TestExportKeysHandler_ExportsActorKey(t *testing.T) {
1641
testutil.All(t, func(ctx context.Context, t *testing.T, db *gorm.DB) {
1742
ks, err := keystore.NewLocalKeyStore(t.TempDir())
1843
require.NoError(t, err)
1944

20-
// create an actor with a legacy private key
21-
actor := model.Actor{
22-
ID: "f01234",
23-
Address: testutil.TestWalletAddr,
24-
PrivateKey: testutil.TestPrivateKeyHex,
25-
}
26-
require.NoError(t, db.Create(&actor).Error)
45+
addLegacyColumn(t, db)
46+
createLegacyActor(t, db, "f01234", testutil.TestWalletAddr, testutil.TestPrivateKeyHex)
2747

2848
result, err := ExportKeysHandler(ctx, db, ks)
2949
require.NoError(t, err)
@@ -50,13 +70,8 @@ func TestExportKeysHandler_SkipsExistingWallet(t *testing.T) {
5070
ks, err := keystore.NewLocalKeyStore(t.TempDir())
5171
require.NoError(t, err)
5272

53-
// create actor with legacy key
54-
actor := model.Actor{
55-
ID: "f01234",
56-
Address: testutil.TestWalletAddr,
57-
PrivateKey: testutil.TestPrivateKeyHex,
58-
}
59-
require.NoError(t, db.Create(&actor).Error)
73+
addLegacyColumn(t, db)
74+
createLegacyActor(t, db, "f01234", testutil.TestWalletAddr, testutil.TestPrivateKeyHex)
6075

6176
// pre-import the wallet via normal import path
6277
h := DefaultHandler{}
@@ -83,13 +98,8 @@ func TestExportKeysHandler_SkipsExistingWalletLinksActor(t *testing.T) {
8398
ks, err := keystore.NewLocalKeyStore(t.TempDir())
8499
require.NoError(t, err)
85100

86-
// create actor with legacy key
87-
actor := model.Actor{
88-
ID: "f01234",
89-
Address: testutil.TestWalletAddr,
90-
PrivateKey: testutil.TestPrivateKeyHex,
91-
}
92-
require.NoError(t, db.Create(&actor).Error)
101+
addLegacyColumn(t, db)
102+
createLegacyActor(t, db, "f01234", testutil.TestWalletAddr, testutil.TestPrivateKeyHex)
93103

94104
// pre-import the wallet WITHOUT actor linkage
95105
h := DefaultHandler{}
@@ -116,12 +126,8 @@ func TestExportKeysHandler_Idempotent(t *testing.T) {
116126
ks, err := keystore.NewLocalKeyStore(t.TempDir())
117127
require.NoError(t, err)
118128

119-
actor := model.Actor{
120-
ID: "f01234",
121-
Address: testutil.TestWalletAddr,
122-
PrivateKey: testutil.TestPrivateKeyHex,
123-
}
124-
require.NoError(t, db.Create(&actor).Error)
129+
addLegacyColumn(t, db)
130+
createLegacyActor(t, db, "f01234", testutil.TestWalletAddr, testutil.TestPrivateKeyHex)
125131

126132
// first run exports
127133
r1, err := ExportKeysHandler(ctx, db, ks)
@@ -146,12 +152,12 @@ func TestExportKeysHandler_NoActorsWithKeys(t *testing.T) {
146152
ks, err := keystore.NewLocalKeyStore(t.TempDir())
147153
require.NoError(t, err)
148154

155+
addLegacyColumn(t, db)
149156
// actor with no private key
150-
actor := model.Actor{
157+
require.NoError(t, db.Create(&model.Actor{
151158
ID: "f09999",
152159
Address: "f1abc",
153-
}
154-
require.NoError(t, db.Create(&actor).Error)
160+
}).Error)
155161

156162
result, err := ExportKeysHandler(ctx, db, ks)
157163
require.NoError(t, err)
@@ -166,13 +172,8 @@ func TestExportKeysHandler_InvalidKeyRecordsError(t *testing.T) {
166172
ks, err := keystore.NewLocalKeyStore(t.TempDir())
167173
require.NoError(t, err)
168174

169-
// actor with garbage key
170-
actor := model.Actor{
171-
ID: "f05555",
172-
Address: "f1bad",
173-
PrivateKey: "not-a-valid-key",
174-
}
175-
require.NoError(t, db.Create(&actor).Error)
175+
addLegacyColumn(t, db)
176+
createLegacyActor(t, db, "f05555", "f1bad", "not-a-valid-key")
176177

177178
result, err := ExportKeysHandler(ctx, db, ks)
178179
require.NoError(t, err) // overall operation succeeds
@@ -189,13 +190,8 @@ func TestExportKeysHandler_MissingKeyFile(t *testing.T) {
189190
ks, err := keystore.NewLocalKeyStore(t.TempDir())
190191
require.NoError(t, err)
191192

192-
// create actor with legacy key
193-
actor := model.Actor{
194-
ID: "f01234",
195-
Address: testutil.TestWalletAddr,
196-
PrivateKey: testutil.TestPrivateKeyHex,
197-
}
198-
require.NoError(t, db.Create(&actor).Error)
193+
addLegacyColumn(t, db)
194+
createLegacyActor(t, db, "f01234", testutil.TestWalletAddr, testutil.TestPrivateKeyHex)
199195

200196
// create wallet record pointing to a nonexistent key file
201197
require.NoError(t, db.Create(&model.Wallet{
@@ -219,12 +215,8 @@ func TestExportKeysHandler_CorruptKeyFile(t *testing.T) {
219215
ks, err := keystore.NewLocalKeyStore(dir)
220216
require.NoError(t, err)
221217

222-
actor := model.Actor{
223-
ID: "f01234",
224-
Address: testutil.TestWalletAddr,
225-
PrivateKey: testutil.TestPrivateKeyHex,
226-
}
227-
require.NoError(t, db.Create(&actor).Error)
218+
addLegacyColumn(t, db)
219+
createLegacyActor(t, db, "f01234", testutil.TestWalletAddr, testutil.TestPrivateKeyHex)
228220

229221
// write garbage to the key file path
230222
corruptPath := dir + "/corrupt"
@@ -245,17 +237,22 @@ func TestExportKeysHandler_CorruptKeyFile(t *testing.T) {
245237
})
246238
}
247239

248-
func TestDropPrivateKeyColumn(t *testing.T) {
240+
func TestExportKeysHandler_AutoMigrateDoesNotRecreateColumn(t *testing.T) {
249241
testutil.All(t, func(ctx context.Context, t *testing.T, db *gorm.DB) {
250-
// column exists after AutoMigrate
251-
require.True(t, db.Migrator().HasColumn(&model.Actor{}, "private_key"))
242+
// AutoMigrate already ran (via testutil.All) -- column should NOT exist
243+
require.False(t, db.Migrator().HasColumn(&model.Actor{}, "private_key"),
244+
"AutoMigrate must not create private_key column (gorm:-:migration)")
252245

253-
// drop it
246+
// simulate legacy db, export, drop
247+
addLegacyColumn(t, db)
248+
require.True(t, HasPrivateKeyColumn(db))
254249
require.NoError(t, DropPrivateKeyColumn(db))
255-
require.False(t, db.Migrator().HasColumn(&model.Actor{}, "private_key"))
250+
require.False(t, HasPrivateKeyColumn(db))
256251

257-
// idempotent -- second call is a no-op
258-
require.NoError(t, DropPrivateKeyColumn(db))
252+
// re-run AutoMigrate -- must NOT re-add the column
253+
require.NoError(t, model.AutoMigrate(db))
254+
require.False(t, HasPrivateKeyColumn(db),
255+
"AutoMigrate must not re-add private_key column after drop")
259256
})
260257
}
261258

@@ -264,13 +261,8 @@ func TestDropPrivateKeyColumn_ExportThenDrop(t *testing.T) {
264261
ks, err := keystore.NewLocalKeyStore(t.TempDir())
265262
require.NoError(t, err)
266263

267-
// create actor with legacy key
268-
actor := model.Actor{
269-
ID: "f01234",
270-
Address: testutil.TestWalletAddr,
271-
PrivateKey: testutil.TestPrivateKeyHex,
272-
}
273-
require.NoError(t, db.Create(&actor).Error)
264+
addLegacyColumn(t, db)
265+
createLegacyActor(t, db, "f01234", testutil.TestWalletAddr, testutil.TestPrivateKeyHex)
274266

275267
// export
276268
result, err := ExportKeysHandler(ctx, db, ks)

‎model/migrate.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ var fkMigrations = []fkMigration{
7070
// Returns:
7171
// - An error if any issues arise during the process, otherwise nil.
7272
func AutoMigrate(db *gorm.DB) error {
73+
// pre-migration: rename legacy wallets table to actors before GORM
74+
// tries to reconcile the new Wallet model with the old table schema
75+
if err := renameLegacyWalletsTable(db); err != nil {
76+
return errors.Wrap(err, "failed to rename legacy wallets table")
77+
}
78+
7379
logger.Info("Auto migrating tables")
7480
err := db.AutoMigrate(Tables...)
7581
if err != nil {
@@ -487,6 +493,33 @@ func migrateWalletAssignments(db *gorm.DB) error {
487493
return nil
488494
}
489495

496+
// renameLegacyWalletsTable detects the pre-v0.8 schema where "wallets" was the
497+
// actor identity table (id=f0..., private_key) and renames it to "actors" so
498+
// AutoMigrate can create the new "wallets" table (keystore-backed model).
499+
// idempotent -- skips if wallets already has key_path (new schema) or if
500+
// actors already exists.
501+
func renameLegacyWalletsTable(db *gorm.DB) error {
502+
if !db.Migrator().HasTable("wallets") {
503+
return nil // fresh install
504+
}
505+
if db.Migrator().HasColumn(&Wallet{}, "key_path") {
506+
return nil // already migrated
507+
}
508+
// old wallets table has private_key but no key_path -- it's the actor table
509+
if db.Migrator().HasTable("actors") {
510+
return nil // actors table already exists, can't rename
511+
}
512+
513+
logger.Info("renaming legacy wallets table to actors")
514+
if err := db.Exec("ALTER TABLE wallets RENAME TO actors").Error; err != nil {
515+
return err
516+
}
517+
// drop old indexes that followed the rename -- they'd conflict with
518+
// indexes AutoMigrate creates on the new wallets table
519+
db.Exec("DROP INDEX IF EXISTS idx_wallets_address")
520+
return nil
521+
}
522+
490523
// DropAll removes all tables specified in the Tables slice from the database.
491524
//
492525
// This function is typically used during development or testing where a clean database

‎model/replication.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,12 @@ type Schedule struct {
179179
// actors we control have a Wallet with ActorID pointing here.
180180
// actors we don't control (counterparties) exist as bare records.
181181
type Actor struct {
182-
ID string `gorm:"primaryKey;size:15" json:"id"` // actor ID (f0...)
183-
Address string `gorm:"index" json:"address"` // filecoin address
184-
PrivateKey string `json:"privateKey,omitempty" table:"-"` // orphaned: run `singularity wallet export-keys` to migrate, then drop column
182+
ID string `gorm:"primaryKey;size:15" json:"id"` // actor ID (f0...)
183+
Address string `gorm:"index" json:"address"` // filecoin address
184+
// PrivateKey is excluded from GORM entirely so AutoMigrate won't
185+
// create the column and SELECTs won't reference it. The export-keys
186+
// handler reads it via raw SQL for legacy databases that still have it.
187+
185188
}
186189

187190
// GORM will rename "wallets" table to "actors" on AutoMigrate

0 commit comments

Comments
 (0)