Skip to content

Conversation

vishnumad
Copy link
Contributor

Summary

  • Removes manual serialization and deserialization for messages
  • Improvements to Java interop
  • Misc fixes

How did you test your changes?

Tested manually with CB Wallet to make sure changes are backwards compatible with older versions

return instances[wallet.url] ?: CoinbaseWalletSDK(
hostPackageName = wallet.packageName,
scheme = wallet.url,
keyManager = KeyManager(ClientConfiguration.config.context, wallet.packageName)
)
}

fun handleResponse(uri: Uri): Boolean {
@JvmStatic
fun handleResponseUrl(uri: Uri): Boolean {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed name here to prevent naming clash when marking function with @JvmStatic


typealias UnencryptedRequestMessage = Message<UnencryptedRequestContent>

sealed interface RequestContent {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping this RequestContent to keep interface CoinbaseWalletSDK.makeRequest the same

"Must initialize open intent callback before getting CoinbaseWalletSDK instance"
)
if (openIntent == null) {
throw IllegalStateException("Must initialize open intent callback before getting CoinbaseWalletSDK instance")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this to throw IllegalStateException because this error is not returned from the wallet

import java.security.interfaces.ECPrivateKey
import java.security.interfaces.ECPublicKey

object RequestConverter : MessageConverter<UnencryptedRequestMessage, EncryptedRequestMessage> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some code duplication with RequestConverter and ResponseConverter, but I was not able to get the kotlin serialization library to play nicely with generic types.

@vishnumad vishnumad marked this pull request as ready for review December 9, 2022 02:53
@vishnumad vishnumad requested a review from bangtoven December 9, 2022 02:53
@@ -14,13 +14,16 @@ data class Wallet(
}

object DefaultWallets {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a scope for this pr, but i think we might just consolidate Wallet and DefaultWallets into one, with the thing called companion object or something on kotlin world.
on swift, those default wallets and static method to get those are defined under Wallet
image

@vishnumad vishnumad merged commit 6771a9c into master Dec 9, 2022
@vishnumad vishnumad deleted the vishnu/android-serialization-changes branch December 9, 2022 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants