eosrio / bp-info-standard

JSON Standard for Block Producer Information on EOSIO Blockchains
48 stars 58 forks source link

Clarify producer public key #7

Closed topcat2001 closed 6 years ago

topcat2001 commented 6 years ago

It is unclear if the producer_public_key is the account active key, the account owner key, the block signing key or some other. Would be good to clarify.

ellipticasec commented 6 years ago

My own opinion, since many BPs will (and should) have multi-sig accounts this gets complicated.

Technically the key used with regproducer is the one used to mint blocks so effectively you vote to allow someone to use that key to mint.

On the other hand that key may be rotated more frequently...

matthewdarwin commented 6 years ago

Or maybe allow multiple keys?

systemzax commented 6 years ago

I would suggest allowing multiple keys with their role description (owner, active, block signing, etc...).

eosrio commented 6 years ago

What about the following change? From:

{
  "producer_account_name": "",
  "producer_public_key": "",
  "org": {
    "candidate_name": "",
...

To:

{
  "producer_account_name": "",
  "permissions": {
      "owner": "",
      "active": "",
      "sign": ""
   },
  "org": {
    "candidate_name": "",
...
lukestokes commented 6 years ago

Right now tooling seems to be validating it against the signing key. Example:

https://validate.eosnation.io/producers/eosdacserver.html

field=<producer_public_key> does not match between bp.json and regproducer

If that's the case, this should not be a high level field, but a field for individual nodes. We should then be able to switch between our primary producer and our backup producer without seeing errors like this. Each signing key should be seen as valid. If we add our producing nodes to the bp.json, then there should be no endpoint requirements at all since those should remain private and unknown for producing nodes.

"node_type": "producer",
"producer_key": "EOS7aBPDACAn1SpJDjmaZHSEUgqfNWdUaNawVZhVuEWUx5aoepJf6",

So essentially producer_key is only required for node_type = producer and all the endpoints are only required for node_type != "producer"

lukestokes commented 6 years ago

Follow up question:

Is producer_public_key actually used for anything? Why is it part of the spec? It's never shown to the user, so how will voting tools use this signing key? What purpose does it serve? If it serves no purpose, we should remove it.

n8d commented 6 years ago

I don't think it's useful. We have account, so if there is a reason someone needs a key they could get it from the chain.

n8d commented 6 years ago

Unless the idea is to extend it with a signature as well, as suggested in https://github.com/eosrio/bp-info-standard/issues/10.

eosrio commented 6 years ago

Okay, lets so a vote (using thumbs-up/down) to remove this. Please vote thumbs down if you want to keep or extend it to allow multiple-keys.

matthewdarwin commented 6 years ago

I would prefer to remove it at this time. If someone comes up with a really good use case for it in the future, then can be re-discussed. I have not yet seen anyone say what business value this provides.

lukestokes commented 6 years ago

@eosrio sorry, I was confused by your vote question earlier. I thought it was a thumbs down to remove. I've changed my vote to a thumbs up for removal. When we switch back and forth between producing nodes, we don't want to also have to update the signing key. It makes no sense as the signing key is defined by the value in listproducers, not by what is in a json file on a website.

n8d commented 6 years ago

Oh, I was confused too :)

My vote is/was to remove it.

matthewdarwin commented 6 years ago

There is one use case where I know where having the key is useful. That is when someone steals a BP's identity entirely it is obvious if the key doesn't match. This already happened. (regproduce using an existing BP's URL).

DenisCarriere commented 6 years ago

Removing the block signing key from the bp.json standard would also be my vote.

If someone steals your BP key, you've got bigger problems then your bp.json not syncing up.

The block signing key is already registered on the blockchain, no need to duplicate this information in the bp.json.

matthewdarwin commented 6 years ago

I think you missed my point. If someone steals your bp.json and makes no changes, the block signing key will not match. (I am not saying they are stealing your key).

Anyway, as I said in the original thread, I'm fine to also remove this field entirely.

n8d commented 6 years ago

If I'm understanding that case correctly, producer_account_name provides the same usefulness as it wouldn't match as well. Is that right?

But yeah, I think we are all in agreement to remove it.

DenisCarriere commented 6 years ago

@matthewdarwin oh yes, if someone takes over your bp.json at least you have a way to check on chain if the information is accurate & matching.

Good argument.

lukestokes commented 6 years ago

It's not easy to steal an identity entirely because they would have to register a different bp account name and have a different website to host their fraudulent bp.json. The account name, url, and signing key are already on the blockchain. Signing keys can change often as BPs move from primaries to backups and back again. Those changes are seen as false positives for a problem under the current schema.

I vote to ditch it.

notchxor commented 6 years ago

I agree that this needs to be remove. In our case we use different keys for different nodes, so everytime we change node the keys dont match.

eosauthority commented 6 years ago

We removed this on our bp.json and our validation failed on the portal side. This is still required on the schema https://github.com/eosrio/bp-info-standard/blob/master/schema.json#L35

Can we get the Schema updated?

igorls commented 6 years ago

Yes, updating soon