pmarches / jStellarAPI

java API for the stellar network
GNU Lesser General Public License v3.0
15 stars 3 forks source link

Ripplebase58 decode #18

Closed jargoman closed 10 years ago

jargoman commented 10 years ago

I have another one for you. Lines 58, 59 of Ripplebase58

Instead of... byte[] tmp = new byte[bytes.length - (stripSignByte ? 1 : 0) + leadingZeros]; System.arraycopy(bytes, stripSignByte ? 1 : 0, tmp, leadingZeros, tmp.length - leadingZeros);

I believe they should be byte[] tmp = new byte[bytes.length - (stripSignByte ? leadingZeros : 0)]; System.arraycopy(bytes, (stripSignByte ? leadingZeros : 0), tmp, 0, tmp.length - leadingZeros);

jargoman commented 10 years ago

returning bigInt : 5723087235679228605600552202233769738437593828739600517137 ToByteArray() = { 0, 233, 103, 209, 152, 167, 72, 126, 195, 227, 245, 111, 166, 183, 46, 5, 149, 160, 73, 136, 214, 121, 66, 136, 17, }

stripSignByte = True

leadingZeros = 1

tmp[] = { 0, 233, 103, 209, 152, 167, 72, 126, 195, 227, 245, 111, 166, 183, 46, 5, 149, 160, 73, 136, 214, 121, 66, 136, 17, }

Note: I'm aware I just posted the secret of my test account, which just occurred to me is a bug in my app that should not place the secret in debugging info lmao

jargoman commented 10 years ago

ok, although I do think your intention was to strip the leading zeros, if the code actually does successfully strip the zeros it introduces bugs later in the code where the identifier byte would be stripped and in the event of an address coincidentally containing leading zeros they would be stripped as well. RippleIdentifier payloadbytes would have a shorter length than intended.

The array in the above comment is the public-address and not the seed as I originally thought. The leading 0 is the 'r' identifier byte and should not be stripped. Ironically the bug in the code is causing it to function correctly.

jargoman commented 10 years ago

I apologize after running many addresses through the function I now see that the function does do what it is supposed to do. The purpose of the function is not to strip the leading bytes but to match the number of bytes implied by the base58 string. I switched from c#'s System.Numeric.BigInteger to Org.Bouncycastle.BigInteger and now things are running smoothly.

Sorry it is hard to understand the intention of another programmers code.

pmarches commented 10 years ago

No problem. Actually, this portion of the code was lifted from bitcoinj.

Cheers!

On Thu, Feb 6, 2014 at 6:25 PM, jargoman notifications@github.com wrote:

I apologize after running many addresses through the function I now see that the function does do what it is supposed to do. The purpose of the function is not to strip the leading bytes but to match the number of bytes implied by the base58 string. I switched from c#'s System.Numeric.BigInteger to Org.Bouncycastle.BigInteger and now things are running smoothly.

Sorry it is hard to understand the intention of another programmers code.

Reply to this email directly or view it on GitHubhttps://github.com/pmarches/jRippleAPI/issues/18#issuecomment-34398465 .