Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement conflict reconciliation for private directories #431

Merged
merged 14 commits into from
Apr 25, 2024

Conversation

matheus23
Copy link
Member

@matheus23 matheus23 commented Apr 23, 2024

TODO:

  • [ ] Extract out the proptest generation from the public directory tests for re-usenot doing proptesting
  • Decide whether we want to prioritize merging towards directories in the private case, too (Yes)
  • [ ] Write proptests for private directory merging not doing proptesting
  • Write function documentation

Notes on not doing proptesting:

  • I learned a lot trying to state machine proptest private reconciliation.
  • When the proptests failed, not once have I found a bug in the actual code, but always in the reference state machine that I was comparing to.
  • To properly do this, I'd need to implement a close-to-full implementation of a file system CRDT. That's a lot of work. I didn't manage to find suitable simplifications yet. (I tried simplifying it to only files & obviously removing encryption, which works but is still a lot of work. I also tried "flattening" the file system to work on full paths only, but that throws away valuable information which makes it mismatch the actual file system.)
  • I've considered just testing the private file system against equivalent operations in the public file system. That'd be really close to working, except for two problematic cases: (1) if two directories are concurrently created, the private filesystem has to tie-break, because they're encrypted using different keys, wheras the public file system can simply merge and (2) the tie-breaking on file content works differently, because in the public case it can just tie-break on the file content CID, but in the private file system there is no such CID.

@matheus23 matheus23 self-assigned this Apr 23, 2024
@matheus23 matheus23 force-pushed the matheus23/private-conflict-reconciliation branch from 76a6c08 to b6475c5 Compare April 23, 2024 14:38
Copy link

codecov bot commented Apr 23, 2024

Codecov Report

Attention: Patch coverage is 57.64192% with 97 lines in your changes are missing coverage. Please review.

Project coverage is 56.05%. Comparing base (a0bf23c) to head (13bcec9).

❗ Current head 13bcec9 differs from pull request most recent head 41b105a. Consider uploading reports for the commit 41b105a to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #431      +/-   ##
==========================================
+ Coverage   54.93%   56.05%   +1.12%     
==========================================
  Files          57       58       +1     
  Lines        4156     4410     +254     
  Branches     1036     1108      +72     
==========================================
+ Hits         2283     2472     +189     
- Misses       1197     1201       +4     
- Partials      676      737      +61     
Files Coverage Δ
wnfs/src/private/forest/traits.rs 100.00% <ø> (ø)
wnfs/src/private/node/keys.rs 81.81% <100.00%> (+2.87%) ⬆️
wnfs/src/utils/common.rs 66.66% <100.00%> (+16.66%) ⬆️
wnfs/src/private/keys/privateref.rs 55.17% <0.00%> (ø)
wnfs/src/private/encrypted.rs 57.14% <0.00%> (-6.50%) ⬇️
wnfs/src/private/link.rs 56.66% <27.27%> (+2.23%) ⬆️
wnfs/src/private/node/header.rs 75.00% <69.44%> (-3.13%) ⬇️
wnfs/src/private/node/node.rs 70.76% <67.56%> (-0.32%) ⬇️
wnfs/src/private/file.rs 67.92% <53.06%> (-2.19%) ⬇️
wnfs/src/private/directory.rs 67.64% <54.87%> (-1.28%) ⬇️

... and 6 files with indirect coverage changes

@matheus23 matheus23 marked this pull request as ready for review April 25, 2024 13:59
@matheus23 matheus23 requested a review from a team as a code owner April 25, 2024 13:59
@matheus23 matheus23 enabled auto-merge (squash) April 25, 2024 14:00
@matheus23 matheus23 merged commit abfefef into main Apr 25, 2024
10 checks passed
@matheus23 matheus23 deleted the matheus23/private-conflict-reconciliation branch April 25, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant