Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

VDB-1137: Statediff check #6

Merged
merged 1 commit into from
Jan 28, 2020

Conversation

elizabethengelman
Copy link

@elizabethengelman elizabethengelman commented Jan 27, 2020

In this release, the statediff.watchedaddresses flag will make sure that the statediff service only processes statediffs for given contract addresses. However, there is currently no filter to make sure that the statediff Payload is not empty before sending it over the subscription. The resulting behavior is that for all blocks that don't have a state diff for the given watched contract address, we end up sending a payload with an "empty" StateDiffRlp:

{ 
   BlockNumber: 60670 
   BlockHash:[52 100 238 122 70 68 56 0 8 232 58 201 18 35 163 192 8 15 70 129 97 112 179 25 60 45 208 183 232 215 212 205]
   CreatedAccounts:[]
   DeletedAccounts:[]
   UpdatedAccounts:[]
   encoded:[]
   err:<nil>
}

and see a lot of sending state diff payload to subscription... logs, which is misleading.

This issue has been fixed in the updated approach to statediffing: #5. Not sure if it makes sense to put in the time to fix the issue here in this statediffing approach as well or just wait until we start using the updated approach. 🤔

Copy link

@rmulhol rmulhol left a comment

Choose a reason for hiding this comment

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

Definitely on board with adding it to the docs, and creating a story to deal with it later (especially if we build out the diffing service further) :shipit:

elizabethengelman added a commit that referenced this pull request Jan 28, 2020
* Open a trie from the in-memory database

* Use a node's LeafKey as an identifier instead of the address

It was proving difficult to find look the address up from a given path
with a full node (sometimes the value wouldn't exist in the disk db).
So, instead, for now we are using the node's LeafKey with is a Keccak256
hash of the address, so if we know the address we can figure out which
LeafKey it matches up to.

* Make sure that statediff has been processed before pruning

* Use blockchain stateCache.OpenTrie for storage diffs

* Clean up log lines and remove unnecessary fields from builder

* Apply go fmt changes

* Add a sleep to the blockchain test

* Address PR comments

* Address PR comments
@elizabethengelman
Copy link
Author

New story to deal with filtering out empty payloads before sending to subscribers: https://makerdao.atlassian.net/browse/VDB-1157

@elizabethengelman elizabethengelman merged this pull request into statediffing-service Jan 28, 2020
@elizabethengelman elizabethengelman deleted the statediff-check branch January 28, 2020 18:49
@elizabethengelman elizabethengelman restored the statediff-check branch January 28, 2020 18:50
@elizabethengelman elizabethengelman deleted the statediff-check branch January 28, 2020 22:11
yaoandrew pushed a commit that referenced this pull request Jan 13, 2021
* internal/build: implement signify's signing func
* Add signify to the ci utility
* fix output file format
* Add unit test for signify
* holiman's + travis' feedback
* internal/build: verify signify's output
* crypto: move signify to common dir
* use go-minisign to verify binaries
* more holiman feedback
* crypto, ci: support minisign output
* only accept one-line trusted comments
* configurable untrusted comments
* code cleanup in tests
* revert to use ed25519 from the stdlib
* bug: fix for empty untrusted comments
* write timestamp as comment if trusted comment isn't present
* rename line checker to commentHasManyLines
* crypto: added signify fuzzer (#6)
* crypto: added signify fuzzer
* stuff
* crypto: updated signify fuzzer to fuzz comments
* crypto: repro signify crashes
* rebased fuzzer on build-signify branch
* hide fuzzer behind gofuzz build flag
* extract key data inside a single function
* don't treat \r as a newline
* travis: fix signing command line
* do not use an external binary in tests
* crypto: move signify to crypto/signify
* travis: fix formatting issue
* ci: fix linter build after package move

Co-authored-by: Marius van der Wijden <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants