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.
This commit is contained in:
Abdullahi Yunus
2024-11-01 12:24:04 +01:00
parent a51dfe9a7d
commit 84f039db45
5 changed files with 119 additions and 18 deletions

View File

@@ -54,12 +54,15 @@ type MultiFile struct {
// archiveDir is the directory where we'll store old channel backups. // archiveDir is the directory where we'll store old channel backups.
archiveDir string archiveDir string
// noBackupArchive indicates whether old backups should be deleted
// rather than archived.
noBackupArchive bool
} }
// NewMultiFile create a new multi-file instance at the target location on the // NewMultiFile create a new multi-file instance at the target location on the
// file system. // file system.
func NewMultiFile(fileName string) *MultiFile { func NewMultiFile(fileName string, noBackupArchive bool) *MultiFile {
// We'll our temporary backup file in the very same directory as the // We'll our temporary backup file in the very same directory as the
// main backup file. // main backup file.
backupFileDir := filepath.Dir(fileName) backupFileDir := filepath.Dir(fileName)
@@ -71,15 +74,17 @@ func NewMultiFile(fileName string) *MultiFile {
) )
return &MultiFile{ return &MultiFile{
fileName: fileName, fileName: fileName,
tempFileName: tempFileName, tempFileName: tempFileName,
archiveDir: archiveDir, archiveDir: archiveDir,
noBackupArchive: noBackupArchive,
} }
} }
// UpdateAndSwap will attempt write a new temporary backup file to disk with // UpdateAndSwap will attempt write a new temporary backup file to disk with
// the newBackup encoded, then atomically swap (via rename) the old file for // the newBackup encoded, then atomically swap (via rename) the old file for
// the new file by updating the name of the new file to the old. // the new file by updating the name of the new file to the old. It also checks
// if the old file should be archived first before swapping it.
func (b *MultiFile) UpdateAndSwap(newBackup PackedMulti) error { func (b *MultiFile) UpdateAndSwap(newBackup PackedMulti) error {
// If the main backup file isn't set, then we can't proceed. // If the main backup file isn't set, then we can't proceed.
if b.fileName == "" { if b.fileName == "" {
@@ -172,7 +177,13 @@ func (b *MultiFile) ExtractMulti(keyChain keychain.KeyRing) (*Multi, error) {
// specified archive directory, and copies the contents of the main backup file // specified archive directory, and copies the contents of the main backup file
// to the new archive file. // to the new archive file.
func (b *MultiFile) createArchiveFile() error { func (b *MultiFile) createArchiveFile() error {
// We check for old channel backup file first. // User can skip archiving of old backup files to save disk space.
if b.noBackupArchive {
log.Debug("Skipping archive of old backup file as configured")
return nil
}
// Check for old channel backup file.
oldFileExists := lnrpc.FileExists(b.fileName) oldFileExists := lnrpc.FileExists(b.fileName)
if !oldFileExists { if !oldFileExists {
log.Debug("No old channel backup file to archive") log.Debug("No old channel backup file to archive")

View File

@@ -48,7 +48,9 @@ func assertFileDeleted(t *testing.T, filePath string) {
// TestUpdateAndSwap test that we're able to properly swap out old backups on // TestUpdateAndSwap test that we're able to properly swap out old backups on
// disk with new ones. Additionally, after a swap operation succeeds, then each // disk with new ones. Additionally, after a swap operation succeeds, then each
// time we should only have the main backup file on disk, as the temporary file // time we should only have the main backup file on disk, as the temporary file
// has been removed. // has been removed. Finally, we check for noBackupArchive to ensure that the
// archive file is created when it's set to false, and not created when it's
// set to true.
func TestUpdateAndSwap(t *testing.T) { func TestUpdateAndSwap(t *testing.T) {
t.Parallel() t.Parallel()
@@ -58,7 +60,8 @@ func TestUpdateAndSwap(t *testing.T) {
fileName string fileName string
tempFileName string tempFileName string
oldTempExists bool oldTempExists bool
noBackupArchive bool
valid bool valid bool
}{ }{
@@ -92,9 +95,37 @@ func TestUpdateAndSwap(t *testing.T) {
), ),
valid: true, valid: true,
}, },
// Test with noBackupArchive set to true - should not create
// archive.
{
fileName: filepath.Join(
tempTestDir, DefaultBackupFileName,
),
tempFileName: filepath.Join(
tempTestDir, DefaultTempBackupFileName,
),
noBackupArchive: true,
valid: true,
},
// Test with v set to false - should create
// archive.
{
fileName: filepath.Join(
tempTestDir, DefaultBackupFileName,
),
tempFileName: filepath.Join(
tempTestDir, DefaultTempBackupFileName,
),
noBackupArchive: false,
valid: true,
},
} }
for i, testCase := range testCases { for i, testCase := range testCases {
backupFile := NewMultiFile(testCase.fileName) backupFile := NewMultiFile(
testCase.fileName, testCase.noBackupArchive,
)
// To start with, we'll make a random byte slice that'll pose // To start with, we'll make a random byte slice that'll pose
// as our packed multi backup. // as our packed multi backup.
@@ -160,6 +191,41 @@ func TestUpdateAndSwap(t *testing.T) {
// Additionally, we shouldn't be able to find the temp backup // Additionally, we shouldn't be able to find the temp backup
// file on disk, as it should be deleted each time. // file on disk, as it should be deleted each time.
assertFileDeleted(t, testCase.tempFileName) assertFileDeleted(t, testCase.tempFileName)
// Now check if archive was created when noBackupArchive is
// false.
archiveDir := filepath.Join(
filepath.Dir(testCase.fileName),
DefaultChanBackupArchiveDirName,
)
if !testCase.noBackupArchive {
files, err := os.ReadDir(archiveDir)
require.NoError(t, err)
require.Len(t, files, 1)
// Verify the archive contents match the previous
// backup.
archiveFile := filepath.Join(
archiveDir, files[0].Name(),
)
// The archived content should match the previous
// backup (newPackedMulti) that was just swapped out.
assertBackupMatches(t, archiveFile, newPackedMulti)
// Clean up the archive directory.
os.RemoveAll(archiveDir)
continue
}
// When noBackupArchive is true, no new archive file should be
// created. Note: In a real environment, the archive directory
// might exist with older backups before the feature is
// disabled, but for test simplicity (since we clean up the
// directory between test cases), we verify the directory
// doesn't exist at all.
require.NoDirExists(t, archiveDir)
} }
} }
@@ -238,7 +304,7 @@ func TestExtractMulti(t *testing.T) {
} }
for i, testCase := range testCases { for i, testCase := range testCases {
// First, we'll make our backup file with the specified name. // First, we'll make our backup file with the specified name.
backupFile := NewMultiFile(testCase.fileName) backupFile := NewMultiFile(testCase.fileName, false)
// With our file made, we'll now attempt to read out the // With our file made, we'll now attempt to read out the
// multi-file. // multi-file.
@@ -292,12 +358,18 @@ func TestCreateArchiveFile(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
tests := []struct { tests := []struct {
name string name string
setup func() setup func()
wantError bool noBackupArchive bool
wantError bool
}{ }{
{ {
name: "successful archive", name: "successful archive",
noBackupArchive: false,
},
{
name: "skip archive when disabled",
noBackupArchive: true,
}, },
{ {
name: "invalid archive directory permissions", name: "invalid archive directory permissions",
@@ -306,7 +378,8 @@ func TestCreateArchiveFile(t *testing.T) {
err := os.MkdirAll(archiveDir, 0500) err := os.MkdirAll(archiveDir, 0500)
require.NoError(t, err) require.NoError(t, err)
}, },
wantError: true, noBackupArchive: false,
wantError: true,
}, },
} }
@@ -318,7 +391,9 @@ func TestCreateArchiveFile(t *testing.T) {
tc.setup() tc.setup()
} }
multiFile := NewMultiFile(backupFile) multiFile := NewMultiFile(
backupFile, tc.noBackupArchive,
)
err := multiFile.createArchiveFile() err := multiFile.createArchiveFile()
if tc.wantError { if tc.wantError {
@@ -328,6 +403,13 @@ func TestCreateArchiveFile(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
// If archiving is disabled, verify no archive was
// created.
if tc.noBackupArchive {
require.NoDirExists(t, archiveDir)
return
}
// Verify archive exists and content matches. // Verify archive exists and content matches.
files, err := os.ReadDir(archiveDir) files, err := os.ReadDir(archiveDir)
require.NoError(t, err) require.NoError(t, err)

View File

@@ -360,6 +360,8 @@ type Config struct {
MaxPendingChannels int `long:"maxpendingchannels" description:"The maximum number of incoming pending channels permitted per peer."` MaxPendingChannels int `long:"maxpendingchannels" description:"The maximum number of incoming pending channels permitted per peer."`
BackupFilePath string `long:"backupfilepath" description:"The target location of the channel backup file"` BackupFilePath string `long:"backupfilepath" description:"The target location of the channel backup file"`
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."`
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"` 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"`
Bitcoin *lncfg.Chain `group:"Bitcoin" namespace:"bitcoin"` Bitcoin *lncfg.Chain `group:"Bitcoin" namespace:"bitcoin"`

View File

@@ -309,6 +309,10 @@
; Example: ; Example:
; backupfilepath=~/.lnd/data/chain/bitcoin/mainnet/channel.backup ; backupfilepath=~/.lnd/data/chain/bitcoin/mainnet/channel.backup
; When false (default), old channel backups are archived to a designated location.
; When true, old backups are simply replaced.
; no-backup-archive=false
; The maximum capacity of the block cache in bytes. Increasing this will result ; The maximum capacity of the block cache in bytes. Increasing this will result
; in more blocks being kept in memory but will increase performance when the ; in more blocks being kept in memory but will increase performance when the
; same block is required multiple times. ; same block is required multiple times.

View File

@@ -1648,7 +1648,9 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
chanNotifier: s.channelNotifier, chanNotifier: s.channelNotifier,
addrs: s.addrSource, addrs: s.addrSource,
} }
backupFile := chanbackup.NewMultiFile(cfg.BackupFilePath) backupFile := chanbackup.NewMultiFile(
cfg.BackupFilePath, cfg.NoBackupArchive,
)
startingChans, err := chanbackup.FetchStaticChanBackups( startingChans, err := chanbackup.FetchStaticChanBackups(
s.chanStateDB, s.addrSource, s.chanStateDB, s.addrSource,
) )