BigInteger RLP bug

[jolestar]

@Test
    fun testBigIntRLP() {
        val bigInt = BigInteger.valueOf(54408193066555392L)
        val rlp = bigInt.toRLP()
        println(rlp.bytes.size)
        val bitInt1 = rlp.toBigIntegerFromRLP()
        Assert.assertEquals(bigInt, bitInt1)
}

7

java.lang.AssertionError: 
Expected :54408193066555392
Actual   :-17649400971372544

same as #49

[ligi]

Thanks for reporting!

[ligi]

@jolestar unfortunately this issue is nothing like #49 - actually it is a huge time sink and I fear I will not have time for it this year anymore. Is this issue blocking you somehow?

[jolestar]

@ligi thanks，I will try to fix it first. Can you provide some suggestions for fixing it?

[jolestar]

@ligi use UnsignedBigInteger can fix this bug.

fun RLPElement.toLongFromRLP(): Long = this.toUnsignedBigIntegerFromRLP().toLong()

[ligi]

thanks for the follow-up. Yea unsigned solves the problem - but still not yet sure how a good fix looks like. Perhaps just allowing unsigned ..
So I would propose this:

diff --git a/rlp/src/main/kotlin/org/kethereum/functions/rlp/RLPTypeConverter.kt b/rlp/src/main/kotlin/org/kethereum/functions/rlp/RLPTypeConverter.kt
index d7da0ab..936409a 100644
--- a/rlp/src/main/kotlin/org/kethereum/functions/rlp/RLPTypeConverter.kt
+++ b/rlp/src/main/kotlin/org/kethereum/functions/rlp/RLPTypeConverter.kt
@@ -24,7 +24,6 @@ fun RLPElement.toIntFromRLP() = bytes
         .mapIndexed { index, byte -> (byte.toInt() and 0xff).shl((bytes.size - 1 - index) * 8) }
         .reduce { acc, i -> acc + i }
 
-fun RLPElement.toBigIntegerFromRLP(): BigInteger = if (bytes.isEmpty()) ZERO else BigInteger(bytes)
 fun RLPElement.toUnsignedBigIntegerFromRLP(): BigInteger = if (bytes.isEmpty()) ZERO else BigInteger(1, bytes)
 fun RLPElement.toByteFromRLP(): Byte {
     if (bytes.size != 1) {
diff --git a/rlp/src/test/kotlin/org/kethereum/functions/rlp/TheRLPTypeConverter.kt b/rlp/src/test/kotlin/org/kethereum/functions/rlp/TheRLPTypeConverter.kt
index 1d7dc3d..46b04cc 100644
--- a/rlp/src/test/kotlin/org/kethereum/functions/rlp/TheRLPTypeConverter.kt
+++ b/rlp/src/test/kotlin/org/kethereum/functions/rlp/TheRLPTypeConverter.kt
@@ -2,6 +2,7 @@ package org.kethereum.functions.rlp
 
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
+import org.walleth.khex.toHexString
 import java.math.BigInteger
 import java.math.BigInteger.*
 
@@ -14,11 +15,12 @@ class TheRLPTypeConverter {
             assertThat(it.toRLP().toIntFromRLP()).isEqualTo(it)
         }
 
-        assertThat(ZERO.toRLP().toBigIntegerFromRLP()).isEqualTo(ZERO)
-        assertThat(ONE.toRLP().toBigIntegerFromRLP()).isEqualTo(ONE)
-        assertThat(TEN.toRLP().toBigIntegerFromRLP()).isEqualTo(TEN)
-        assertThat(BigInteger.valueOf(Long.MAX_VALUE).toRLP().toBigIntegerFromRLP())
-                .isEqualTo(BigInteger.valueOf(Long.MAX_VALUE))
+        arrayOf(ZERO, ONE, TEN,  BigInteger.valueOf(70_000),BigInteger.valueOf(Long.MAX_VALUE),BigInteger.valueOf(54408193066555392L) ).forEach {
+            assertThat(it.toRLP().toUnsignedBigIntegerFromRLP()).isEqualTo(it)
+        }

what do you think?

[ligi]

I think I will go this way for now. So it is more correct - but we loose one feature for now. Did not yet find a way to support signed bigintegers - happy about ideas there.