cerc-io/go-ethereum
12
0
Fork 0
Code Issues 9 Pull Requests Packages Projects Releases 66 Wiki Activity

(VDB-309) rpc method to get statediff at specific block #16

Closed
i-norden wants to merge 118 commits from statediff_at into statediffing
pull from: statediff_at
merge into: :statediffing
Conversation 12 Commits 118 Files Changed 328
+18441 -2452
i-norden commented 2019-09-30 14:39:03 +00:00
Member
Copy Link
Copy Source

This is for vulcanizedb issue #123. This rpc method allows us to fetch the statediff at a particular block height, enabling us to backfill gaps in our data. Only works with a full node if the block height is within the pruning limit, otherwise an archival node is needed.

This is for vulcanizedb issue [#123](https://github.com/vulcanize/vulcanizedb/issues/123). This rpc method allows us to fetch the statediff at a particular block height, enabling us to backfill gaps in our data. Only works with a full node if the block height is within the pruning limit, otherwise an archival node is needed.
rmulhol (Migrated from github.com) reviewed 2019-09-30 14:39:03 +00:00
yaoandrew (Migrated from github.com) reviewed 2019-09-30 14:39:03 +00:00
aaizuss (Migrated from github.com) reviewed 2019-09-30 14:39:03 +00:00
Gslaughl (Migrated from github.com) reviewed 2019-09-30 14:39:03 +00:00
m0ar commented 2019-10-15 01:02:50 +00:00 (Migrated from github.com)
Copy Link
Copy Source

@elizabethengelman @rmulhol status on this? How long is the pruning limit?

@elizabethengelman @rmulhol status on this? How long is the pruning limit?
rmulhol commented 2019-10-16 15:23:30 +00:00 (Migrated from github.com)
Copy Link
Copy Source

@m0ar I think right now our thinking is that we could use the work by @i-norden in https://github.com/vulcanize/vulcanizedb/pull/157

@m0ar I think right now our thinking is that we could use the work by @i-norden in https://github.com/vulcanize/vulcanizedb/pull/157
👍 1
i-norden commented 2019-10-24 20:13:57 +00:00
Author
Member
Copy Link
Copy Source

Going to cut a new release for this branch. I'm not sure we need/want to merge this into statediffing, since that branch still has a PR open against the main geth branch.

Going to cut a new release for this branch. I'm not sure we need/want to merge this into `statediffing`, since that branch still has a PR open against the main geth branch.
elizabethengelman (Migrated from github.com) approved these changes 2019-10-30 21:16:44 +00:00
elizabethengelman (Migrated from github.com) left a comment
Copy Link
Copy Source

This looks awesome, nice job!

There was just one question about one of them mock methods, but it's not merge block.

This looks awesome, nice job! There was just one question about one of them mock methods, but it's not merge block.
statediff/service.go
@@ -270,2 +274,4 @@
sds.Unlock()
}
// StateDiffAt returns a statediff payload at the specific blockheight
elizabethengelman (Migrated from github.com) commented 2019-10-30 20:56:05 +00:00
Copy Link
Copy Source

🎉

🎉
statediff/service.go
elizabethengelman (Migrated from github.com) commented 2019-10-30 21:01:42 +00:00
Copy Link
Copy Source

I'm trying to think through if we'll ever want to get a diff at a block given it's hash instead of/in addition to it's number. 🤔

I'm trying to think through if we'll ever want to get a diff at a block given it's hash instead of/in addition to it's number. 🤔
statediff/testhelpers/mocks/blockchain.go
elizabethengelman (Migrated from github.com) commented 2019-10-30 21:16:20 +00:00
Copy Link
Copy Source

Not a big deal, but I'm a bit confused about how this works. In the test statediff/service_test.go#L234 we're setting testBlock1 to testBlock1.NumberU64() which I think is the child block. But then in the internals of this method we're returning a block from the parent blocks collection. 🤔

Not a big deal, but I'm a bit confused about how this works. In the test statediff/service_test.go#L234 we're setting `testBlock1` to `testBlock1.NumberU64()` which I think is the child block. But then in the internals of this method we're returning a block from the parent blocks collection. 🤔
m0ar (Migrated from github.com) approved these changes 2019-11-04 09:08:00 +00:00
m0ar (Migrated from github.com) left a comment
Copy Link
Copy Source

Nice!

A bit curious what happens if we request diffs for a block that is pruned, and how we can handle that sanely in vDB?

Nice! A bit curious what happens if we request diffs for a block that is pruned, and how we can handle that sanely in vDB?
statediff/service.go
m0ar (Migrated from github.com) commented 2019-11-04 09:04:42 +00:00
Copy Link
Copy Source

@elizabethengelman In theory it should be possible we ask for a state diff for a block that was just re-ordered, so maybe it's safer for us to ask for a hash? With reorg:d state diffs in vDB it shouldn't matter, since the diffs disappear on reorg anyway

@elizabethengelman In theory it should be possible we ask for a state diff for a block that was just re-ordered, so maybe it's safer for us to ask for a hash? With reorg:d state diffs in vDB it shouldn't matter, since the diffs disappear on reorg anyway
statediff/testhelpers/mocks/api.go
m0ar (Migrated from github.com) commented 2019-11-04 09:07:13 +00:00
Copy Link
Copy Source

:)

:)
statediff/service.go
i-norden commented 2019-11-04 14:40:09 +00:00
Author
Member
Copy Link
Copy Source

Performance wise fetching by hash is going to be a strain, we would need to fetch the block hash from Postgres for each block in the block range we want to fetch (which can be extremely large). Both backfillers are written to work with this end point at the moment. I think it would make sense to add a hash endpoint going forward though.

Performance wise fetching by hash is going to be a strain, we would need to fetch the block hash from Postgres for each block in the block range we want to fetch (which can be extremely large). Both backfillers are written to work with this end point at the moment. I think it would make sense to add a hash endpoint going forward though.
statediff/testhelpers/mocks/blockchain.go
i-norden commented 2019-11-04 14:49:49 +00:00
Author
Member
Copy Link
Copy Source

Sorry, the parent naming is confusing, all we are doing here is mapping blocks to return in the mock GetBlockByHash to the hashes we want to return them for. In the context of the this service/these tests the GetBlockByHash method is only called to fetch a parent block. Will update the naming.

Sorry, the `parent` naming is confusing, all we are doing here is mapping blocks to return in the mock `GetBlockByHash` to the hashes we want to return them for. In the context of the this service/these tests the `GetBlockByHash` method is only called to fetch a parent block. Will update the naming.
statediff/testhelpers/mocks/blockchain.go
i-norden commented 2019-11-04 15:09:00 +00:00
Author
Member
Copy Link
Copy Source

Parent block for the test is set here, since it is looked up by hash in StateDiffAt, the block for the block number we provide as an arg to StateDiffAt (the child block) is set here

Parent block for the test is set [here](https://github.com/vulcanize/go-ethereum/blob/statediff_at/statediff/service_test.go#L233), since it is looked up by hash in `StateDiffAt`, the block for the block number we provide as an arg to `StateDiffAt` (the child block) is set [here](https://github.com/vulcanize/go-ethereum/blob/statediff_at/statediff/service_test.go#L234)

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
i-norden
rmulhol
Gslaughl
aaizuss
yaoandrew
elizabethengelman
m0ar
Labels
Clear labels
bug
critical
duplicate
enhancement
epic
help wanted
in progress
invalid
low priority
question
rebase
v1
v5
wontfix
Copied from Github
An issue or PR manually copied from GitHub.
 Kind/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
The priority is critical
Priority
High
The priority is high
Priority
Low
The priority is low
Priority
Medium
The priority is medium
Reviewed
Confirmed
Issue has been confirmed
Reviewed
Duplicate
This issue or pull request already exists
Reviewed
Invalid
Invalid issue
Reviewed
Won't Fix
This issue won't be fixed
Status
Abandoned
Somebody has started to work on this but abandoned work
Status
Blocked
Something is blocking this issue or pull request
Status
Need More Info
Feedback is required to reproduce issue or to continue work
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
AFDudley (A. F. Dudley) Alex ashwin cerccicd ericlewis i-norden (Ian Norden) prathamesh (Prathamesh Musale) rachmaninovquar rjk-laconic roysc (Roy Crihfield) srw (srw) zramsay
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: cerc-io/go-ethereum#16