Validator manager import to allow overriding fields with CLI flag

[chong-he]

Issue Addressed

• Validator manager import to allow --suggested-fee-recipient for all keystore files #7651

[mergify]

Some required checks have failed. Could you please take a look @chong-he? 🙏

[mergify]

Some required checks have failed. Could you please take a look @chong-he? 🙏

[michaelsproul]

@macladson would you mind reviewing this? 🙏

[macladson]

We could move the logging inside the for loop? I doubt we need to log anything in the case that validators.len() == 0 and it would consolidate the if lets

[chong-he]

Thanks for the review. I got what you mean.

Previously, before consolidating the if let, the message would be logged whenever the flags (e.g., -suggested-fee-recipient) are used together with lighthouse vm import, even though there are no validators

Now, after moving in the if loop, when there is no validators, the if loop will not be executed, so no message is logged. Makes complete sense

Revise in 6237ab3

[macladson]

This looks pretty good! Just needs some fixes for the logging behaviour

[macladson]

Ah so as a consequence of moving the logs inside the for loop, this will log a separate instance of each Please note! log for every validator in your validator.json file. So we should either:

1. Add the validator public key to the log line. This makes it clear that each validator will have the field overridden.
2. Move the logging outside back outside the for loop but add a conditional that it doesn't run when validators.len() == 0.

I think I'm fine with either, it's just a matter of which we feel is more ergonomic (1 log per validator vs 1 log total for all validators). For 100s of validators, 1 log per validator would likely be too much. But if you had 2 validators, having a reminder that the override will apply to both might be nice

[chong-he]

I am leaning towards no. 2, I will revise

[chong-he]

I added the check if !validators.is_empty() (for validators.len() == 0), but I am thinking that this may not be exactly accurate?

For example, when we import new validators and the VC has no validators, then the logs Please note! ... wouldn't be shown, which is not really the intention. i.e., it's not only about modifying existing validators, it can also be used to modify the fields for newly added validators (including the scenario when the VC has no validators)