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

Add support for the PUB_K1 public key format to v1/chain/get_fio_names #36

Open
aaroncox opened this issue Mar 25, 2020 · 10 comments · Fixed by #64
Open

Add support for the PUB_K1 public key format to v1/chain/get_fio_names #36

aaroncox opened this issue Mar 25, 2020 · 10 comments · Fixed by #64
Assignees

Comments

@aaroncox
Copy link
Contributor

It'd be very helpful of the FIO specific API endpoints that deal with public keys also support the PUB_K1 format of keys, which is what eosjs v20+ deals with internally for the most part. The endpoint I ran into where this was unsupported was the /v1/chain/get_fio_names endpoint, though there may be others that accept public keys as inputs that this would also help support.

For example, this does not currently work:

curl https://fiotestnet.greymass.com/v1/chain/get_fio_names -d '{"fio_public_key":"PUB_K1_6smr7ThQMWYBHzEvkzTZdxNNmUwxqh2VXdXZdDdzYHgak1U8Di"}'

I discovered the v1 history plugin endpoints already support both, likely because the conversion is happening. Examples as follows.

Using a PUB_K1 public key:

curl https://fiotestnet.greymass.com/v1/history/get_key_accounts -d '{"public_key":"PUB_K1_6smr7ThQMWYBHzEvkzTZdxNNmUwxqh2VXdXZdDdzYHgak1U8Di"}'

Using a FIO public key:

curl https://fiotestnet.greymass.com/v1/history/get_key_accounts -d '{"public_key":"FIO6smr7ThQMWYBHzEvkzTZdxNNmUwxqh2VXdXZdDdzYHgakgqCeb"}'
@0xCasey
Copy link
Member

0xCasey commented Apr 8, 2020

The following API endpoints could be modified to support this:

  • get_fio_balance
  • get_pending_fio_requests
  • get_sent_fio_requests
  • get_obt_data

However, the signing and the responses will still be in the FIO format.

@0xCasey
Copy link
Member

0xCasey commented Apr 8, 2020

@aaroncox would this type of fix be beneficial to BPs despite the response data being in the FIO format?

@aaroncox
Copy link
Contributor Author

aaroncox commented Apr 8, 2020

I believe so. At that point it might require some validation logic to be changed (if it exists), but will at least allow results to be returned to the client.

@0xCasey
Copy link
Member

0xCasey commented Apr 9, 2020

PR Submitted

@0xCasey 0xCasey assigned 0xCasey and unassigned adsorptionenthalpy Apr 9, 2020
@ericbutz ericbutz added this to the v1.1.0 milestone May 5, 2020
@ericbutz
Copy link
Contributor

ericbutz commented May 5, 2020

Resolved by #64

@ericbutz ericbutz closed this as completed May 5, 2020
@cc32d9 cc32d9 reopened this May 26, 2020
@cc32d9
Copy link
Contributor

cc32d9 commented May 26, 2020

You can't just replace FIO prefix with PUB_K1 because they are encoding the public key differently, You can see the difference here:

https://github.com/EOSIO/eosjs/blob/master/src/eosjs-numeric.ts#L272

@cc32d9
Copy link
Contributor

cc32d9 commented May 26, 2020

here's a better example in c++ https://github.com/EOSIO/abieos/blob/master/src/crypto.cpp#L134

@ericbutz ericbutz removed this from the v2.0 milestone Jul 29, 2020
@lukestokes
Copy link
Contributor

Someone floated the idea of just adding a prefix since we still have to use this as a human-readable address people can send to. Is that doable? Something like FIO_PUB_K1_6smr7ThQMWYBHzEvkzTZdxNNmUwxqh2VXdXZdDdzYHgak1U8Di?

@aaroncox
Copy link
Contributor Author

aaroncox commented Sep 9, 2020

It should be. Another option would be to use FIO_K1_6smr7ThQMWYBHzEvkzTZdxNNmUwxqh2VXdXZdDdzYHgak1U8Di (without the PUB portion)... though you could realistically do either way. The various SDKs would just need updated to support it.

The code @cc32d9 mentioned would need updated, eosjs/fiojs would need updated in the same way, and eosio-core would all have to be updated. It's not a comprehensive list, but those are a few areas I immediately recognize that would need updates.

@lukestokes
Copy link
Contributor

Yeah, I like FIO_KY vs. FIO_PUB_K1. I could think of it this way: "PUB_..." is a generic public key. "FIO_..." is a specific public key.

misterleet pushed a commit that referenced this issue Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants