From 2ed70985daab17e2014748ceff19ba6b210f9a1f Mon Sep 17 00:00:00 2001 From: 10gic Date: Tue, 1 Oct 2024 19:19:42 +0800 Subject: [PATCH] [KMP] Fix issue: memory leak found in Base58.decode in iOS (#4031) * Fix kmp issue: memory leak found in Base58.decode in iOS * Remove unused functions * Fix failed test cases * Revert "Fix failed test cases" This reverts commit 57eee395df508d95a5e570b1f54143b7de20eb31. * Revert val -> value argument name refactoring * Output better indentation * Revert changes in TWEthereumAbiFunction.h * Fix inconsistent naming --------- Co-authored-by: satoshiotomakan <127754187+satoshiotomakan@users.noreply.github.com> --- codegen/lib/kotlin_helper.rb | 25 +++++----- codegen/lib/templates/kotlin/ios_class.erb | 50 +++++++++++++++---- codegen/lib/templates/kotlin/ios_enum.erb | 8 ++- .../templates/kotlin/ios_parameter_access.erb | 27 ++++++++++ .../kotlin/ios_parameter_release.erb | 23 +++++++++ codegen/lib/templates/kotlin/ios_struct.erb | 10 +++- .../kotlin/com/trustwallet/core/AnySigner.kt | 28 ++++++++--- .../com/trustwallet/core/ByteArrayExt.kt | 11 ++-- .../kotlin/com/trustwallet/core/StringExt.kt | 11 ++-- 9 files changed, 149 insertions(+), 44 deletions(-) create mode 100644 codegen/lib/templates/kotlin/ios_parameter_access.erb create mode 100644 codegen/lib/templates/kotlin/ios_parameter_release.erb diff --git a/codegen/lib/kotlin_helper.rb b/codegen/lib/kotlin_helper.rb index b37537b4fa4..b99bf1a631d 100644 --- a/codegen/lib/kotlin_helper.rb +++ b/codegen/lib/kotlin_helper.rb @@ -31,7 +31,13 @@ def self.js_parameters(params) def self.calling_parameters_ios(params) names = params.map do |param| name = fix_name(param.name) + if param.type.name == :data + "#{name}Data#{convert_calling_type_ios(param.type)}" + elsif param.type.name == :string + "#{name}String#{convert_calling_type_ios(param.type)}" + else "#{name}#{convert_calling_type_ios(param.type)}" + end end names.join(', ') end @@ -56,7 +62,7 @@ def self.fix_name(name) when '' "value" when 'val' - "value" + "value" when 'return' '`return`' else @@ -65,19 +71,12 @@ def self.fix_name(name) end def self.convert_calling_type_ios(t) - case t.name - when :data - "#{if t.is_nullable then '?' else '' end}.toTwData()" - when :string - "#{if t.is_nullable then '?' else '' end}.toTwString()" + if t.is_enum + "#{if t.is_nullable then '?' else '' end}.nativeValue" + elsif t.is_class + "#{if t.is_nullable then '?' else '' end}.pointer" else - if t.is_enum - "#{if t.is_nullable then '?' else '' end}.nativeValue" - elsif t.is_class - "#{if t.is_nullable then '?' else '' end}.pointer" - else - '' - end + '' end end diff --git a/codegen/lib/templates/kotlin/ios_class.erb b/codegen/lib/templates/kotlin/ios_class.erb index 240b2dc7f23..94d7a42f24b 100644 --- a/codegen/lib/templates/kotlin/ios_class.erb +++ b/codegen/lib/templates/kotlin/ios_class.erb @@ -2,6 +2,7 @@ import cnames.structs.TW<%= entity.name %> import kotlinx.cinterop.CPointer +import kotlinx.cinterop.toCValues <% constructors = entity.static_methods.select { |method| method.name.start_with?('Create') } -%> <% methods = entity.methods.select { |method| not method.name.start_with?('Delete') } -%> @@ -20,12 +21,9 @@ actual class <%= entity.name %> constructor( <% if constructor.return_type.is_nullable -%> @Throws(IllegalArgumentException::class) - actual constructor(<%= KotlinHelper.parameters(constructor.parameters) %>) : this( - TW<%= entity.name %><%= constructor.name %>(<%= KotlinHelper.calling_parameters_ios(constructor.parameters) %>) ?: throw IllegalArgumentException() -<% else -%> - actual constructor(<%= KotlinHelper.parameters(constructor.parameters) %>) : this( - TW<%= entity.name %><%= constructor.name %>(<%= KotlinHelper.calling_parameters_ios(constructor.parameters) %>)!! <% end -%> + actual constructor(<%= KotlinHelper.parameters(constructor.parameters) %>) : this( + wrapperTW<%= entity.name %><%= constructor.name %>(<%= KotlinHelper.arguments(constructor.parameters) %>) ) <% end -%> <%# Property declarations -%> @@ -37,12 +35,38 @@ actual class <%= entity.name %> constructor( <%# Method declarations -%> <% methods.each do |method| -%> - actual fun <%= KotlinHelper.format_name(method.name) %>(<%= KotlinHelper.parameters(method.parameters.drop(1)) %>)<%= KotlinHelper.return_type(method.return_type) %> = - <%= KotlinHelper.convert_calling_return_type_ios(method.return_type, "TW#{entity.name}#{method.name}(pointer#{', ' if not method.parameters.one?}#{KotlinHelper.calling_parameters_ios(method.parameters.drop(1))})") %> + actual fun <%= KotlinHelper.format_name(method.name) %>(<%= KotlinHelper.parameters(method.parameters.drop(1)) %>)<%= KotlinHelper.return_type(method.return_type) %> { +<%= render('kotlin/ios_parameter_access.erb', { method: method }) -%> + val result = <%= KotlinHelper.convert_calling_return_type_ios(method.return_type, "TW#{entity.name}#{method.name}(pointer#{', ' if not method.parameters.one?}#{KotlinHelper.calling_parameters_ios(method.parameters.drop(1))})") %> +<%= render('kotlin/ios_parameter_release.erb', { method: method }) -%> + return result + } <% end -%> -<% if entity.static_properties.any? || static_methods.any? -%> +<% if entity.static_properties.any? || static_methods.any? || constructors.any? -%> + + <%= if entity.static_properties.any? || static_methods.any? then "actual" else "private" end %> companion object { +<%# Constructor wrappers -%> +<% if constructors.any? -%> +<% constructors.each do |constructor| -%> - actual companion object { +<% if constructor.return_type.is_nullable -%> + @Throws(IllegalArgumentException::class) +<% end -%> + private fun wrapperTW<%= entity.name %><%= constructor.name %>(<%= KotlinHelper.parameters(constructor.parameters) %>): CPointer> { +<%= render('kotlin/ios_parameter_access.erb', { method: constructor, more_index: 4 }) -%> + val result = TW<%= entity.name %><%= constructor.name %>(<%= KotlinHelper.calling_parameters_ios(constructor.parameters) %>) +<%= render('kotlin/ios_parameter_release.erb', { method: constructor, more_index: 4 }) -%> +<% if constructor.return_type.is_nullable -%> + if (result == null) { + throw IllegalArgumentException() + } + return result +<% else -%> + return result!! +<% end -%> + } +<% end -%> +<% end -%> <%# Static property declarations -%> <% entity.static_properties.each do |property| -%> @@ -52,8 +76,12 @@ actual class <%= entity.name %> constructor( <%# Static method declarations -%> <% static_methods.each do |method| -%> - actual fun <%= KotlinHelper.format_name(method.name) %>(<%= KotlinHelper.parameters(method.parameters) %>)<%= KotlinHelper.return_type(method.return_type) %> = - <%= KotlinHelper.convert_calling_return_type_ios(method.return_type, "TW#{entity.name}#{method.name}(#{KotlinHelper.calling_parameters_ios(method.parameters)})") %> + actual fun <%= KotlinHelper.format_name(method.name) %>(<%= KotlinHelper.parameters(method.parameters) %>)<%= KotlinHelper.return_type(method.return_type) %> { +<%= render('kotlin/ios_parameter_access.erb', { method: method, more_index: 4 }) -%> + val result = <%= KotlinHelper.convert_calling_return_type_ios(method.return_type, "TW#{entity.name}#{method.name}(#{KotlinHelper.calling_parameters_ios(method.parameters)})") %> +<%= render('kotlin/ios_parameter_release.erb', { method: method, more_index: 4 }) -%> + return result + } <% end -%> } <% end -%> diff --git a/codegen/lib/templates/kotlin/ios_enum.erb b/codegen/lib/templates/kotlin/ios_enum.erb index 5f64c80a7fa..16d26ba89b4 100644 --- a/codegen/lib/templates/kotlin/ios_enum.erb +++ b/codegen/lib/templates/kotlin/ios_enum.erb @@ -41,8 +41,12 @@ actual enum class <%= entity.name %>( <%- entity.methods.each do |method| -%> <%- next if method.name.start_with?('Delete') -%> - actual fun <%= KotlinHelper.format_name(method.name) %>(<%= KotlinHelper.parameters(method.parameters.drop(1)) %>)<%= KotlinHelper.return_type(method.return_type) %> = - TW<%= entity.name %><%= method.name %>(value<%= ', ' if not method.parameters.one? %><%= KotlinHelper.calling_parameters_ios(method.parameters.drop(1)) %>)<%= KotlinHelper.convert_calling_return_type_ios(method.return_type) %> + actual fun <%= KotlinHelper.format_name(method.name) %>(<%= KotlinHelper.parameters(method.parameters.drop(1)) %>)<%= KotlinHelper.return_type(method.return_type) %> { +<%= render('kotlin/ios_parameter_access.erb', { method: method }) -%> + val result = TW<%= entity.name %><%= method.name %>(value<%= ', ' if not method.parameters.one? %><%= KotlinHelper.calling_parameters_ios(method.parameters.drop(1)) %>)<%= KotlinHelper.convert_calling_return_type_ios(method.return_type) %> +<%= render('kotlin/ios_parameter_release.erb', { method: method }) -%> + return result + } <%- end -%> <%# Value -%> <% if entity.cases.any? { |e| !e.value.nil? } -%> diff --git a/codegen/lib/templates/kotlin/ios_parameter_access.erb b/codegen/lib/templates/kotlin/ios_parameter_access.erb new file mode 100644 index 00000000000..b16ce477107 --- /dev/null +++ b/codegen/lib/templates/kotlin/ios_parameter_access.erb @@ -0,0 +1,27 @@ +<% + method = locals[:method] + more_index = locals[:more_index] || 0 + + method.parameters.each do |param| -%> +<% if param.type.name == :data -%> +<% if param.type.is_nullable -%> +<%= ' ' * more_index %> val <%= KotlinHelper.fix_name(param.name) %>Data = if (<%= KotlinHelper.fix_name(param.name) %> == null) { +<%= ' ' * more_index %> null +<%= ' ' * more_index %> } else { +<%= ' ' * more_index %> TWDataCreateWithBytes(<%= KotlinHelper.fix_name(param.name) %>.toUByteArray().toCValues(), <%= KotlinHelper.fix_name(param.name) %>.size.toULong()) +<%= ' ' * more_index %> } +<% else -%> +<%= ' ' * more_index %> val <%= KotlinHelper.fix_name(param.name) %>Data = TWDataCreateWithBytes(<%= KotlinHelper.fix_name(param.name) %>.toUByteArray().toCValues(), <%= KotlinHelper.fix_name(param.name) %>.size.toULong()) +<% end -%> +<% elsif param.type.name == :string -%> +<% if param.type.is_nullable -%> +<%= ' ' * more_index %> val <%= KotlinHelper.fix_name(param.name) %>String = if (<%= KotlinHelper.fix_name(param.name) %> == null) { +<%= ' ' * more_index %> null +<%= ' ' * more_index %> } else { +<%= ' ' * more_index %> TWStringCreateWithUTF8Bytes(<%= KotlinHelper.fix_name(param.name) %>) +<%= ' ' * more_index %> } +<% else -%> +<%= ' ' * more_index %> val <%= KotlinHelper.fix_name(param.name) %>String = TWStringCreateWithUTF8Bytes(<%= KotlinHelper.fix_name(param.name) %>) +<% end -%> +<% end -%> +<% end -%> diff --git a/codegen/lib/templates/kotlin/ios_parameter_release.erb b/codegen/lib/templates/kotlin/ios_parameter_release.erb new file mode 100644 index 00000000000..8f90bb6aeaf --- /dev/null +++ b/codegen/lib/templates/kotlin/ios_parameter_release.erb @@ -0,0 +1,23 @@ +<% + method = locals[:method] + more_index = locals[:more_index] || 0 + + method.parameters.each do |param| -%> +<% if param.type.name == :data -%> +<% if param.type.is_nullable -%> +<%= ' ' * more_index %> if (<%= KotlinHelper.fix_name(param.name) %>Data != null) { +<%= ' ' * more_index %> TWDataDelete(<%= KotlinHelper.fix_name(param.name) %>Data) +<%= ' ' * more_index %> } +<% else -%> +<%= ' ' * more_index %> TWDataDelete(<%= KotlinHelper.fix_name(param.name) %>Data) +<% end -%> +<% elsif param.type.name == :string -%> +<% if param.type.is_nullable -%> +<%= ' ' * more_index %> if (<%= KotlinHelper.fix_name(param.name) %>String != null) { +<%= ' ' * more_index %> TWStringDelete(<%= KotlinHelper.fix_name(param.name) %>String) +<%= ' ' * more_index %> } +<% else -%> +<%= ' ' * more_index %> TWStringDelete(<%= KotlinHelper.fix_name(param.name) %>String) +<% end -%> +<% end -%> +<% end -%> diff --git a/codegen/lib/templates/kotlin/ios_struct.erb b/codegen/lib/templates/kotlin/ios_struct.erb index 6bf3da876ee..08fbcc34ef8 100644 --- a/codegen/lib/templates/kotlin/ios_struct.erb +++ b/codegen/lib/templates/kotlin/ios_struct.erb @@ -1,5 +1,7 @@ <%= render('kotlin/package.erb') %> +import kotlinx.cinterop.toCValues + actual object <%= entity.name %> { <%# Static property declarations -%> <% entity.static_properties.each do |property| -%> @@ -10,7 +12,11 @@ actual object <%= entity.name %> { <% entity.static_methods.each do |method| -%> <% next if method.name.start_with?('Create') -%> - actual fun <%= KotlinHelper.format_name(method.name) %>(<%= KotlinHelper.parameters(method.parameters) %>)<%= KotlinHelper.return_type(method.return_type) %> = - <%= KotlinHelper.convert_calling_return_type_ios(method.return_type, "TW#{entity.name}#{method.name}(#{KotlinHelper.calling_parameters_ios(method.parameters)})") %> + actual fun <%= KotlinHelper.format_name(method.name) %>(<%= KotlinHelper.parameters(method.parameters) %>)<%= KotlinHelper.return_type(method.return_type) %> { +<%= render('kotlin/ios_parameter_access.erb', { method: method }) -%> + val result = <%= KotlinHelper.convert_calling_return_type_ios(method.return_type, "TW#{entity.name}#{method.name}(#{KotlinHelper.calling_parameters_ios(method.parameters)})") %> +<%= render('kotlin/ios_parameter_release.erb', { method: method }) -%> + return result + } <% end -%> } diff --git a/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/AnySigner.kt b/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/AnySigner.kt index 6de0b110bea..480d260002f 100644 --- a/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/AnySigner.kt +++ b/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/AnySigner.kt @@ -4,17 +4,33 @@ package com.trustwallet.core +import kotlinx.cinterop.toCValues + actual object AnySigner { - actual fun sign(input: ByteArray, coin: CoinType): ByteArray = - TWAnySignerSign(input.toTwData(), coin.value)!!.readTwBytes()!! + actual fun sign(input: ByteArray, coin: CoinType): ByteArray { + val inputData = TWDataCreateWithBytes(input.toUByteArray().toCValues(), input.size.toULong()) + val result = TWAnySignerSign(inputData, coin.value)!!.readTwBytes()!! + TWDataDelete(inputData) + return result + } actual fun supportsJson(coin: CoinType): Boolean = TWAnySignerSupportsJSON(coin.value) - actual fun signJson(json: String, key: ByteArray, coin: CoinType): String = - TWAnySignerSignJSON(json.toTwString(), key.toTwData(), coin.value).fromTwString()!! + actual fun signJson(json: String, key: ByteArray, coin: CoinType): String { + val jsonString = TWStringCreateWithUTF8Bytes(json) + val keyData = TWDataCreateWithBytes(key.toUByteArray().toCValues(), key.size.toULong()) + val result = TWAnySignerSignJSON(jsonString, keyData, coin.value).fromTwString()!! + TWStringDelete(jsonString) + TWDataDelete(keyData) + return result + } - actual fun plan(input: ByteArray, coin: CoinType): ByteArray = - TWAnySignerPlan(input.toTwData(), coin.value)?.readTwBytes()!! + actual fun plan(input: ByteArray, coin: CoinType): ByteArray { + val inputData = TWDataCreateWithBytes(input.toUByteArray().toCValues(), input.size.toULong()) + val result = TWAnySignerPlan(inputData, coin.value)?.readTwBytes()!! + TWDataDelete(inputData) + return result + } } diff --git a/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/ByteArrayExt.kt b/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/ByteArrayExt.kt index ec5923272a1..bc537de453d 100644 --- a/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/ByteArrayExt.kt +++ b/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/ByteArrayExt.kt @@ -8,9 +8,10 @@ import kotlinx.cinterop.COpaquePointer import kotlinx.cinterop.readBytes import kotlinx.cinterop.toCValues +// Build ByteArray from TWData, and then delete TWData internal fun COpaquePointer?.readTwBytes(): ByteArray? = - TWDataBytes(this)?.readBytes(TWDataSize(this).toInt()) - -@OptIn(ExperimentalUnsignedTypes::class) -internal fun ByteArray?.toTwData(): COpaquePointer? = - TWDataCreateWithBytes(this?.toUByteArray()?.toCValues(), this?.size?.toULong() ?: 0u) + this?.let { + val result = TWDataBytes(it)?.readBytes(TWDataSize(it).toInt()) + TWDataDelete(it) + result + } diff --git a/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/StringExt.kt b/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/StringExt.kt index 2eb7fdd311e..79cd8125d84 100644 --- a/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/StringExt.kt +++ b/kotlin/wallet-core-kotlin/src/iosMain/kotlin/com/trustwallet/core/StringExt.kt @@ -4,12 +4,13 @@ package com.trustwallet.core -import kotlinx.cinterop.COpaquePointer import kotlinx.cinterop.CValuesRef import kotlinx.cinterop.toKString -internal fun String?.toTwString(): COpaquePointer? = - this?.let { TWStringCreateWithUTF8Bytes(it) } - +// Build String from TWString, and then delete TWString internal fun CValuesRef<*>?.fromTwString(): String? = - this?.let { TWStringUTF8Bytes(it)?.toKString() } + this?.let { + val result = TWStringUTF8Bytes(it)?.toKString() + TWStringDelete(it) + result + }