Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Can't get status when validator key is blank #1417

Open
penso opened this issue Apr 27, 2024 · 4 comments
Open

Can't get status when validator key is blank #1417

penso opened this issue Apr 27, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@penso
Copy link
Contributor

penso commented Apr 27, 2024

What went wrong?

There is an issue with secp256k1 parsing blank secp256k1 public keys.

❯ export TENDERMINT_RPC_URL=https://dydx-rpc.publicnode.com
❯ ./target/release/tendermint-rpc status
2024-04-27T16:52:01.302601Z  INFO tendermint_rpc: Using HTTP client to submit request to: https://dydx-rpc.publicnode.com/
2024-04-27T16:52:01.647400Z ERROR tendermint_rpc: Failed: serde parse error

Caused by:
    invalid secp256k1 key at line 1 column 1113

Location:
    /Users/penso/.cargo/registry/src/index.crates.io-6f17d22bba15001f/flex-error-0.4.4/src/tracer_impl/eyre.rs:10:9

The status json includes:

{
    "validator_info": {
      "address": "0000000000000000000000000000000000000000",
      "pub_key": {
        "type": "tendermint/PubKeySecp256k1",
        "value": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
      },
      "voting_power": "0"
    }
}

I don't think an invalid key should block using tendermint-rs for RPC calls, but not sure how this should be fixed. Do we expect public keys to be verified by default?

@penso penso added the bug Something isn't working label Apr 27, 2024
@tony-iqlusion
Copy link
Collaborator

Ugh, it's rather unfortunate whatever decided to do that used a garbage public key, especially as the value 0 is an actually valid SEC1 encoding of the identity point which would've made more sense for this application.

What decided to encode the key this way? Was it CometBFT itself?

@penso
Copy link
Contributor Author

penso commented Apr 28, 2024

Ugh, it's rather unfortunate whatever decided to do that used a garbage public key, especially as the value 0 is an actually valid SEC1 encoding of the identity point which would've made more sense for this application.

What decided to encode the key this way? Was it CometBFT itself?

I'm not sure how it got there since I'm not involved in CometBFT neither DYDX. I feel like it's related to the node itself, maybe in its configuration?

@tony-iqlusion
Copy link
Collaborator

There is a way to decode these points where k256 will accept this value, namely 33-bytes of all zero: using <k256::AffinePoint as GroupEncoding>::from_repr, which always accepts a 33-byte array as input, and will decode 33-bytes of all zero as the identity point:

https://docs.rs/k256/latest/k256/struct.AffinePoint.html#impl-GroupEncoding-for-AffinePoint

Note however this is not a standard SEC1 encoding, just an eccentricity of how that particular API works because it takes an array as input and can't handle a shorter message, but if it matches what I hope is a CometBFT behavior and not some chain-specific one-off behavior it would fix this particular problem.

The current tendermint::PublicKey::Secp256k1 variant expressly disallows the identity point though, as it's not typically a valid public key, so switching to that would entail changing that variant to represent all secp256k1 public keys as k256::AffinePoint rather than k256::ecdsa::VerifyingKey, or handling this case with e.g. Option, which might be a bit onerous from an API perspective.

@penso
Copy link
Contributor Author

penso commented May 9, 2024

As a follow-up, the way I "fixed" it is using my own code with reqwest (and not tendermint-rs) to fetch status information from nodes.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants