-
Notifications
You must be signed in to change notification settings - Fork 4
Relay all block data + statediffs over full node websocket subscription #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* port statediff from https://github.com/jpmorganchase/quorum/blob/9b7fd9af8082795eeeb6863d9746f12b82dd5078/statediff/statediff.go; minor fixes * integrating state diff extracting, building, and persisting into geth processes * work towards persisting created statediffs in ipfs; based off github.com/vulcanize/eth-block-extractor * Add a state diff service * Remove diff extractor from blockchain * Update imports * Move statediff on/off check to geth cmd config * Update starting state diff service * Add debugging logs for creating diff * Add statediff extractor and builder tests and small refactoring * Start to write statediff to a CSV * Restructure statediff directory * Pull CSV publishing methods into their own file * Reformatting due to go fmt * Add gomega to vendor dir * Remove testing focuses * Update statediff tests to use golang test pkg instead of ginkgo - builder_test - extractor_test - publisher_test * Use hexutil.Encode instead of deprecated common.ToHex * Remove OldValue from DiffBigInt and DiffUint64 fields * Update builder test * Remove old storage value from updated accounts * Remove old values from created/deleted accounts * Update publisher to account for only storing current account values * Update service loop and fetching previous block * Update testing - remove statediff ginkgo test suite file - move mocks to their own dir * Updates per go fmt * Updates to tests * Pass statediff mode and path in through cli * Return filename from publisher * Remove some duplication in builder * Remove code field from state diff output this is the contract byte code, and it can still be obtained by querying the db by the codeHash * Consolidate acct diff structs for updated & updated/deleted accts * Include block number in csv filename * Clean up error logging * Cleanup formatting, spelling, etc * Address PR comments * Add contract address and storage value to csv * Refactor accumulating account row in csv publisher * Add DiffStorage struct * Add storage key to csv * Address PR comments * Fix publisher to include rows for accounts that don't have store updates * Update builder test after merging in release/1.8 * Update test contract to include storage on contract intialization - so that we're able to test that storage diffing works for created and deleted accounts (not just updated accounts). * Factor out a common trie iterator method in builder
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.
…publish + pass their proofs and paths along with them; adjust tests
elizabethengelman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking super cool, thanks for doing all this work! 🎉 🏅 💯
I wonder if we should open this PR against staging or statediffing? From what I remember, the statediffing branch was a bit more up-to-date than staging - was current with geth v1.8.19 (I think). It also makes sense to me that we include the state diff work separate from the work that gets this repo up-to-date.
statediff/builder/builder_test.go
Outdated
| // | ||
| // You should have received a copy of the GNU Lesser General Public License | ||
| // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
statediff/builder/struct.go
Outdated
| // SO each account diffs map is reall a map of shortnode keys to values | ||
| // Flatten to a slice of short nodes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These notes are helpful, but I'm wondering if we want to check them in here.
statediff/builder/builder_test.go
Outdated
| @@ -1,3 +1,19 @@ | |||
| // Copyright 2015 The go-ethereum Authors | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be updated to 2019?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, it probably should! The license date is different across a lot of the files but I think new files should have the year they were created.
| @@ -0,0 +1,95 @@ | |||
| // Copyright 2015 The go-ethereum Authors | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here - 2019?
| } | ||
|
|
||
| // Subscribe is the public method to setup a subscription that fires off state-diff payloads as they are created | ||
| func (api *PublicStateDiffAPI) Subscribe(ctx context.Context) (*rpc.Subscription, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep confusing the two concepts of subscription that we have within the statediff pkg:
I wonder if there's a way to distinguish these a bit more? Perhaps I am getting it confused because it's new, and I need to sit with the code a bit more. So this may be a non-issue 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm definitely not opposed to making this distinction more clear, the issue is that the rpc connection subscription method needs to be named Subscribe to work with this rpcClient method.
| log.Error("Failed to unsubscribe from the state diff service", err) | ||
| } | ||
| return | ||
| case <-notifier.Closed(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this Closed() method is deprecated in favor of an error channel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! So that means we can just drop this notifier.Closed() and the rpcSub.Err() should take care of everything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's how I'm interpreting it!
|
Hey @elizabethengelman thank you for the review! You're right about it being better to open up the PR against |
|
@i-norden Good call, I wonder if it makes sense keeping a branch around with the CSV state diff work, just in case we want to go back to that in the future. 🤔 Update: just pushed up this branch, just in case we'd like to go back an look at the csv work, once this socket work is merged into |
Handling BlockBatches in Geth at `SendBlockBatches` endpoint (eth_sendBlockBatches) Other: * Adding PR template * Adding ability to set timestamp and making blocks use configured timestamp * Adding ability to encode original tx nonce in calldata * Adding L1MessageSender to Contract Creation Txs
* Get basic getStorage/setStorage stubs working * Clean up tests * Add state_manager * Add StateManager set & getStorage * Add state mananger create function * Add get & increment nonce * Add getCodeContractBytecode * Add GetCodeContractHash * Add getCodeContractHash to the state manager * Add associateCodeContract to state manager * Pass the tests * go fmt * Add stateTransition to test with * Fix tests * Test deploying contract with transition state * Call executeTransaction on contract deployment * Added ExecutionManager deployment * Get contract deployments working * Cleanup logging * Get stubbed ExecutionManager working * Get a simple contract to deploy through the ExecutionManager * Refactor simpleAbiEncode * Revert unnecessary changes * Remove comments * Revert changes outside of this PR * Revert changes outside of this PR * Revert changes outside of this PR * Fix broken tests * Move OVM bytecode & ABI into constants * Add crazy printlines * Remove crazy comments * Add a bunch of debug printlns * Add helper fn for applying msgs to the EVM * Update ExecutionManager bytecode * Shim CREATE for EM to use correct addr * Add SimpleStorage test * Add the EM/SM to all new states * Force all txs to be routed through the EM * Remove unused files * Remove unused comments * Increment nonce after failed tx * Add debug statements * Use evm.Time for timestamp * Change EM deployment, fix broken tests, clean up * Add an OVM test & remove printlns * Fix lint errors & remove final printlns * Final cleanup--remove some comments * Limiting Geth to one transaction per block (#3) * Limiting Geth to one transaction per block * Adding TransitionBatchBuilder to build & submit rollup blocks * Adding L1MessageSender to Transaction (#4) * Adding L1MessageSender to Transaction * Adding logic to omit L1MessageSender in encoding / decoding when nil and never use it in hash computation Co-authored-by: ben-chain <ben@pseudonym.party> * Fixing Geth Tests (#6) Fixing broken tests, skipping tests we intentionally break, and configuring CI within Github Actions * Hex Trie -> Binary Trie (#7) *** Changing Hex Trie to Binary Trie *** Note: This changes and/or comments out a bunch of tests, so if things break down the line, this is likely the cause! * Ingest Block Batches (#8) Handling BlockBatches in Geth at `SendBlockBatches` endpoint (eth_sendBlockBatches) Other: * Adding PR template * Adding ability to set timestamp and making blocks use configured timestamp * Adding ability to encode original tx nonce in calldata * Adding L1MessageSender to Contract Creation Txs * Add L1MessageSender to Message * Increment nonce on CREATE failure * Fix bug where evm.Time=0 * Use state dump with hardcoded EM & SM addrs - ExecutionMgr address should always be 0x0000...dead0000 - StateMgr address should always be 0x0000...dead0001 * Move EM deployment into genesis block maker * Update EM contracts to latest version * Update EM to remove events * Fix the OVM tests * Skip an ungodly number of tests * Fix lint errors * Clean up logging * Cleanup more logs * Use local reference to state manager * Rename applyOvmToState(..) * Remove unneeded check * Clean up logging & add EM ABI panic * Add gas metering to SM & small refactor * Update core/vm/state_manager.go Co-authored-by: Kevin Ho <kevinjho1996@gmail.com> Co-authored-by: Mason Fischer <mason@kissr.co> Co-authored-by: Will Meister <william.k.meister@gmail.com> Co-authored-by: ben-chain <ben@pseudonym.party> Co-authored-by: Kevin Ho <kevinjho1996@gmail.com>
This build is focused on the seed node and I've gotten rid of the CSV stuff, because I think it is more likely to get accepted into Geth this way (there's no precedent for writing out CSVs anywhere else in Geth), so this might not be ideal for all purposes.
This is the branch that syncAndPublish needs to work with.
Also, I made a point to move files in the same commit but for some reason it looks like some history was still lost...