[Pettycoin-dev] PATCH: Generalize raw_decode_base58 adding raw_decode_base_n

[Rusty Russell]

Hi, sorry for the delayed response!

Minor feedback below.

First, I generally prefer patches inline (git am can do this for you),
so I can directly quote them.  Also mailman seems to eat your
attachments, so they didn't reach the archive.

Second, I've added a Signed-off-by: requirement a-la Linux kernel.  I've
had lawyers tell me it's important in the past, and maybe they're right.

> commit d9a7fd88ae802d5ca8d4aad6e57ad0f09dd8ab8e
> Author: Nelson Castillo <nelsoneci at gmail.com>
> Date:   Wed Aug 6 04:28:06 2014 -0400
> 
>     Generalize raw_decode_base58 adding raw_decode_base_n
>     
>     This will make this function useful for other bases.
>     
>     I was trying to make code to import from a raw hex bitcoin key.
>     I do not know if this (raw hex import) makes sense for Pettycoin.
> 
> diff --git a/base58.c b/base58.c
> index da4a17d..5093b77 100644
> --- a/base58.c
> +++ b/base58.c
> @@ -12,16 +12,17 @@
>  #include <openssl/sha.h>
>  #include <string.h>
>  
> -static const char enc[] =
> +static const char enc_16[] = "0123456789abcdef";
> +static const char enc_58[] =
>  	"123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz";
>  
> -static char encode_char(unsigned long val)
> +static char encode_char(unsigned long val, const char *enc)
>  {
>  	assert(val < strlen(enc));
>  	return enc[val];
>  }
>  
> -static int decode_char(char c)
> +static int decode_char(char c, const char *enc)
>  {
>  	const char *pos = strchr(enc, c);
>  	if (!pos)
> @@ -58,7 +59,7 @@ static char *encode_base58(char *buf, size_t buflen,
>  			p = NULL;
>  			goto out;
>  		}
> -		*p = encode_char(rem);
> +		*p = encode_char(rem, enc_58);
>  	}
>  
>  	/* Now, this is really weird.  We pad with zeroes, but not at
> @@ -69,7 +70,7 @@ static char *encode_base58(char *buf, size_t buflen,
>  			p = NULL;
>  			goto out;
>  		}
> -		*p = encode_char(0);
> +		*p = encode_char(0, enc_58);
>  		data_len--;
>  		data++;
>  	}
> @@ -80,20 +81,36 @@ out:
>  }
>  
>  /*
> - * Decode a base58-encoded string into a byte sequence.
> + * Decode a base_n-encoded string into a byte sequence.
>   */
> -bool raw_decode_base58(BIGNUM *bn, const char *src, size_t len)
> +bool raw_decode_base_n(BIGNUM *bn, const char *src, size_t len, int base)
>  {
> +	const char *enc;
>  	BN_init(bn);
>  	BN_zero(bn);
>  
> +	assert(base == 16 || base == 58);
> +	switch (base) {
> +	case 16:
> +		enc = enc_16;
> +		break;
> +	case 58:
> +		enc = enc_58;
> +		break;
> +	default:
> +		abort();

Doesn't need both assert() and abort() :)

And given your comment in your test, you might want to have the caller
do the BN_init()?

Cheers,
Rusty.