Abstract out disk-writing utilities from FilesystemPersister
authorValentine Wallace <vwallace@protonmail.com>
Thu, 19 Nov 2020 16:26:21 +0000 (11:26 -0500)
committerValentine Wallace <vwallace@protonmail.com>
Fri, 19 Feb 2021 20:00:43 +0000 (15:00 -0500)
lightning-persister/src/lib.rs
lightning-persister/src/util.rs [new file with mode: 0644]

index 7307ef0d17c99994e7ce9d3b3593ae5099195f3f..da5cba95e673d72ecfdb7ed90d1e5340f045195c 100644 (file)
@@ -1,8 +1,11 @@
+mod util;
+
 extern crate lightning;
 extern crate bitcoin;
 extern crate libc;
 
 use bitcoin::hashes::hex::ToHex;
+use crate::util::DiskWriteable;
 use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr};
 use lightning::chain::channelmonitor;
 use lightning::chain::keysinterface::ChannelKeys;
@@ -10,7 +13,6 @@ use lightning::chain::transaction::OutPoint;
 use lightning::util::ser::Writeable;
 use std::fs;
 use std::io::Error;
-use std::path::{Path, PathBuf};
 
 #[cfg(test)]
 use {
@@ -22,9 +24,6 @@ use {
        std::io::Cursor
 };
 
-#[cfg(not(target_os = "windows"))]
-use std::os::unix::io::AsRawFd;
-
 /// FilesystemPersister persists channel data on disk, where each channel's
 /// data is stored in a file named after its funding outpoint.
 ///
@@ -41,13 +40,9 @@ pub struct FilesystemPersister {
        path_to_channel_data: String,
 }
 
-trait DiskWriteable {
-       fn write(&self, writer: &mut fs::File) -> Result<(), Error>;
-}
-
 impl<ChanSigner: ChannelKeys> DiskWriteable for ChannelMonitor<ChanSigner> {
-       fn write(&self, writer: &mut fs::File) -> Result<(), Error> {
-               Writeable::write(self, writer)
+       fn write_to_file(&self, writer: &mut fs::File) -> Result<(), Error> {
+               self.write(writer)
        }
 }
 
@@ -60,41 +55,6 @@ impl FilesystemPersister {
                }
        }
 
-       fn get_full_filepath(&self, funding_txo: OutPoint) -> String {
-               let mut path = PathBuf::from(&self.path_to_channel_data);
-               path.push(format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index));
-               path.to_str().unwrap().to_string()
-       }
-
-       // Utility to write a file to disk.
-       fn write_channel_data(&self, funding_txo: OutPoint, monitor: &dyn DiskWriteable) -> std::io::Result<()> {
-               fs::create_dir_all(&self.path_to_channel_data)?;
-               // Do a crazy dance with lots of fsync()s to be overly cautious here...
-               // We never want to end up in a state where we've lost the old data, or end up using the
-               // old data on power loss after we've returned.
-               // The way to atomically write a file on Unix platforms is:
-               // open(tmpname), write(tmpfile), fsync(tmpfile), close(tmpfile), rename(), fsync(dir)
-               let filename = self.get_full_filepath(funding_txo);
-               let tmp_filename = format!("{}.tmp", filename.clone());
-
-               {
-                       // Note that going by rust-lang/rust@d602a6b, on MacOS it is only safe to use
-                       // rust stdlib 1.36 or higher.
-                       let mut f = fs::File::create(&tmp_filename)?;
-                       monitor.write(&mut f)?;
-                       f.sync_all()?;
-               }
-               fs::rename(&tmp_filename, &filename)?;
-               // Fsync the parent directory on Unix.
-               #[cfg(not(target_os = "windows"))]
-               {
-                       let path = Path::new(&filename).parent().unwrap();
-                       let dir_file = fs::OpenOptions::new().read(true).open(path)?;
-                       unsafe { libc::fsync(dir_file.as_raw_fd()); }
-               }
-               Ok(())
-       }
-
        #[cfg(test)]
        fn load_channel_data<Keys: KeysInterface>(&self, keys: &Keys) ->
                Result<HashMap<OutPoint, ChannelMonitor<Keys::ChanKeySigner>>, ChannelMonitorUpdateErr> {
@@ -132,28 +92,18 @@ impl FilesystemPersister {
 
 impl<ChanSigner: ChannelKeys + Send + Sync> channelmonitor::Persist<ChanSigner> for FilesystemPersister {
        fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChanSigner>) -> Result<(), ChannelMonitorUpdateErr> {
-               self.write_channel_data(funding_txo, monitor)
+               let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
+               util::write_to_file(self.path_to_channel_data.clone(), filename, monitor)
                  .map_err(|_| ChannelMonitorUpdateErr::PermanentFailure)
        }
 
        fn update_persisted_channel(&self, funding_txo: OutPoint, _update: &ChannelMonitorUpdate, monitor: &ChannelMonitor<ChanSigner>) -> Result<(), ChannelMonitorUpdateErr> {
-               self.write_channel_data(funding_txo, monitor)
+               let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
+               util::write_to_file(self.path_to_channel_data.clone(), filename, monitor)
                  .map_err(|_| ChannelMonitorUpdateErr::PermanentFailure)
        }
 }
 
-#[cfg(test)]
-impl Drop for FilesystemPersister {
-       fn drop(&mut self) {
-               // We test for invalid directory names, so it's OK if directory removal
-               // fails.
-               match fs::remove_dir_all(&self.path_to_channel_data) {
-                       Err(e) => println!("Failed to remove test persister directory: {}", e),
-                       _ => {}
-               }
-       }
-}
-
 #[cfg(test)]
 mod tests {
        extern crate lightning;
@@ -162,8 +112,6 @@ mod tests {
        use bitcoin::blockdata::block::{Block, BlockHeader};
        use bitcoin::hashes::hex::FromHex;
        use bitcoin::Txid;
-       use DiskWriteable;
-       use Error;
        use lightning::chain::channelmonitor::{Persist, ChannelMonitorUpdateErr};
        use lightning::chain::transaction::OutPoint;
        use lightning::{check_closed_broadcast, check_added_monitors};
@@ -171,20 +119,22 @@ mod tests {
        use lightning::ln::functional_test_utils::*;
        use lightning::ln::msgs::ErrorAction;
        use lightning::util::events::{MessageSendEventsProvider, MessageSendEvent};
-       use lightning::util::ser::Writer;
        use lightning::util::test_utils;
        use std::fs;
-       use std::io;
        #[cfg(target_os = "windows")]
        use {
                lightning::get_event_msg,
                lightning::ln::msgs::ChannelMessageHandler,
        };
 
-       struct TestWriteable{}
-       impl DiskWriteable for TestWriteable {
-               fn write(&self, writer: &mut fs::File) -> Result<(), Error> {
-                       writer.write_all(&[42; 1])
+       impl Drop for FilesystemPersister {
+               fn drop(&mut self) {
+                       // We test for invalid directory names, so it's OK if directory removal
+                       // fails.
+                       match fs::remove_dir_all(&self.path_to_channel_data) {
+                               Err(e) => println!("Failed to remove test persister directory: {}", e),
+                               _ => {}
+                       }
                }
        }
 
@@ -256,88 +206,6 @@ mod tests {
                check_persisted_data!(11);
        }
 
-       // Test that if the persister's path to channel data is read-only, writing
-       // data to it fails. Windows ignores the read-only flag for folders, so this
-       // test is Unix-only.
-       #[cfg(not(target_os = "windows"))]
-       #[test]
-       fn test_readonly_dir() {
-               let persister = FilesystemPersister::new("test_readonly_dir_persister".to_string());
-               let test_writeable = TestWriteable{};
-               let test_txo = OutPoint {
-                       txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(),
-                       index: 0
-               };
-               // Create the persister's directory and set it to read-only.
-               let path = &persister.path_to_channel_data;
-               fs::create_dir_all(path).unwrap();
-               let mut perms = fs::metadata(path).unwrap().permissions();
-               perms.set_readonly(true);
-               fs::set_permissions(path, perms).unwrap();
-               match persister.write_channel_data(test_txo, &test_writeable) {
-                       Err(e) => assert_eq!(e.kind(), io::ErrorKind::PermissionDenied),
-                       _ => panic!("Unexpected error message")
-               }
-       }
-
-       // Test failure to rename in the process of atomically creating a channel
-       // monitor's file. We induce this failure by making the `tmp` file a
-       // directory.
-       // Explanation: given "from" = the file being renamed, "to" = the
-       // renamee that already exists: Windows should fail because it'll fail
-       // whenever "to" is a directory, and Unix should fail because if "from" is a
-       // file, then "to" is also required to be a file.
-       #[test]
-       fn test_rename_failure() {
-               let persister = FilesystemPersister::new("test_rename_failure".to_string());
-               let test_writeable = TestWriteable{};
-               let txid_hex = "8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be";
-               let outp_idx = 0;
-               let test_txo = OutPoint {
-                       txid: Txid::from_hex(txid_hex).unwrap(),
-                       index: outp_idx,
-               };
-               // Create the channel data file and make it a directory.
-               let path = &persister.path_to_channel_data;
-               fs::create_dir_all(format!("{}/{}_{}", path, txid_hex, outp_idx)).unwrap();
-               match persister.write_channel_data(test_txo, &test_writeable) {
-                       Err(e) => {
-                               #[cfg(not(target_os = "windows"))]
-                               assert_eq!(e.kind(), io::ErrorKind::Other);
-                               #[cfg(target_os = "windows")]
-                               assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
-                       }
-                       _ => panic!("Unexpected error message")
-               }
-       }
-
-       // Test failure to create the temporary file in the persistence process.
-       // We induce this failure by having the temp file already exist and be a
-       // directory.
-       #[test]
-       fn test_tmp_file_creation_failure() {
-               let persister = FilesystemPersister::new("test_tmp_file_creation_failure".to_string());
-               let test_writeable = TestWriteable{};
-               let txid_hex = "8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be";
-               let outp_idx = 0;
-               let test_txo = OutPoint {
-                       txid: Txid::from_hex(txid_hex).unwrap(),
-                       index: outp_idx,
-               };
-               // Create the tmp file and make it a directory.
-               let path = &persister.path_to_channel_data;
-               fs::create_dir_all(format!("{}/{}_{}.tmp", path, txid_hex, outp_idx)).unwrap();
-               match persister.write_channel_data(test_txo, &test_writeable) {
-                       Err(e) => {
-                               #[cfg(not(target_os = "windows"))]
-                               assert_eq!(e.kind(), io::ErrorKind::Other);
-                               #[cfg(target_os = "windows")]
-                               assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
-                       }
-                       _ => panic!("Unexpected error message")
-               }
-       }
-
        // Test that if the persister's path to channel data is read-only, writing a
        // monitor to it results in the persister returning a PermanentFailure.
        // Windows ignores the read-only flag for folders, so this test is Unix-only.
diff --git a/lightning-persister/src/util.rs b/lightning-persister/src/util.rs
new file mode 100644 (file)
index 0000000..0e39998
--- /dev/null
@@ -0,0 +1,149 @@
+use std::fs;
+use std::path::{Path, PathBuf};
+
+#[cfg(not(target_os = "windows"))]
+use std::os::unix::io::AsRawFd;
+
+pub(crate) trait DiskWriteable {
+       fn write_to_file(&self, writer: &mut fs::File) -> Result<(), std::io::Error>;
+}
+
+pub fn get_full_filepath(filepath: String, filename: String) -> String {
+       let mut path = PathBuf::from(filepath);
+       path.push(filename);
+       path.to_str().unwrap().to_string()
+}
+
+#[allow(bare_trait_objects)]
+pub(crate) fn write_to_file<D: DiskWriteable>(path: String, filename: String, data: &D) -> std::io::Result<()> {
+       fs::create_dir_all(path.clone())?;
+       // Do a crazy dance with lots of fsync()s to be overly cautious here...
+       // We never want to end up in a state where we've lost the old data, or end up using the
+       // old data on power loss after we've returned.
+       // The way to atomically write a file on Unix platforms is:
+       // open(tmpname), write(tmpfile), fsync(tmpfile), close(tmpfile), rename(), fsync(dir)
+       let filename_with_path = get_full_filepath(path, filename);
+       let tmp_filename = format!("{}.tmp", filename_with_path);
+
+       {
+               // Note that going by rust-lang/rust@d602a6b, on MacOS it is only safe to use
+               // rust stdlib 1.36 or higher.
+               let mut f = fs::File::create(&tmp_filename)?;
+               data.write_to_file(&mut f)?;
+               f.sync_all()?;
+       }
+       fs::rename(&tmp_filename, &filename_with_path)?;
+       // Fsync the parent directory on Unix.
+       #[cfg(not(target_os = "windows"))]
+       {
+               let path = Path::new(&filename_with_path).parent().unwrap();
+               let dir_file = fs::OpenOptions::new().read(true).open(path)?;
+               unsafe { libc::fsync(dir_file.as_raw_fd()); }
+       }
+       Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+       use super::{DiskWriteable, get_full_filepath, write_to_file};
+       use std::fs;
+       use std::io;
+       use std::io::Write;
+
+       struct TestWriteable{}
+       impl DiskWriteable for TestWriteable {
+               fn write_to_file(&self, writer: &mut fs::File) -> Result<(), io::Error> {
+                       writer.write_all(&[42; 1])
+               }
+       }
+
+       // Test that if the persister's path to channel data is read-only, writing
+       // data to it fails. Windows ignores the read-only flag for folders, so this
+       // test is Unix-only.
+       #[cfg(not(target_os = "windows"))]
+       #[test]
+       fn test_readonly_dir() {
+               let test_writeable = TestWriteable{};
+               let filename = "test_readonly_dir_persister_filename".to_string();
+               let path = "test_readonly_dir_persister_dir";
+               fs::create_dir_all(path.to_string()).unwrap();
+               let mut perms = fs::metadata(path.to_string()).unwrap().permissions();
+               perms.set_readonly(true);
+               fs::set_permissions(path.to_string(), perms).unwrap();
+               match write_to_file(path.to_string(), filename, &test_writeable) {
+                       Err(e) => assert_eq!(e.kind(), io::ErrorKind::PermissionDenied),
+                       _ => panic!("Unexpected error message")
+               }
+       }
+
+       // Test failure to rename in the process of atomically creating a channel
+       // monitor's file. We induce this failure by making the `tmp` file a
+       // directory.
+       // Explanation: given "from" = the file being renamed, "to" = the
+       // renamee that already exists: Windows should fail because it'll fail
+       // whenever "to" is a directory, and Unix should fail because if "from" is a
+       // file, then "to" is also required to be a file.
+       #[test]
+       fn test_rename_failure() {
+               let test_writeable = TestWriteable{};
+               let filename = "test_rename_failure_filename";
+               let path = "test_rename_failure_dir";
+               // Create the channel data file and make it a directory.
+               fs::create_dir_all(get_full_filepath(path.to_string(), filename.to_string())).unwrap();
+               match write_to_file(path.to_string(), filename.to_string(), &test_writeable) {
+                       Err(e) => {
+                               #[cfg(not(target_os = "windows"))]
+                               assert_eq!(e.kind(), io::ErrorKind::Other);
+                               #[cfg(target_os = "windows")]
+                               assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
+                       }
+                       _ => panic!("Unexpected error message")
+               }
+               fs::remove_dir_all(path).unwrap();
+       }
+
+       #[test]
+       fn test_diskwriteable_failure() {
+               struct FailingWriteable {}
+               impl DiskWriteable for FailingWriteable {
+                       fn write_to_file(&self, _writer: &mut fs::File) -> Result<(), std::io::Error> {
+                               Err(std::io::Error::new(std::io::ErrorKind::Other, "expected failure"))
+                       }
+               }
+
+               let filename = "test_diskwriteable_failure";
+               let path = "test_diskwriteable_failure_dir";
+               let test_writeable = FailingWriteable{};
+               match write_to_file(path.to_string(), filename.to_string(), &test_writeable) {
+                       Err(e) => {
+                               assert_eq!(e.kind(), std::io::ErrorKind::Other);
+                               assert_eq!(e.get_ref().unwrap().to_string(), "expected failure");
+                       },
+                       _ => panic!("unexpected result")
+               }
+               fs::remove_dir_all(path).unwrap();
+       }
+
+       // Test failure to create the temporary file in the persistence process.
+       // We induce this failure by having the temp file already exist and be a
+       // directory.
+       #[test]
+       fn test_tmp_file_creation_failure() {
+               let test_writeable = TestWriteable{};
+               let filename = "test_tmp_file_creation_failure_filename".to_string();
+               let path = "test_tmp_file_creation_failure_dir".to_string();
+
+               // Create the tmp file and make it a directory.
+               let tmp_path = get_full_filepath(path.clone(), format!("{}.tmp", filename.clone()));
+               fs::create_dir_all(tmp_path).unwrap();
+               match write_to_file(path, filename, &test_writeable) {
+                       Err(e) => {
+                               #[cfg(not(target_os = "windows"))]
+                               assert_eq!(e.kind(), io::ErrorKind::Other);
+                               #[cfg(target_os = "windows")]
+                               assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
+                       }
+                       _ => panic!("Unexpected error message")
+               }
+       }
+}