Fix: Log validator index, status and pubkey each epoch #5161

maschad · 2023-02-17T00:19:11Z

packages/validator/src/services/indices.ts

nflaig

looks good to me from a technical point of view, I am not really opinionated on how the log message should be displayed. Since this is a UX topic makes sense to get actual user feedback so we might just wanna push this out asap.

maschad · 2023-02-17T21:09:27Z

Would be good to get @g11tech opinion on this about log formatting

wemeetagain

#4785 says
every epoch, we should log the pubkey count with the validator grouped by status
as lighthouse is doing

I don't think this is doing that. Also don't think this is necessarily the right place to add the log.

packages/validator/src/services/indices.ts

g11tech · 2023-02-18T16:45:17Z

#4785 says every epoch, we should log the pubkey count with the validator grouped by status as lighthouse is doing

I don't think this is doing that. Also don't think this is necessarily the right place to add the log.

yes #4785 is meant to do as suggested by @wemeetagain 👍

…4785)

…hainSafe#4785)

packages/validator/src/services/indices.ts

packages/validator/src/services/prepareBeaconProposer.ts

maschad · 2023-02-22T16:56:36Z

Thanks for the feedback @nflaig :) made the updates!

nflaig

Looks pretty clean now, great refactoring!

packages/validator/src/services/indices.ts

packages/validator/src/services/prepareBeaconProposer.ts

packages/validator/src/services/indices.ts

nflaig · 2023-03-12T19:46:19Z

packages/validator/src/services/indices.ts

      const pubkeyHex = toHexString(validatorState.validator.pubkey);
      if (!this.pubkey2index.has(pubkeyHex)) {
-        this.logger.debug("Discovered validator", {pubkey: pubkeyHex, index: validatorState.index});
+        this.logger.info("Validator exists in beacon chain", {


@maschad running this on goerli with quite a lot of validators and this inital log is quite noisy in my opinion, we are printing out Validator exists in beacon chain for each validator which seems bit redundant because these logs all happen after each other.

I think we should reconsider this and maybe only print out Validator exists in beacon chain once and then just list all the validators, with index, pubkey and ideally also fee recipient (if possible)

Good point, I've included that change in be5a7e4

wemeetagain · 2023-03-21T16:49:47Z

🎉 This PR is included in v1.6.0 🎉

nflaig · 2023-04-06T07:52:59Z

packages/validator/src/services/indices.ts

+    // The total number of validators
+    const total = pubkeysHex.length;
+
+    this.logger.info("Validator statuses", {...statuses, total});


@maschad wondering here, wasn't this intended to be logged every epoch?

Once we discovered all pubkey we are not calling this due to this check

lodestar/packages/validator/src/services/indices.ts

Lines 102 to 106 in ddc58ce

const pubkeysHexToDiscover = pubkeysHex.filter((pubkey) => !this.pubkey2index.has(pubkey));

if (pubkeysHexToDiscover.length === 0) {

return [];

}

I guess this should be fixed by the other PR #5239

Fix log validator index, status and pubkey each epoch (ChainSafe#4785)

01b32cb

nflaig requested changes Feb 17, 2023

View reviewed changes

packages/validator/src/services/indices.ts Outdated Show resolved Hide resolved

packages/validator/src/services/indices.ts Outdated Show resolved Hide resolved

Update validator to display correct status and pubkey each epoch (Cha…

a7a778d

…inSafe#4785)

maschad changed the title ~~Fix: Log validator index, status and pubkey each epoch (#4785)~~ Fix: Log validator index, status and pubkey each epoch Feb 17, 2023

maschad marked this pull request as ready for review February 17, 2023 17:10

maschad requested a review from a team as a code owner February 17, 2023 17:10

maschad requested a review from nflaig February 17, 2023 17:10

Fix update log info formatting for validator status (ChainSafe#4785)

fe09b0d

nflaig previously approved these changes Feb 17, 2023

View reviewed changes

wemeetagain requested changes Feb 17, 2023

View reviewed changes

wemeetagain reviewed Feb 17, 2023

View reviewed changes

packages/validator/src/services/indices.ts Outdated Show resolved Hide resolved

maschad marked this pull request as draft February 20, 2023 19:26

Update logs to include total validator availability count (ChainSafe#…

b4eac94

…4785)

maschad dismissed nflaig’s stale review via b4eac94 February 22, 2023 01:47

maschad requested a review from wemeetagain February 22, 2023 01:47

maschad marked this pull request as ready for review February 22, 2023 01:47

Merge branch 'unstable' into fix/log-validator-status-and-pubkey

2a9b72f

maschad marked this pull request as draft February 22, 2023 02:14

Extracted logging to a function and included all validator statuses (C…

1014495

…hainSafe#4785)

maschad marked this pull request as ready for review February 22, 2023 02:27

nflaig requested changes Feb 22, 2023

View reviewed changes

Refactor to print logs more efficiently (ChainSafe#4785)

f1f0e36

maschad requested review from nflaig and removed request for wemeetagain February 22, 2023 16:56

maschad requested review from wemeetagain and removed request for nflaig February 22, 2023 16:56

nflaig reviewed Feb 22, 2023

View reviewed changes

packages/validator/src/services/indices.ts Outdated Show resolved Hide resolved

packages/validator/src/services/indices.ts Outdated Show resolved Hide resolved

packages/validator/src/services/prepareBeaconProposer.ts Outdated Show resolved Hide resolved

Removed debug log for validator exists in beacon chain

07e9d07

maschad requested review from wemeetagain and nflaig and removed request for wemeetagain and nflaig March 2, 2023 14:30

Merge branch 'unstable' into fix/log-validator-status-and-pubkey

7a2110a

nflaig self-requested a review March 2, 2023 20:20

updated import in prepareBeaconProposer

d8094a4

maschad force-pushed the fix/log-validator-status-and-pubkey branch from 1dce24b to d8094a4 Compare March 2, 2023 20:30

reverted changes in prepareBeaconProposer and added TODO comments

700b062

maschad mentioned this pull request Mar 2, 2023

Add VC metric to track validator statuses #5158

Open

wemeetagain previously approved these changes Mar 3, 2023

View reviewed changes

update lint error

5eb7541

maschad dismissed wemeetagain’s stale review via 5eb7541 March 3, 2023 16:08

maschad requested review from wemeetagain and removed request for nflaig March 3, 2023 16:08

nflaig reviewed Mar 3, 2023

View reviewed changes

packages/validator/src/services/indices.ts Outdated Show resolved Hide resolved

maschad added 2 commits March 3, 2023 11:24

updated log params

335ebbc

removed TODOs

b01b460

nflaig approved these changes Mar 3, 2023

View reviewed changes

wemeetagain approved these changes Mar 3, 2023

View reviewed changes

wemeetagain merged commit 0d638ba into ChainSafe:unstable Mar 3, 2023

maschad deleted the fix/log-validator-status-and-pubkey branch March 3, 2023 20:01

nflaig mentioned this pull request Mar 7, 2023

Add eslint rule to format TODO tags and similar meta comments #5242

Open

nflaig reviewed Mar 12, 2023

View reviewed changes

nflaig mentioned this pull request Mar 14, 2023

Track validator client statuses as metrics #5239

Closed

nflaig reviewed Apr 6, 2023

View reviewed changes

nflaig mentioned this pull request May 22, 2023

refactor: revise validator log to output seen instead of exists #5507

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Log validator index, status and pubkey each epoch #5161

Fix: Log validator index, status and pubkey each epoch #5161

maschad commented Feb 17, 2023

nflaig left a comment

maschad commented Feb 17, 2023

wemeetagain left a comment

g11tech commented Feb 18, 2023

maschad commented Feb 22, 2023

nflaig left a comment •

edited

Loading

nflaig Mar 12, 2023

maschad Mar 13, 2023

wemeetagain commented Mar 21, 2023

nflaig Apr 6, 2023 •

edited

Loading

	const pubkeysHexToDiscover = pubkeysHex.filter((pubkey) => !this.pubkey2index.has(pubkey));

	if (pubkeysHexToDiscover.length === 0) {
	return [];
	}

Fix: Log validator index, status and pubkey each epoch #5161

Fix: Log validator index, status and pubkey each epoch #5161

Conversation

maschad commented Feb 17, 2023

nflaig left a comment

Choose a reason for hiding this comment

maschad commented Feb 17, 2023

wemeetagain left a comment

Choose a reason for hiding this comment

g11tech commented Feb 18, 2023

maschad commented Feb 22, 2023

nflaig left a comment • edited Loading

Choose a reason for hiding this comment

nflaig Mar 12, 2023

Choose a reason for hiding this comment

maschad Mar 13, 2023

Choose a reason for hiding this comment

wemeetagain commented Mar 21, 2023

nflaig Apr 6, 2023 • edited Loading

Choose a reason for hiding this comment

nflaig left a comment •

edited

Loading

nflaig Apr 6, 2023 •

edited

Loading