Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add base58 support #392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add base58 support #392

wants to merge 1 commit into from

Conversation

k26dr
Copy link

@k26dr k26dr commented Jan 29, 2019

EOS public keys are encoded in base58, but CDT has no support for base58 encoding and decoding. This makes debugging smart contracts using public_key types difficult.

There are atleast 3 contracts that use EOS public keys in signatures and require a method of encoding a public_key type into the standard base58 representation. eosio.unregd, eosio.lost, and the proposed eosio.utxo

This pull requests adds 3 major functions:

  • eosio::b58enc: encode an arbitrary char array into base58.
  • eosio::b58tobin: decode an arbitrary base58 string into a char array
  • eosio::public_key::to_string: Get the base58 string representation of a public_key object.

@k26dr
Copy link
Author

k26dr commented Feb 6, 2019

Bump on this. Is there any interest in integrating this into CDT?

@k26dr
Copy link
Author

k26dr commented Feb 6, 2019

This will also be useful while developing IBC

@larryk85
Copy link
Contributor

larryk85 commented Feb 8, 2019

Outside of debugging your smart contract. I can see the use case for adding base58 support to cdt. The main reason I don't want to add it, 1) licensing with your code is bit iffy, 2) we don't want to make it easier for developers to do a lot of string manipulations in smart contracts. That being said, I could be swayed into rethinking some of these assumptions. What use cases for base58 do you think you would need for IBC?

@k26dr
Copy link
Author

k26dr commented Feb 8, 2019

Mostly debugging to see whether keys have been verified properly. The eosio.lost contract signs string versions of the keys, so we've imported base58 libraries to make the conversion when verifying the signature.

For IBC though, I think it would be mostly debugging. I don't see the conversion being necessary to actually verify txs or blocks.

What we can do instead is maintain this fork on the LibertyBlock CDT repo, and if anyone needs access to base58 for debugging, we can refer people to that fork. Would that be a better solution?

@larryk85
Copy link
Contributor

In possibly the next release of CDT (v.1.7.0), I am planning on making a "debug" build variant, that would then prune certain things from the smart contract. Things like debug prints and debug functions. I would like to move this into that framework, I do see the usefulness of it. Until that time, I would like to keep this P.R. open to act as a reminder for me and also as a place we could send people for way to grab the base58 implementation. Does this seem reasonable to you? Or do you see this being needed much sooner than that?

@k26dr
Copy link
Author

k26dr commented Feb 11, 2019

That seems like a great solution. I'm on board with it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants