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

Disable ArchiveDB counter check #2016

Merged
merged 1 commit into from
Sep 1, 2016
Merged

Disable ArchiveDB counter check #2016

merged 1 commit into from
Sep 1, 2016

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Aug 29, 2016

In some rare cases it is possible for the same trie node to added twice in the same block. E.g.:
Transaction A adds a storage value for contract C at address 0 to 1. The nodes for this changes are committed to transaction memory overlay. Reference counter is set to 1.
Transaction B resets the same storage value to 0. Storage key is marked as modified in the account state cache.
Transaction B immediately changes the value back to 1. Storage key is reset to original value but still marked as modified.
Transaction B gets committed and the node gets re-inserted with counter set to 2.

I've considered keeping the original value in the state cache to detect such cases but this complicates the code and seems too expensive for such a rare case.

Fixes #2012

@arkpar arkpar added the A0-pleasereview 🤓 Pull request needs code review. label Aug 29, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 86.366% when pulling 4394c31 on archivedb-assert into 4389742 on master.

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 30, 2016
@rphmeier
Copy link
Contributor

rphmeier commented Aug 30, 2016

Seems like a bug in TrieDBMut (inserting an existing value writes the node twice).
Edit: fixed in #2025, but probably should still merge this.

@rphmeier rphmeier merged commit ca03cfa into master Sep 1, 2016
@debris debris deleted the archivedb-assert branch September 1, 2016 11:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants