Skip to content

Commit

Permalink
Merge pull request #2344 from subspace/farm-locking
Browse files Browse the repository at this point in the history
Add farm locking, preventing multiple farmers from using the same farm concurrently that leads to slashing and other issues
  • Loading branch information
nazar-pc authored Dec 18, 2023
2 parents 5a93c49 + 005a826 commit 66797a8
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
15 changes: 13 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/subspace-farmer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ criterion = { version = "0.5.1", default-features = false, features = ["rayon",
derive_more = "0.99.17"
event-listener-primitives = "2.0.1"
fdlimit = "0.3.0"
fs4 = "0.7.0"
futures = "0.3.29"
hex = { version = "0.4.3", features = ["serde"] }
jsonrpsee = { version = "0.16.3", features = ["client"] }
Expand Down
31 changes: 30 additions & 1 deletion crates/subspace-farmer/src/single_disk_farm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ impl SingleDiskFarmId {
}
}

/// Exclusive lock for single disk farm info file, ensuring no concurrent edits by cooperating processes is done
#[must_use = "Lock file must be kept around or as long as farm is used"]
pub struct SingleDiskFarmInfoLock {
_file: File,
}

/// Important information about the contents of the `SingleDiskFarm`
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -162,6 +168,15 @@ impl SingleDiskFarmInfo {
)
}

/// Try to acquire exclusive lock on the single disk farm info file, ensuring no concurrent edits by cooperating
/// processes is done
pub fn try_lock(directory: &Path) -> io::Result<SingleDiskFarmInfoLock> {
let file = File::open(directory.join(Self::FILE_NAME))?;
fs4::FileExt::try_lock_exclusive(&file)?;

Ok(SingleDiskFarmInfoLock { _file: file })
}

// ID of the farm
pub fn id(&self) -> &SingleDiskFarmId {
let Self::V0 { id, .. } = self;
Expand Down Expand Up @@ -287,6 +302,9 @@ pub enum SingleDiskFarmError {
/// Failed to open or create identity
#[error("Failed to open or create identity: {0}")]
FailedToOpenIdentity(#[from] IdentityError),
/// Farm is likely already in use, make sure no other farmer is using it
#[error("Farm is likely already in use, make sure no other farmer is using it: {0}")]
LikelyAlreadyInUse(io::Error),
// TODO: Make more variants out of this generic one
/// I/O error occurred
#[error("I/O error: {0}")]
Expand Down Expand Up @@ -378,6 +396,9 @@ pub enum SingleDiskFarmError {
/// Errors happening during scrubbing
#[derive(Debug, Error)]
pub enum SingleDiskFarmScrubError {
/// Farm is likely already in use, make sure no other farmer is using it
#[error("Farm is likely already in use, make sure no other farmer is using it: {0}")]
LikelyAlreadyInUse(io::Error),
/// Failed to determine file size
#[error("Failed to file size of {file}: {error}")]
FailedToDetermineFileSize {
Expand Down Expand Up @@ -559,6 +580,7 @@ pub struct SingleDiskFarm {
start_sender: Option<broadcast::Sender<()>>,
/// Sender that will be used to signal to background threads that they must stop
stop_sender: Option<broadcast::Sender<()>>,
_single_disk_farm_info_lock: SingleDiskFarmInfoLock,
}

impl Drop for SingleDiskFarm {
Expand Down Expand Up @@ -684,6 +706,9 @@ impl SingleDiskFarm {
}
};

let single_disk_farm_info_lock = SingleDiskFarmInfo::try_lock(&directory)
.map_err(SingleDiskFarmError::LikelyAlreadyInUse)?;

let pieces_in_sector = single_disk_farm_info.pieces_in_sector();
let sector_size = sector_size(pieces_in_sector);
let sector_metadata_size = SectorMetadataChecksummed::encoded_size();
Expand Down Expand Up @@ -745,7 +770,6 @@ impl SingleDiskFarm {
}
};

// TODO: Consider file locking to prevent other apps from modifying it
let mut metadata_file = OpenOptions::new()
.read(true)
.write(true)
Expand Down Expand Up @@ -1195,6 +1219,7 @@ impl SingleDiskFarm {
piece_reader,
start_sender: Some(start_sender),
stop_sender: Some(stop_sender),
_single_disk_farm_info_lock: single_disk_farm_info_lock,
};

Ok(farm)
Expand Down Expand Up @@ -1446,6 +1471,10 @@ impl SingleDiskFarm {
}
}
};

let _single_disk_farm_info_lock = SingleDiskFarmInfo::try_lock(directory)
.map_err(SingleDiskFarmScrubError::LikelyAlreadyInUse)?;

let identity = {
let file = directory.join(Identity::FILE_NAME);
info!(path = %file.display(), "Checking identity file");
Expand Down

0 comments on commit 66797a8

Please sign in to comment.