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

fuzzing core/state package without skip slot cache #4883

Merged
merged 14 commits into from Feb 18, 2020
Merged

Conversation

shayzluf
Copy link
Contributor

@shayzluf shayzluf commented Feb 17, 2020

[Part of] #4882


Description

Fuzz test state package and fix errors that have been caught
will include skip slot cache in another pr

@codecov
Copy link

codecov bot commented Feb 17, 2020

Codecov Report

Merging #4883 into master will increase coverage by 9.29%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4883      +/-   ##
==========================================
+ Coverage   46.34%   55.64%   +9.29%     
==========================================
  Files         207      254      +47     
  Lines       15703    18900    +3197     
==========================================
+ Hits         7277    10516    +3239     
+ Misses       7286     6809     -477     
- Partials     1140     1575     +435     

@shayzluf shayzluf added the Ready For Review A pull request ready for code review label Feb 17, 2020
@shayzluf shayzluf changed the title fuzzing core/state package fuzzing core/state package without skip slot cache Feb 17, 2020
rauljordan
rauljordan previously approved these changes Feb 17, 2020
Copy link
Contributor

@rauljordan rauljordan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @shayzluf these are excellent finds and good nil protection

Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment on error log

@@ -65,6 +65,9 @@ func GenesisBeaconState(deposits []*ethpb.Deposit, genesisTime uint64, eth1Data
// Process initial deposits.
leaves := [][]byte{}
for _, deposit := range deposits {
if deposit == nil || deposit.Data == nil {
return nil, errors.New("eth1data contains nil or deposits with nil data field")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this just be deposits? where is eth1data?

@rauljordan
Copy link
Contributor

PTAL @terencechain

@shayzluf
Copy link
Contributor Author

thanks @rauljordan

terencechain
terencechain previously approved these changes Feb 17, 2020
beacon-chain/core/state/transition.go Outdated Show resolved Hide resolved
beacon-chain/core/state/transition_fuzz_test.go Outdated Show resolved Hide resolved
…te_no_cache

# Conflicts:
#	beacon-chain/state/getters.go
@shayzluf
Copy link
Contributor Author

@prestonvanloon PTAL

var chainStartDepositCount, currentTime uint64
for i := 0; i < 100000; i++ {
fuzzer.Fuzz(&chainStartDepositCount)
fuzzer.Fuzz(&chainStartDepositCount)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by mistake

Suggested change
fuzzer.Fuzz(&chainStartDepositCount)
fuzzer.Fuzz(&currentTime)

Copy link
Member

@prestonvanloon prestonvanloon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@shayzluf shayzluf merged commit 25308ef into master Feb 18, 2020
@delete-merged-branch delete-merged-branch bot deleted the fuzz_state_no_cache branch February 18, 2020 05:08
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 24, 2020
* fuzzing core/state package

* named error msg

* err comment

* terence feedback

* preston feedback

* preston feedback

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 28, 2020
* fuzzing core/state package

* named error msg

* err comment

* terence feedback

* preston feedback

* preston feedback

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants