Less fork choice #679
Merged
Less fork choice #679
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paulhauner
commented
Dec 7, 2019
| @@ -555,7 +571,7 @@ impl<T: BeaconChainTypes> SyncManager<T> { | |||
| self.network | |||
| .downvote_peer(parent_request.last_submitted_peer.clone()); | |||
| self.request_parent(parent_request); | |||
| return; | |||
| break; | |||
There was a problem hiding this comment.
@Age please confirm break is equivalent to return here and below
There was a problem hiding this comment.
We want the break, so we can optionally run fork choice.. but we only want to run fork choice if we have processed a block.
You need to check if successes > 0, which it looks like you have forgotten
AgeManning
requested changes
Dec 7, 2019
| @@ -555,7 +571,7 @@ impl<T: BeaconChainTypes> SyncManager<T> { | |||
| self.network | |||
| .downvote_peer(parent_request.last_submitted_peer.clone()); | |||
| self.request_parent(parent_request); | |||
| return; | |||
| break; | |||
There was a problem hiding this comment.
We want the break, so we can optionally run fork choice.. but we only want to run fork choice if we have processed a block.
You need to check if successes > 0, which it looks like you have forgotten
| // | ||
| // The `MessageHandler` would be the place to put this, however it doesn't seem | ||
| // to have a reference to the `BeaconChain`. I will leave this for future | ||
| // works. |
There was a problem hiding this comment.
Yeah agreed. The threading will be refactored in the network re-write. Will deal with this then
AgeManning
added a commit
that referenced
this pull request
Dec 9, 2019
* Apply clippy lints to beacon node * Remove unnecessary logging and correct formatting * Initial bones of load-balanced range-sync * Port bump meshsup tests * Further structure and network handling logic added * Basic structure, ignoring error handling * Correct max peers delay bug * Clean up and re-write message processor and sync manager * Restructure directory, correct type issues * Fix compiler issues * Completed first testing of new sync * Correct merge issues * Clean up warnings * Push attestation processed log down to dbg * Correct math error, downgraded logs * Add RPC error handling and improved syncing code * Add libp2p stream error handling and dropping of invalid peers * Lower logs * Add discovery tweak * Correct libp2p service locking * Handles peer disconnects for sync * Add logs downgrade discovery log * Less fork choice (#679) * Try merge in change to reduce fork choice calls * Remove fork choice from process block * Minor log fix * Check successes > 0 * Fix failing beacon chain tests * Fix re-org warnings * Fix mistake in prev commit * Range sync refactor - Introduces `ChainCollection` - Correct Disconnect node handling - Removes duplicate code * Various bug fixes * Remove unnecessary logs * Maintain syncing state in the transition from finalied to head * Improved disconnect handling * Adds forwards block interator * Notifies lighthouse on stream timeouts * Apply new gossipsub updates
paulhauner
added a commit
that referenced
this pull request
Dec 9, 2019
* Apply clippy lints to beacon node * Remove unnecessary logging and correct formatting * Initial bones of load-balanced range-sync * Port bump meshsup tests * Further structure and network handling logic added * Basic structure, ignoring error handling * Correct max peers delay bug * Clean up and re-write message processor and sync manager * Restructure directory, correct type issues * Fix compiler issues * Completed first testing of new sync * Correct merge issues * Clean up warnings * Push attestation processed log down to dbg * Add state enc/dec benches * Correct math error, downgraded logs * Add example for flamegraph * Use `PublicKeyBytes` for `Validator` * Ripple PublicKeyBytes change through codebase * Add RPC error handling and improved syncing code * Add benches, optimizations to store BeaconState * Store BeaconState in StorageContainer too * Optimize StorageContainer with std::mem magic * Add libp2p stream error handling and dropping of invalid peers * Lower logs * Update lcli to parse spec at boot, remove pycli * Fix issues when starting with mainnet spec * Set default spec to mainnet * Fix lcli --spec param * Add discovery tweak * Ensure ETH1_FOLLOW_DISTANCE is in YamlConfig * Set testnet ETH1_FOLLOW_DISTANCE to 16 * Fix rest_api tests * Set testnet min validator count * Update with new testnet dir * Remove some dbg, println * Add timeout when notifier waits for libp2p lock * Add validator count CLI flag to lcli contract deploy * Extend genesis delay time * Correct libp2p service locking * Update testnet dir * Add basic block/state caching on beacon chain * Add decimals display to notifier sync speed * Try merge in change to reduce fork choice calls * Remove fork choice from process block * Minor log fix * Check successes > 0 * Adds checkpoint cache * Stop storing the tree hash cache in the db * Handles peer disconnects for sync * Fix failing beacon chain tests * Change eth2_testnet_config tests to Mainnet * Add logs downgrade discovery log * Remove dedunant beacon state write * Fix re-org warnings * Use caching get methods in fork choice * Fix mistake in prev commit * Use caching state getting in state_by_slot * Add state.cacheless_clone * Less fork choice (#679) * Try merge in change to reduce fork choice calls * Remove fork choice from process block * Minor log fix * Check successes > 0 * Fix failing beacon chain tests * Fix re-org warnings * Fix mistake in prev commit * Attempt to improve attestation processing times * Introduce HeadInfo struct * Used cache tree hash for block processing * Use cached tree hash for block production too * Range sync refactor - Introduces `ChainCollection` - Correct Disconnect node handling - Removes duplicate code * Add more logging for DB * Various bug fixes * Remove unnecessary logs * Maintain syncing state in the transition from finalied to head * Improved disconnect handling * Add `Speedo` struct * Fix bugs in speedo * Fix bug in speedo * Fix rounding bug in speedo * Move code around, reduce speedo observation count * Adds forwards block interator * Fix inf NaN * Add first draft of validator onboarding * Update docs * Add documentation link to main README * Continue docs development * Update book readme * Update docs * Allow vc to run without testnet subcommand * Small change to onboarding docs * Tidy CLI help messages * Update docs * Add check to val client see if beacon node is synced * Attempt to fix NaN bug * Fix compile bug * Add notifier service to validator client * Re-order onboarding steps * Update deposit contract address * Update testnet dir * Add note about public eth1 node * Fix installation link * Set default eth1 endpoint to sigp * Fix broken test * Try fix eth1 cache locking * Be more specific about eth1 endpoint * Increase gas limit for deposit * Fix default deposit amount * Fix re-org log
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Addressed
NA
Proposed Changes
Removes fork choice from
BeaconChain::process_blockand calls it on demand instead.