Skip to content

Sync_with_hub cleanup wipes the whole hub_ops/ cache instead of the completed operation's files #82

Description

@nklyy

hub_ops/ is a wallet-wide flat download cache for hub files: files of all operations live side by side, filename = file_id (src/wallet/multisig.rs:1148-1170, tag v0.3.0-beta.28; same on dev). get_or_download_file checks the cache first and only then downloads from the hub.

The problem is in the cleanup step of sync_with_hub (src/wallet/multisig.rs:1938-1945): when the just-processed operation reaches Approved or Discarded, the code removes every file in the directory (read_dir + remove_file over the whole hub_ops/), not just the files belonging to that operation. Cached files of other pending operations disappear until they are re-downloaded.

// cleanup cache for approved or discarded operations
if op.status == OperationStatus::Approved || op.status == OperationStatus::Discarded {
    let ops_dir = self.get_hub_ops_dir();
    if ops_dir.exists() {
        for entry in fs::read_dir(&ops_dir)? {
            fs::remove_file(entry?.path())?;
        }
    }
}

Impact:

  • Race with concurrent readers. A long-lived service holds one wallet dir across several loops (sign, completion, sync). If one flow has already resolved a file path (get_or_download_files returns filepath) and a neighbouring sync_with_hub completes an unrelated operation at that moment, the file vanishes from under the reader: reading the PSBT or consignment fails mid-signing.
  • Unnecessary hub dependency at signing time. Even without the race, every pending operation's files must be re-downloaded; if the hub is unreachable at that moment, an operation whose files were already local is blocked.

Proposed fix (semantics unchanged, only "cleaning up someone else's files" goes away) - either:

  1. delete only the completed operation's files - OperationResponse.files already carries their file_ids, so the loop becomes for file in &op.files { remove get_cached_file_path(&file.file_id) }; or
  2. key the cache by subdirectory per operation_idx and remove that subdirectory.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions