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

i-norden · 2019-09-30T14:39:03Z

i-norden commented

2019-09-30 14:39:03 +00:00

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)

@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)

@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

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

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

🎉

statediff/service.go

elizabethengelman (Migrated from github.com) commented

2019-10-30 21:01:42 +00:00

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

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

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

@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

:)

i-norden reviewed 2019-11-04 14:40:09 +00:00

statediff/service.go

i-norden commented

2019-11-04 14:40:09 +00:00

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.

i-norden reviewed 2019-11-04 14:49:50 +00:00

statediff/testhelpers/mocks/blockchain.go

i-norden commented

2019-11-04 14:49:49 +00:00

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.

i-norden reviewed 2019-11-04 15:09:00 +00:00

statediff/testhelpers/mocks/blockchain.go

i-norden commented

2019-11-04 15:09:00 +00:00

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)