Skip to content

Commit 73c7a3f

Browse files
committed
lnd+chanbackup: add lnd config flag
In this commit, we add the --no-backup-archive with a default as false. When set to true then previous channel backup file will not be archived but replaced. We also modify TestUpdateAndSwap test to make sure the new behaviour works as expected.
1 parent eeca262 commit 73c7a3f

File tree

5 files changed

+105
-23
lines changed

5 files changed

+105
-23
lines changed

Diff for: chanbackup/backupfile.go

+26-18
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,15 @@ type MultiFile struct {
5454

5555
// archiveDir is the directory where we'll store old channel backups.
5656
archiveDir string
57+
58+
// noBackupArchive indicates whether old backups should be deleted
59+
// rather than archived.
60+
noBackupArchive bool
5761
}
5862

5963
// NewMultiFile create a new multi-file instance at the target location on the
6064
// file system.
61-
func NewMultiFile(fileName string) *MultiFile {
62-
65+
func NewMultiFile(fileName string, noBackupArchive bool) *MultiFile {
6366
// We'll our temporary backup file in the very same directory as the
6467
// main backup file.
6568
backupFileDir := filepath.Dir(fileName)
@@ -71,15 +74,17 @@ func NewMultiFile(fileName string) *MultiFile {
7174
)
7275

7376
return &MultiFile{
74-
fileName: fileName,
75-
tempFileName: tempFileName,
76-
archiveDir: archiveDir,
77+
fileName: fileName,
78+
tempFileName: tempFileName,
79+
archiveDir: archiveDir,
80+
noBackupArchive: noBackupArchive,
7781
}
7882
}
7983

8084
// UpdateAndSwap will attempt write a new temporary backup file to disk with
8185
// the newBackup encoded, then atomically swap (via rename) the old file for
82-
// the new file by updating the name of the new file to the old.
86+
// the new file by updating the name of the new file to the old. It also checks
87+
// if the old file should be archived first before swapping it.
8388
func (b *MultiFile) UpdateAndSwap(newBackup PackedMulti) error {
8489
// If the main backup file isn't set, then we can't proceed.
8590
if b.fileName == "" {
@@ -131,18 +136,21 @@ func (b *MultiFile) UpdateAndSwap(newBackup PackedMulti) error {
131136
return fmt.Errorf("unable to close file: %w", err)
132137
}
133138

134-
// Archive the main backup file if it exists before replacing it with a
135-
// new one.
136-
backupExists := lnrpc.FileExists(b.fileName)
137-
if backupExists {
138-
log.Infof("Archiving old channel backup to %v", b.archiveDir)
139-
140-
err := createArchiveFile(
141-
b.archiveDir, b.fileName,
142-
)
143-
if err != nil {
144-
return fmt.Errorf("unable to archive old channel "+
145-
"backup file: %w", err)
139+
if !b.noBackupArchive {
140+
// Archive the main backup file if it exists before replacing
141+
// it with a new one.
142+
backupExists := lnrpc.FileExists(b.fileName)
143+
if backupExists {
144+
log.Infof("Archiving old channel backup to %v",
145+
b.archiveDir)
146+
147+
err := createArchiveFile(
148+
b.archiveDir, b.fileName,
149+
)
150+
if err != nil {
151+
return fmt.Errorf("unable to archive old "+
152+
"channel backup file: %w", err)
153+
}
146154
}
147155
}
148156

Diff for: chanbackup/backupfile_test.go

+70-4
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ func assertFileDeleted(t *testing.T, filePath string) {
4848
// TestUpdateAndSwap test that we're able to properly swap out old backups on
4949
// disk with new ones. Additionally, after a swap operation succeeds, then each
5050
// time we should only have the main backup file on disk, as the temporary file
51-
// has been removed.
51+
// has been removed. Finally, we check for noBackupArchive to ensure that the
52+
// archive file is created when it's set to false, and not created when it's
53+
// set to true.
5254
func TestUpdateAndSwap(t *testing.T) {
5355
t.Parallel()
5456

@@ -58,7 +60,8 @@ func TestUpdateAndSwap(t *testing.T) {
5860
fileName string
5961
tempFileName string
6062

61-
oldTempExists bool
63+
oldTempExists bool
64+
noBackupArchive bool
6265

6366
valid bool
6467
}{
@@ -92,9 +95,37 @@ func TestUpdateAndSwap(t *testing.T) {
9295
),
9396
valid: true,
9497
},
98+
99+
// Test with noBackupArchive set to true - should not create
100+
// archive.
101+
{
102+
fileName: filepath.Join(
103+
tempTestDir, DefaultBackupFileName,
104+
),
105+
tempFileName: filepath.Join(
106+
tempTestDir, DefaultTempBackupFileName,
107+
),
108+
noBackupArchive: true,
109+
valid: true,
110+
},
111+
112+
// Test with v set to false - should create
113+
// archive.
114+
{
115+
fileName: filepath.Join(
116+
tempTestDir, DefaultBackupFileName,
117+
),
118+
tempFileName: filepath.Join(
119+
tempTestDir, DefaultTempBackupFileName,
120+
),
121+
noBackupArchive: false,
122+
valid: true,
123+
},
95124
}
96125
for i, testCase := range testCases {
97-
backupFile := NewMultiFile(testCase.fileName)
126+
backupFile := NewMultiFile(
127+
testCase.fileName, testCase.noBackupArchive,
128+
)
98129

99130
// To start with, we'll make a random byte slice that'll pose
100131
// as our packed multi backup.
@@ -160,6 +191,41 @@ func TestUpdateAndSwap(t *testing.T) {
160191
// Additionally, we shouldn't be able to find the temp backup
161192
// file on disk, as it should be deleted each time.
162193
assertFileDeleted(t, testCase.tempFileName)
194+
195+
// Now check if archive was created when noBackupArchive is
196+
// false.
197+
archiveDir := filepath.Join(
198+
filepath.Dir(testCase.fileName),
199+
DefaultChanBackupArchiveDirName,
200+
)
201+
if !testCase.noBackupArchive {
202+
files, err := os.ReadDir(archiveDir)
203+
require.NoError(t, err)
204+
require.Len(t, files, 1)
205+
206+
// Verify the archive contents match the previous
207+
// backup.
208+
archiveFile := filepath.Join(
209+
archiveDir, files[0].Name(),
210+
)
211+
// The archived content should match the previous
212+
// backup (newPackedMulti) that was just swapped out.
213+
assertBackupMatches(t, archiveFile, newPackedMulti)
214+
215+
// Clean up the archive directory.
216+
os.RemoveAll(archiveDir)
217+
218+
continue
219+
}
220+
221+
// When noBackupArchive is true, no new archive file should be
222+
// created. Note: In a real environment, the archive directory
223+
// might exist with older backups before the feature is
224+
// disabled, but for test simplicity (since we clean up the
225+
// directory between test cases), we verify the directory
226+
// doesn't exist at all.
227+
require.NoDirExists(t, archiveDir)
228+
163229
}
164230
}
165231

@@ -238,7 +304,7 @@ func TestExtractMulti(t *testing.T) {
238304
}
239305
for i, testCase := range testCases {
240306
// First, we'll make our backup file with the specified name.
241-
backupFile := NewMultiFile(testCase.fileName)
307+
backupFile := NewMultiFile(testCase.fileName, false)
242308

243309
// With our file made, we'll now attempt to read out the
244310
// multi-file.

Diff for: config.go

+2
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,8 @@ type Config struct {
360360
MaxPendingChannels int `long:"maxpendingchannels" description:"The maximum number of incoming pending channels permitted per peer."`
361361
BackupFilePath string `long:"backupfilepath" description:"The target location of the channel backup file"`
362362

363+
NoBackupArchive bool `long:"no-backup-archive" description:"If set to true, channel backups will be deleted or replaced rather than being archived to a separate location."`
364+
363365
FeeURL string `long:"feeurl" description:"DEPRECATED: Use 'fee.url' option. Optional URL for external fee estimation. If no URL is specified, the method for fee estimation will depend on the chosen backend and network. Must be set for neutrino on mainnet." hidden:"true"`
364366

365367
Bitcoin *lncfg.Chain `group:"Bitcoin" namespace:"bitcoin"`

Diff for: sample-lnd.conf

+4
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,10 @@
309309
; Example:
310310
; backupfilepath=~/.lnd/data/chain/bitcoin/mainnet/channel.backup
311311

312+
; When false (default), old channel backups are archived to a designated location.
313+
; When true, old backups are simply replaced.
314+
; no-backup-archive=false
315+
312316
; The maximum capacity of the block cache in bytes. Increasing this will result
313317
; in more blocks being kept in memory but will increase performance when the
314318
; same block is required multiple times.

Diff for: server.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -1648,7 +1648,9 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
16481648
chanNotifier: s.channelNotifier,
16491649
addrs: s.addrSource,
16501650
}
1651-
backupFile := chanbackup.NewMultiFile(cfg.BackupFilePath)
1651+
backupFile := chanbackup.NewMultiFile(
1652+
cfg.BackupFilePath, cfg.NoBackupArchive,
1653+
)
16521654
startingChans, err := chanbackup.FetchStaticChanBackups(
16531655
s.chanStateDB, s.addrSource,
16541656
)

0 commit comments

Comments
 (0)