Skip to content
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

Log Ethereum 1 deposits before chainstart #4499

Merged
merged 2 commits into from Jan 11, 2020
Merged

Log Ethereum 1 deposits before chainstart #4499

merged 2 commits into from Jan 11, 2020

Conversation

mcdee
Copy link
Contributor

@mcdee mcdee commented Jan 11, 2020

Provide occasional (every 512 deposits) log entries at info level to keep users informed that prysm is still alive and working when users start it up for the first time.

Fixes #4492 and #4498

@codecov
Copy link

codecov bot commented Jan 11, 2020

Codecov Report

Merging #4499 into master will decrease coverage by 23.01%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           master    #4499       +/-   ##
===========================================
- Coverage    43.6%   20.58%   -23.02%     
===========================================
  Files         193      192        -1     
  Lines       12899    13211      +312     
===========================================
- Hits         5625     2720     -2905     
- Misses       6343    10040     +3697     
+ Partials      931      451      -480

@@ -184,6 +184,13 @@ func (s *Service) ProcessDepositLog(ctx context.Context, depositLog gethTypes.Lo
"merkleTreeIndex": index,
}).Debug("Deposit registered from deposit contract")
validDepositsCount.Inc()
// Notify users what is going on, from time to time.
if !s.chainStartData.Chainstarted {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different than the following log?
https://github.com/prysmaticlabs/prysm/blob/master/beacon-chain/powchain/log_processing.go#L185

Do you want to remove that? They seem similar

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one you linked provides per-deposit information, including the public key of the deposit. This one provides more of a progress meter, showing only one report every 512 deposits and no detailed information. I think they both have a place.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. 👍

@prylabs-bulldozer prylabs-bulldozer bot merged commit ea12ffa into prysmaticlabs:master Jan 11, 2020
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 24, 2020
* Log Ethereum 1 deposits before chainstart
* Merge branch 'master' into logdeposits
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 28, 2020
* Log Ethereum 1 deposits before chainstart
* Merge branch 'master' into logdeposits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display More Useful Information to Node Users via Info Logs Before Sync
3 participants