Skip to content

Commit 6277aa6

Browse files
valentinewallacevalentinewallace
committed
Abstract out utilities from FilesystemPersister
This will be used in upcoming commits for adding ChannelManager persistence.
1 parent 4e82003 commit 6277aa6

File tree

2 files changed

+145
-148
lines changed

2 files changed

+145
-148
lines changed

‎lightning-persister/src/lib.rs

Lines changed: 16 additions & 148 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
pub(crate) mod utils;
2+
13
extern crate lightning;
24
extern crate bitcoin;
35
extern crate libc;
@@ -10,7 +12,7 @@ use lightning::chain::transaction::OutPoint;
1012
use lightning::util::ser::{Writeable, Readable};
1113
use std::fs;
1214
use std::io::Error;
13-
use std::path::{Path, PathBuf};
15+
use crate::utils::DiskWriteable;
1416

1517
#[cfg(test)]
1618
use {
@@ -20,9 +22,6 @@ use {
2022
std::io::Cursor
2123
};
2224

23-
#[cfg(not(target_os = "windows"))]
24-
use std::os::unix::io::AsRawFd;
25-
2625
/// FilesystemPersister persists channel data on disk, where each channel's
2726
/// data is stored in a file named after its funding outpoint.
2827
///
@@ -39,12 +38,8 @@ pub struct FilesystemPersister {
3938
path_to_channel_data: String,
4039
}
4140

42-
trait DiskWriteable {
43-
fn write(&self, writer: &mut fs::File) -> Result<(), Error>;
44-
}
45-
4641
impl<ChanSigner: ChannelKeys + Writeable> DiskWriteable for ChannelMonitor<ChanSigner> {
47-
fn write(&self, writer: &mut fs::File) -> Result<(), Error> {
42+
fn write_to_file(&self, writer: &mut fs::File) -> Result<(), Error> {
4843
self.serialize_for_disk(writer)
4944
}
5045
}
@@ -58,41 +53,6 @@ impl FilesystemPersister {
5853
}
5954
}
6055

61-
fn get_full_filepath(&self, funding_txo: OutPoint) -> String {
62-
let mut path = PathBuf::from(&self.path_to_channel_data);
63-
path.push(format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index));
64-
path.to_str().unwrap().to_string()
65-
}
66-
67-
// Utility to write a file to disk.
68-
fn write_channel_data(&self, funding_txo: OutPoint, monitor: &dyn DiskWriteable) -> std::io::Result<()> {
69-
fs::create_dir_all(&self.path_to_channel_data)?;
70-
// Do a crazy dance with lots of fsync()s to be overly cautious here...
71-
// We never want to end up in a state where we've lost the old data, or end up using the
72-
// old data on power loss after we've returned.
73-
// The way to atomically write a file on Unix platforms is:
74-
// open(tmpname), write(tmpfile), fsync(tmpfile), close(tmpfile), rename(), fsync(dir)
75-
let filename = self.get_full_filepath(funding_txo);
76-
let tmp_filename = format!("{}.tmp", filename.clone());
77-
78-
{
79-
// Note that going by rust-lang/rust@d602a6b, on MacOS it is only safe to use
80-
// rust stdlib 1.36 or higher.
81-
let mut f = fs::File::create(&tmp_filename)?;
82-
monitor.write(&mut f)?;
83-
f.sync_all()?;
84-
}
85-
fs::rename(&tmp_filename, &filename)?;
86-
// Fsync the parent directory on Unix.
87-
#[cfg(not(target_os = "windows"))]
88-
{
89-
let path = Path::new(&filename).parent().unwrap();
90-
let dir_file = fs::OpenOptions::new().read(true).open(path)?;
91-
unsafe { libc::fsync(dir_file.as_raw_fd()); }
92-
}
93-
Ok(())
94-
}
95-
9656
#[cfg(test)]
9757
fn load_channel_data<ChanSigner: ChannelKeys + Readable + Writeable>(&self) ->
9858
Result<HashMap<OutPoint, ChannelMonitor<ChanSigner>>, ChannelMonitorUpdateErr> {
@@ -130,28 +90,18 @@ impl FilesystemPersister {
13090

13191
impl<ChanSigner: ChannelKeys + Readable + Writeable + Send + Sync> channelmonitor::Persist<ChanSigner> for FilesystemPersister {
13292
fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChanSigner>) -> Result<(), ChannelMonitorUpdateErr> {
133-
self.write_channel_data(funding_txo, monitor)
93+
let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
94+
utils::write_to_file(self.path_to_channel_data.clone(), filename, monitor)
13495
.map_err(|_| ChannelMonitorUpdateErr::PermanentFailure)
13596
}
13697

13798
fn update_persisted_channel(&self, funding_txo: OutPoint, _update: &ChannelMonitorUpdate, monitor: &ChannelMonitor<ChanSigner>) -> Result<(), ChannelMonitorUpdateErr> {
138-
self.write_channel_data(funding_txo, monitor)
99+
let filename = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
100+
utils::write_to_file(self.path_to_channel_data.clone(), filename, monitor)
139101
.map_err(|_| ChannelMonitorUpdateErr::PermanentFailure)
140102
}
141103
}
142104

143-
#[cfg(test)]
144-
impl Drop for FilesystemPersister {
145-
fn drop(&mut self) {
146-
// We test for invalid directory names, so it's OK if directory removal
147-
// fails.
148-
match fs::remove_dir_all(&self.path_to_channel_data) {
149-
Err(e) => println!("Failed to remove test persister directory: {}", e),
150-
_ => {}
151-
}
152-
}
153-
}
154-
155105
#[cfg(test)]
156106
mod tests {
157107
extern crate lightning;
@@ -160,8 +110,6 @@ mod tests {
160110
use bitcoin::blockdata::block::{Block, BlockHeader};
161111
use bitcoin::hashes::hex::FromHex;
162112
use bitcoin::Txid;
163-
use DiskWriteable;
164-
use Error;
165113
use lightning::chain::channelmonitor::{Persist, ChannelMonitorUpdateErr};
166114
use lightning::chain::transaction::OutPoint;
167115
use lightning::{check_closed_broadcast, check_added_monitors};
@@ -170,20 +118,22 @@ mod tests {
170118
use lightning::ln::msgs::ErrorAction;
171119
use lightning::util::enforcing_trait_impls::EnforcingChannelKeys;
172120
use lightning::util::events::{MessageSendEventsProvider, MessageSendEvent};
173-
use lightning::util::ser::Writer;
174121
use lightning::util::test_utils;
175122
use std::fs;
176-
use std::io;
177123
#[cfg(target_os = "windows")]
178124
use {
179125
lightning::get_event_msg,
180126
lightning::ln::msgs::ChannelMessageHandler,
181127
};
182128

183-
struct TestWriteable{}
184-
impl DiskWriteable for TestWriteable {
185-
fn write(&self, writer: &mut fs::File) -> Result<(), Error> {
186-
writer.write_all(&[42; 1])
129+
impl Drop for FilesystemPersister {
130+
fn drop(&mut self) {
131+
// We test for invalid directory names, so it's OK if directory removal
132+
// fails.
133+
match fs::remove_dir_all(&self.path_to_channel_data) {
134+
Err(e) => println!("Failed to remove test persister directory: {}", e),
135+
_ => {}
136+
}
187137
}
188138
}
189139

@@ -255,88 +205,6 @@ mod tests {
255205
check_persisted_data!(11);
256206
}
257207

258-
// Test that if the persister's path to channel data is read-only, writing
259-
// data to it fails. Windows ignores the read-only flag for folders, so this
260-
// test is Unix-only.
261-
#[cfg(not(target_os = "windows"))]
262-
#[test]
263-
fn test_readonly_dir() {
264-
let persister = FilesystemPersister::new("test_readonly_dir_persister".to_string());
265-
let test_writeable = TestWriteable{};
266-
let test_txo = OutPoint {
267-
txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(),
268-
index: 0
269-
};
270-
// Create the persister's directory and set it to read-only.
271-
let path = &persister.path_to_channel_data;
272-
fs::create_dir_all(path).unwrap();
273-
let mut perms = fs::metadata(path).unwrap().permissions();
274-
perms.set_readonly(true);
275-
fs::set_permissions(path, perms).unwrap();
276-
match persister.write_channel_data(test_txo, &test_writeable) {
277-
Err(e) => assert_eq!(e.kind(), io::ErrorKind::PermissionDenied),
278-
_ => panic!("Unexpected error message")
279-
}
280-
}
281-
282-
// Test failure to rename in the process of atomically creating a channel
283-
// monitor's file. We induce this failure by making the `tmp` file a
284-
// directory.
285-
// Explanation: given "from" = the file being renamed, "to" = the
286-
// renamee that already exists: Windows should fail because it'll fail
287-
// whenever "to" is a directory, and Unix should fail because if "from" is a
288-
// file, then "to" is also required to be a file.
289-
#[test]
290-
fn test_rename_failure() {
291-
let persister = FilesystemPersister::new("test_rename_failure".to_string());
292-
let test_writeable = TestWriteable{};
293-
let txid_hex = "8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be";
294-
let outp_idx = 0;
295-
let test_txo = OutPoint {
296-
txid: Txid::from_hex(txid_hex).unwrap(),
297-
index: outp_idx,
298-
};
299-
// Create the channel data file and make it a directory.
300-
let path = &persister.path_to_channel_data;
301-
fs::create_dir_all(format!("{}/{}_{}", path, txid_hex, outp_idx)).unwrap();
302-
match persister.write_channel_data(test_txo, &test_writeable) {
303-
Err(e) => {
304-
#[cfg(not(target_os = "windows"))]
305-
assert_eq!(e.kind(), io::ErrorKind::Other);
306-
#[cfg(target_os = "windows")]
307-
assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
308-
}
309-
_ => panic!("Unexpected error message")
310-
}
311-
}
312-
313-
// Test failure to create the temporary file in the persistence process.
314-
// We induce this failure by having the temp file already exist and be a
315-
// directory.
316-
#[test]
317-
fn test_tmp_file_creation_failure() {
318-
let persister = FilesystemPersister::new("test_tmp_file_creation_failure".to_string());
319-
let test_writeable = TestWriteable{};
320-
let txid_hex = "8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be";
321-
let outp_idx = 0;
322-
let test_txo = OutPoint {
323-
txid: Txid::from_hex(txid_hex).unwrap(),
324-
index: outp_idx,
325-
};
326-
// Create the tmp file and make it a directory.
327-
let path = &persister.path_to_channel_data;
328-
fs::create_dir_all(format!("{}/{}_{}.tmp", path, txid_hex, outp_idx)).unwrap();
329-
match persister.write_channel_data(test_txo, &test_writeable) {
330-
Err(e) => {
331-
#[cfg(not(target_os = "windows"))]
332-
assert_eq!(e.kind(), io::ErrorKind::Other);
333-
#[cfg(target_os = "windows")]
334-
assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
335-
}
336-
_ => panic!("Unexpected error message")
337-
}
338-
}
339-
340208
// Test that if the persister's path to channel data is read-only, writing a
341209
// monitor to it results in the persister returning a PermanentFailure.
342210
// Windows ignores the read-only flag for folders, so this test is Unix-only.

‎lightning-persister/src/utils.rs

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
use std::fs;
2+
use std::path::{Path, PathBuf};
3+
4+
#[cfg(not(target_os = "windows"))]
5+
use std::os::unix::io::AsRawFd;
6+
7+
pub(crate) trait DiskWriteable {
8+
fn write_to_file(&self, writer: &mut fs::File) -> Result<(), std::io::Error>;
9+
}
10+
11+
fn get_full_filepath(filepath: String, filename: String) -> String {
12+
let mut path = PathBuf::from(filepath);
13+
path.push(filename);
14+
path.to_str().unwrap().to_string()
15+
}
16+
17+
pub(crate) fn write_to_file(path: String, filename: String, data: &dyn DiskWriteable) -> std::io::Result<()> {
18+
fs::create_dir_all(path.clone())?;
19+
// Do a crazy dance with lots of fsync()s to be overly cautious here...
20+
// We never want to end up in a state where we've lost the old data, or end up using the
21+
// old data on power loss after we've returned.
22+
// The way to atomically write a file on Unix platforms is:
23+
// open(tmpname), write(tmpfile), fsync(tmpfile), close(tmpfile), rename(), fsync(dir)
24+
let filename = get_full_filepath(path, filename);
25+
let tmp_filename = format!("{}.tmp", filename.clone());
26+
27+
{
28+
// Note that going by rust-lang/rust@d602a6b, on MacOS it is only safe to use
29+
// rust stdlib 1.36 or higher.
30+
let mut f = fs::File::create(&tmp_filename)?;
31+
data.write_to_file(&mut f)?;
32+
f.sync_all()?;
33+
}
34+
fs::rename(&tmp_filename, &filename)?;
35+
// Fsync the parent directory on Unix.
36+
#[cfg(not(target_os = "windows"))]
37+
{
38+
let path = Path::new(&filename).parent().unwrap();
39+
let dir_file = fs::OpenOptions::new().read(true).open(path)?;
40+
unsafe { libc::fsync(dir_file.as_raw_fd()); }
41+
}
42+
Ok(())
43+
}
44+
45+
mod tests {
46+
use super::{DiskWriteable, get_full_filepath, write_to_file};
47+
use std::fs;
48+
use std::io;
49+
use std::io::Write;
50+
51+
struct TestWriteable{}
52+
impl DiskWriteable for TestWriteable {
53+
fn write_to_file(&self, writer: &mut fs::File) -> Result<(), io::Error> {
54+
writer.write_all(&[42; 1])
55+
}
56+
}
57+
58+
// Test that if the persister's path to channel data is read-only, writing
59+
// data to it fails. Windows ignores the read-only flag for folders, so this
60+
// test is Unix-only.
61+
#[cfg(not(target_os = "windows"))]
62+
#[test]
63+
fn test_readonly_dir() {
64+
let test_writeable = TestWriteable{};
65+
let filename = "test_readonly_dir_persister_filename".to_string();
66+
let path = "test_readonly_dir_persister_dir";
67+
fs::create_dir_all(path.to_string()).unwrap();
68+
let mut perms = fs::metadata(path.to_string()).unwrap().permissions();
69+
perms.set_readonly(true);
70+
fs::set_permissions(path.to_string(), perms).unwrap();
71+
match write_to_file(path.to_string(), filename, &test_writeable) {
72+
Err(e) => assert_eq!(e.kind(), io::ErrorKind::PermissionDenied),
73+
_ => panic!("Unexpected error message")
74+
}
75+
}
76+
77+
// Test failure to rename in the process of atomically creating a channel
78+
// monitor's file. We induce this failure by making the `tmp` file a
79+
// directory.
80+
// Explanation: given "from" = the file being renamed, "to" = the
81+
// renamee that already exists: Windows should fail because it'll fail
82+
// whenever "to" is a directory, and Unix should fail because if "from" is a
83+
// file, then "to" is also required to be a file.
84+
#[test]
85+
fn test_rename_failure() {
86+
let test_writeable = TestWriteable{};
87+
let filename = "test_rename_failure_filename";
88+
let path = "test_rename_failure_dir";
89+
// Create the channel data file and make it a directory.
90+
// fs::create_dir_all(format!("{}/{}_{}", path, txid_hex, outp_idx)).unwrap();
91+
fs::create_dir_all(get_full_filepath(path.to_string(), filename.to_string())).unwrap();
92+
// match persister.write_channel_data(test_txo, &test_writeable) {
93+
match write_to_file(path.to_string(), filename.to_string(), &test_writeable) {
94+
Err(e) => {
95+
#[cfg(not(target_os = "windows"))]
96+
assert_eq!(e.kind(), io::ErrorKind::Other);
97+
#[cfg(target_os = "windows")]
98+
assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
99+
}
100+
_ => panic!("Unexpected error message")
101+
}
102+
fs::remove_dir_all(path).unwrap();
103+
}
104+
105+
// Test failure to create the temporary file in the persistence process.
106+
// We induce this failure by having the temp file already exist and be a
107+
// directory.
108+
#[test]
109+
fn test_tmp_file_creation_failure() {
110+
let test_writeable = TestWriteable{};
111+
let filename = "test_tmp_file_creation_failure_filename".to_string();
112+
let path = "test_tmp_file_creation_failure_dir".to_string();
113+
114+
// Create the tmp file and make it a directory.
115+
// fs::create_dir_all(format!("{}/{}_{}.tmp", path, txid_hex, outp_idx)).unwrap();
116+
let tmp_path = get_full_filepath(path.clone(), format!("{}.tmp", filename.clone()));
117+
fs::create_dir_all(tmp_path).unwrap();
118+
match write_to_file(path, filename, &test_writeable) {
119+
Err(e) => {
120+
#[cfg(not(target_os = "windows"))]
121+
assert_eq!(e.kind(), io::ErrorKind::Other);
122+
#[cfg(target_os = "windows")]
123+
assert_eq!(e.kind(), io::ErrorKind::PermissionDenied);
124+
}
125+
_ => panic!("Unexpected error message")
126+
}
127+
}
128+
129+
}

0 commit comments

Comments
 (0)