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

Failed to convert BEEFY PublicKey to ETH address (westend & kusama) #1305

Closed
alindima opened this issue Aug 30, 2023 · 3 comments · Fixed by #1520
Closed

Failed to convert BEEFY PublicKey to ETH address (westend & kusama) #1305

alindima opened this issue Aug 30, 2023 · 3 comments · Fixed by #1520
Assignees

Comments

@alindima
Copy link
Contributor

Trying to upgrade the westend runtime in Versi, we start seeing a lot of errors like this one:
Failed to convert BEEFY PublicKey to ETH address.

As it scales with the number of validators and session frequency, this can pollute the logs of testnets.

CC: @acatangiu

@acatangiu
Copy link
Contributor

Thanks for the report @alindima !

Initial BEEFY keys (ecdsa keys) are some dummy keys that need to be replaced by validators with their actual real keys by using rotate_keys().

So, on testnets, each validator's initial dummy BEEFY key will throw an error that it can't be converted to ETH address.

We might be able to "fix" this by modifying initial dummy BEEFY keys to be some custom sequences of bytes that can be successfully converted to ETH addresses.

But there's a question on whether we should or not.. now this error logs are arguably useful to motivate validators to rotate keys and actually register valid/unique BEEFY key. It indirectly reports that validators are using invalid BEEFY keys.
At the same time, they are annoying in tests..

@andresilva wdyt? should we "fix" it or not?

@acatangiu acatangiu changed the title westend: Failed to convert BEEFY PublicKey to ETH address Failed to convert BEEFY PublicKey to ETH address (westend & kusama) Aug 30, 2023
@acatangiu acatangiu self-assigned this Aug 30, 2023
@bkchr
Copy link
Member

bkchr commented Aug 30, 2023

to be some custom sequences of bytes that can be successfully converted to ETH addresses

How hard is this? If it is fairly trivial by setting some bits or something, we should do it.

@andresilva
Copy link
Contributor

andresilva commented Aug 30, 2023

We are already doing .unwrap_or_default() when attempting the conversion, so I don't really see the need to generate "valid" dummy keys. Maybe just downgrade the logging to debug? We can instead do an error here if we see any of those keys, as this will only issue one log statement per block (and not proportional to the number of validators). We can also include the total number of 0x0 ETH addresses in that log statement and percentage of total.

bkchr pushed a commit that referenced this issue Sep 13, 2023
…on (#1520)

# Description

Each time the validator set changes, BEEFY validator keys are converted
to ETH addresses and merkelised into a `keyset_commitment` to be used by
light clients.

This commit downgrades `error` to `debug` when individual conversions
from BEEFY keys to ETH addresses fail, and adds cumulative check that
reports total number of failed conversions, if any, on `error`
log-level.

Fixes #1305

Signed-off-by: Adrian Catangiu <[email protected]>
taqtiqa-mark pushed a commit to subversive-upstream/polkadot-sdk-substrate-frame-beefy-mmr that referenced this issue Sep 14, 2023
…on (#1520)

# Description

Each time the validator set changes, BEEFY validator keys are converted
to ETH addresses and merkelised into a `keyset_commitment` to be used by
light clients.

This commit downgrades `error` to `debug` when individual conversions
from BEEFY keys to ETH addresses fail, and adds cumulative check that
reports total number of failed conversions, if any, on `error`
log-level.

Fixes paritytech/polkadot-sdk#1305

Signed-off-by: Adrian Catangiu <[email protected]>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
…on (paritytech#1520)

# Description

Each time the validator set changes, BEEFY validator keys are converted
to ETH addresses and merkelised into a `keyset_commitment` to be used by
light clients.

This commit downgrades `error` to `debug` when individual conversions
from BEEFY keys to ETH addresses fail, and adds cumulative check that
reports total number of failed conversions, if any, on `error`
log-level.

Fixes paritytech#1305

Signed-off-by: Adrian Catangiu <[email protected]>
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 a pull request may close this issue.

4 participants