Merged
Conversation
* Remove hashing storage keys on storage diffs * Remove adding hashed storage keys to keys lookup * Remove keccaking storage key from storage backfill
Update geth statediff pkg references to filters pkg Update AccountDiffs for new geth patch Stream diffs from geth NewStateChanges subscription Remove support for old geth patches Remove dockerfile for extractDiffs command Use config file for extract diffs watched addresses
rmulhol
previously approved these changes
Nov 12, 2020
rmulhol
left a comment
There was a problem hiding this comment.
would like to take a longer look at this later but I'm fine with merging if it unblocks work in the transformers/actually building and testing the image
cmd/extractDiffs.go
Outdated
| var addresses []string | ||
| for contractName := range contracts { | ||
| address := viper.GetStringMapString("contract." + contractName)["address"] | ||
| addresses = append(addresses, address) |
There was a problem hiding this comment.
Wonder if it's worth tossing a panic in here if address == ""
Author
There was a problem hiding this comment.
Good call - just added the CreateFilterQuery method that we have in prod as part of fe275dc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update: here's the associated vdb-mcd-transformers PR: https://github.com/makerdao/vdb-mcd-transformers/pull/375/files
This branch brings over changes that we made in production to:
I still need to make a corresponding change in vdb-mcd-transformers to make sure that extractDiffs is run from that repo instead, and that the filterQuery is created and passed to the geth subscription. This may be tricky, because as we now know the way we were referencing versioned contract names in the config file don't parse properly using the toml parser, but I think we can probably find a workaround short of needing to update how contract names are referenced in the config file.
I think that other than the changes mentioned above in vdb-mcd-transformers that this may work, though I'd like to run it locally to verify.
libraries/shared/storage/types/diff.go- which hashes the addresses as it's formatting/decoding the diff it gets from the node.