Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

evm: debug non-determinism #496

Merged
merged 3 commits into from
Sep 2, 2020
Merged

Conversation

fedekunze
Copy link
Contributor

Closes: #XXX

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Sep 2, 2020

Codecov Report

❗ No coverage uploaded for pull request base (development@c9639c3). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             development     #496   +/-   ##
==============================================
  Coverage               ?   70.15%           
==============================================
  Files                  ?       38           
  Lines                  ?     2520           
  Branches               ?        0           
==============================================
  Hits                   ?     1768           
  Misses                 ?      620           
  Partials               ?      132           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9639c3...84c233a. Read the comment docs.

// remove from the slice
s.stateObjects = append(s.stateObjects[:idx], s.stateObjects[idx+1:]...)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we were deleting the object from the slice but we weren't deleting the item from the map nor updating the corresponding indexes of the other elements.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is probably the fix for the issue yesterday, can remove the idx < len(csdb.stateObjects) check in getStateObject and setStateObject in #458

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! let me add a few tests so that we can merge this to development

@@ -685,27 +693,29 @@ func (csdb *CommitStateDB) Copy() *CommitStateDB {
ctx: csdb.ctx,
storeKey: csdb.storeKey,
accountKeeper: csdb.accountKeeper,
stateObjects: make([]stateEntry, len(csdb.journal.dirties)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the length/capacity and replaced it with append

address: dirty.address,
stateObject: csdb.stateObjects[idx].stateObject.deepCopy(state),
}
})
state.addressToObjectIndex[dirty.address] = len(state.stateObjects) - 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the addressToObjectIndex was not being updated either

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure why append should reduce by 1.

@@ -721,8 +731,9 @@ func (csdb *CommitStateDB) Copy() *CommitStateDB {
}

// copy pre-images
for hash, preimage := range csdb.preimages {
state.preimages[hash] = preimage
for i, preimageEntry := range csdb.preimages {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

replaced to slice to prevent non-determinism

@fedekunze fedekunze marked this pull request as ready for review September 2, 2020 14:42
@fedekunze fedekunze requested a review from noot as a code owner September 2, 2020 14:42
@fedekunze fedekunze requested a review from araskachoi September 2, 2020 14:43
@fedekunze fedekunze merged commit 26816e2 into development Sep 2, 2020
@fedekunze fedekunze deleted the debug-consensus-failure branch September 2, 2020 18:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants