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

Swarm chunk integrity checks #3665

Merged
merged 4 commits into from Feb 13, 2017

Conversation

Projects
None yet
6 participants
@zelig
Copy link
Contributor

commented Feb 12, 2017

integrity checks on chunks incoming from peers as well as read from the disk database were incorrect.

This PR amends:

  • incoming chunks are checked for integrity always (not only when there is an open request)
  • chunks read from disk are checked for integrity and properly deleted before the process panics
  • implement an offline integrity checker as the cleandb swarm subcommand

This PR fixes the long-persistent INCORRECT _CHUNK _ENCODING and CONTENT_LENGTH_MISMATCH errors that were reproducible on the testnet.

This PR is a collaborative effort with @homotopycolimit and @nolash

@zelig zelig added this to the 1.5.9 milestone Feb 12, 2017

@zelig zelig self-assigned this Feb 12, 2017

@mention-bot

This comment has been minimized.

Copy link

commented Feb 12, 2017

@zelig, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fjl, @maran and @nolash to be potential reviewers.

@GitCop

This comment has been minimized.

Copy link

commented Feb 12, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 53aced33267b634cfade7ddd2ff83530448519c5
  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@zelig zelig force-pushed the ethersphere:swarm-chunk-integrity-checks branch 2 times, most recently Feb 12, 2017


chunkDbPath := args[0]
hash := storage.MakeHashFunc("SHA3")
dbStore, err := storage.NewDbStore(chunkDbPath, hash, 10000000, 0)

This comment has been minimized.

Copy link
@homotopycolimit

homotopycolimit Feb 12, 2017

Contributor

Why do we use 10000000 here? Is there any way to determine the "corrrect" size of the db? (or read the size from the swarm conf file?)

return
}

hasher := s.hashfunc()
hasher.Write(data)
hash := hasher.Sum(nil)
if !bytes.Equal(hash, key) {
s.db.Delete(getDataKey(index.Idx))

This comment has been minimized.

Copy link
@homotopycolimit

homotopycolimit Feb 12, 2017

Contributor

this was the line that caused the original database corruption -- deleting the data entry but not the key.

@zelig zelig added pr:review and removed in progress labels Feb 12, 2017

@@ -0,0 +1,23 @@
package main

This comment has been minimized.

Copy link
@fjl

fjl Feb 12, 2017

Contributor

missing GPLv3 header

This comment has been minimized.

Copy link
@fjl

fjl Feb 13, 2017

Contributor

Please put a newline between the header and package declaration to prevent showing the license header text in GoDoc.

cmd/swarm/main.go Outdated
<-sigc
glog.V(logger.Info).Infoln("Got sigterm, shutting down...")
stack.Stop()
}()

This comment has been minimized.

Copy link
@fjl

fjl Feb 12, 2017

Contributor

please remove this change from this PR

@zelig zelig force-pushed the ethersphere:swarm-chunk-integrity-checks branch Feb 13, 2017

@GitCop

This comment has been minimized.

Copy link

commented Feb 13, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 827e7ce313a9e3b42326f8b02de62676f549f279

  • Your commit message body contains a line that is longer than 80 characters

  • Commit: 560a242c2f258269565dd0e94d42f5d0c8a74227

  • Your commit message body contains a line that is longer than 80 characters

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@zelig

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2017

@fjl pushed the changes, thanks for the review

@zelig zelig force-pushed the ethersphere:swarm-chunk-integrity-checks branch Feb 13, 2017

@GitCop

This comment has been minimized.

Copy link

commented Feb 13, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 827e7ce313a9e3b42326f8b02de62676f549f279

  • Your commit message body contains a line that is longer than 80 characters

  • Commit: 560a242c2f258269565dd0e94d42f5d0c8a74227

  • Your commit message body contains a line that is longer than 80 characters

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

zelig and others added some commits Feb 11, 2017

swarm/storage: imrpoved integrity checking on chunks
* dbstore panics on corrupt chunk entry an prompts user to run cleandb
* memstore adds logging for garbage collection
* dbstore refactor item delete. correct partial deletes in Get

@zelig zelig force-pushed the ethersphere:swarm-chunk-integrity-checks branch to 043d3c5 Feb 13, 2017

@fjl

fjl approved these changes Feb 13, 2017

@fjl fjl merged commit e23e869 into ethereum:master Feb 13, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
ci/circleci Your tests passed on CircleCI!
Details
commit-message-check/gitcop All commit messages are valid
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@fjl fjl removed the pr:review label Feb 13, 2017

farazdagi added a commit to status-im/go-ethereum that referenced this pull request Feb 24, 2017

swarm/network: fix chunk integrity checks (ethereum#3665)
* swarm/network: integrity on incoming known chunks
* swarm/network: fix integrity check for incoming chunks
* swarm/storage: imrpoved integrity checking on chunks
* dbstore panics on corrupt chunk entry an prompts user to run cleandb
* memstore adds logging for garbage collection
* dbstore refactor item delete. correct partial deletes in Get
* cmd/swarm: added cleandb subcommand

@gbalint gbalint deleted the ethersphere:swarm-chunk-integrity-checks branch May 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.