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

fix: consider record split an error, handle it for regs #838

Merged
merged 4 commits into from Oct 16, 2023

Conversation

grumbach
Copy link
Member

@grumbach grumbach commented Oct 16, 2023

Description

Consider records split (when we receive different values from different nodes for the same key) an error instead of accepting the most popular one.
A split means:

  • for chunks we use GetQuorum::One so this error doesn't happen
  • for spends it means DOUBLE SPEND
  • for registers it means we could have 2 different versions that should be merged

This PR considers this situation to be an error, except for Registers where this error is handled on the client side, which merges the registers into one.

If we one day change the GetQuorum of chunks, it would be wise to handle this error accordingly by keeping the valid one (correct hash).


Summary generated by Reviewpad on 16 Oct 23 08:58 UTC

This pull request includes several changes across multiple files.

In the Cargo.toml file, a new dependency on "eyre" has been added.

In the sn_networking/src/error.rs file, there are changes related to imports, type aliases, and the addition of a new variant to the Error enum.

In another file, there are changes related to imports, method modifications, and formatting comments.

Lastly, in a different file, there are changes related to imports, the addition of new functions, and the implementation of a new struct.

Overall, the diff includes various changes improving dependency management, error handling, and register handling in the project.

@reviewpad reviewpad bot requested a review from bochaco October 16, 2023 08:58
@reviewpad reviewpad bot added Medium Medium sized PR waiting-for-review labels Oct 16, 2023
@grumbach grumbach force-pushed the record_split branch 2 times, most recently from b7d8ed2 to 0d363d5 Compare October 16, 2023 09:50
sn_networking/src/event.rs Outdated Show resolved Hide resolved
@grumbach grumbach added this pull request to the merge queue Oct 16, 2023
Merged via the queue into maidsafe:main with commit 1f9829a Oct 16, 2023
28 of 29 checks passed
@grumbach grumbach deleted the record_split branch October 16, 2023 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Medium Medium sized PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants