(VDB-955) Associate diffs with headers #6
Conversation
rmulhol
commented
Nov 21, 2019
rmulhol
commented
- Queue diffs if matching header not found
- Queue diffs if matching header not found
rmulhol
requested review from
m0ar,
yaoandrew,
elizabethengelman and
Gslaughl
- replaces block number and block hash - make corresponding update to storage transformer
| if getHeaderErr != nil { | ||
| return 0, getHeaderErr | ||
| } | ||
| if diff.BlockHash.Hex() != header.Hash { |
There was a problem hiding this comment.
Thinking maybe this should convert header.Hash to a common.Hash and do this comparison with common.Hash instead of hex strings so that we don't run into pesky issues around the presence/absence of the 0x prefix
- When determining equality between diff and db - Prevents false negatives derived from presence/absence of 0x prefix
| ) | ||
|
|
||
| type ErrHeaderMismatch struct { |
| Eventually(func() utils.StorageDiff { | ||
| return mockQueue.AddPassedDiff | ||
| }).Should(Equal(csvDiff)) | ||
| close(done) |
There was a problem hiding this comment.
When I looked at this test quickly it took me a minute to understand why that when execution fails Execute doesn't isn't expected to return an error. Not sure it's necessary (maybe I should have just looked at the test closer 😜 ) - but do you think it's worth it to test that the transformer execution logged the error?
There was a problem hiding this comment.
👍 e5973ea
Not returning an error via Execute is a bit confusing, but I think it makes sense to continue in most failure scenarios. Definitely up for returning errors if we consider anything other than a fetcher error a showstopper
- verify logging when transformer execution fails
- Verify that rows aren't deleted when parsing queue fails
