[KMP] Fix issue: memory leak found in Base58.decode in iOS

[10gic]

Description

Fix issue #4030

How to test

Run Rust, C++, Kotlin, Swift tests

Types of changes

Checklist

• Create pull request as draft initially, unless its complete.
• Add tests to cover changes as needed.
• Update documentation as needed.
• If there is a related Issue, mention it in the description.

If you're adding a new blockchain

• I have read the guidelines for adding a new blockchain.

Screenshots

Before fixing, run Base58.decode 100000 times, it takes up to 200 MB of memory:

After fixing, run Base58.decode 100000 times, it takes up to 97.4 MB of memory:

Solution

The generated code before fixing:

package com.trustwallet.core

actual object Base58 {

    // ......

    actual fun decode(string: String): ByteArray? =
        TWBase58Decode(string.toTwString()).readTwBytes()         // toTwString and readTwBytes leak memory!


    // ......
}

Definition of old readTwBytes:

internal fun COpaquePointer?.readTwBytes(): ByteArray? =
    TWDataBytes(this)?.readBytes(TWDataSize(this).toInt())

The generated code after fixing:

package com.trustwallet.core

actual object Base58 {

    // ......

    actual fun decode(string: String): ByteArray? {
        val stringString = TWStringCreateWithUTF8Bytes(string)
        val result = TWBase58Decode(stringString).readTwBytes()
        TWStringDelete(stringString)
        return result
    }

    // ......
}

Definition of new readTwBytes:

internal fun COpaquePointer?.readTwBytes(): ByteArray? =
    this?.let {
        val result = TWDataBytes(it)?.readBytes(TWDataSize(it).toInt())
        TWDataDelete(it)
        result
    }

[10gic]

@satoshiotomakan Could you please review this PR?

[satoshiotomakan]

Awesome work as usual! Could you please consider minor changes?

[satoshiotomakan]

@10gic can you please fix code formatting?

The code will be available in the iOS Kotlin package, so it should be readable enough.

Please also note that iOS native bindings have been broken by changing val -> value function argument.
Could you please fix it?

[satoshiotomakan]

@10gic is it possible to revert val -> value argument name refactoring?

[10gic]

@10gic can you please fix code formatting?

The code will be available in the iOS Kotlin package, so it should be readable enough.

Please also note that iOS native bindings have been broken by changing val -> value function argument. Could you please fix it?

I noticed this issue. However, if I fix the too little code indentation in AnyAddress.kt, then Base58.kt will have too much indentation because the adjusted code in AnyAddress.kt is located within the actual companion object code block, which inherently requires more indentation.

[10gic]

@10gic is it possible to revert val -> value argument name refactoring?

Do you have any other considerations? Since the keywords in different languages are different, avoiding using them is the simplest way.

[satoshiotomakan]

The main reason is to avoid unnecessary breaking changes

@10gic is it possible to revert val -> value argument name refactoring?

Do you have any other considerations? Since the keywords in different languages are different, avoiding using them is the simplest way.

[10gic]

The main reason is to avoid unnecessary breaking changes

@10gic is it possible to revert val -> value argument name refactoring?

Do you have any other considerations? Since the keywords in different languages are different, avoiding using them is the simplest way.

Actually, I didn't intend to adjust this behavior in this PR. I can't remember what issue led me to change it. However, I can try to revert it.

[10gic]

@satoshiotomakan All comments have been resolved, please review again.

[satoshiotomakan]

LGTM!