Skip to content

Commit

Permalink
[KMP] Fix issue: memory leak found in Base58.decode in iOS (#4031)
Browse files Browse the repository at this point in the history
* 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 57eee39.

* 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>
  • Loading branch information
10gic and satoshiotomakan authored Oct 1, 2024
1 parent 00a8744 commit 2ed7098
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 44 deletions.
25 changes: 12 additions & 13 deletions codegen/lib/kotlin_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -56,7 +62,7 @@ def self.fix_name(name)
when ''
"value"
when 'val'
"value"
"value"
when 'return'
'`return`'
else
Expand All @@ -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

Expand Down
50 changes: 39 additions & 11 deletions codegen/lib/templates/kotlin/ios_class.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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') } -%>
Expand All @@ -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 -%>
Expand All @@ -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<TW<%= entity.name %>> {
<%= 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| -%>

Expand All @@ -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 -%>
Expand Down
8 changes: 6 additions & 2 deletions codegen/lib/templates/kotlin/ios_enum.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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? } -%>
Expand Down
27 changes: 27 additions & 0 deletions codegen/lib/templates/kotlin/ios_parameter_access.erb
Original file line number Diff line number Diff line change
@@ -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 -%>
23 changes: 23 additions & 0 deletions codegen/lib/templates/kotlin/ios_parameter_release.erb
Original file line number Diff line number Diff line change
@@ -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 -%>
10 changes: 8 additions & 2 deletions codegen/lib/templates/kotlin/ios_struct.erb
Original file line number Diff line number Diff line change
@@ -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| -%>
Expand All @@ -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 -%>
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 2ed7098

Please sign in to comment.