-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature parity updates #9
base: main
Are you sure you want to change the base?
Conversation
@@ -4,7 +4,7 @@ import kotlinx.serialization.* | |||
import org.onflow.flow.infrastructure.Base64ByteArray | |||
|
|||
@Serializable | |||
data class FlowAddress private constructor(val bytes: ByteArray) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why the Address class was set to private? Would like to replace FlowAddress
import from Flow JVM SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be public, I think we should get rid of flow jvm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @lmcmz,
https://github.com/onflow/flow-kmm/blob/15167f38c5fd2b51c21bcba6f5f17e8e90f35e0f/flow/src/commonMain/kotlin/org/onflow/flow/models/AccountPublicKey.kt
Is there a specific reasons fields like sequence_number
, weight
, index
are set to type String in flow-kmm? Flow-JVM assumes Int
type for these fields, so I am having to cast a lot of values .toInt() to work with some of the AccessAPI endpoints. I am working on phasing these out to use methods from flow-kmm instead, just wondering if there was a specific decision why String
was used.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, for late reply for this.
The kmm use http rpc directly, the api return in string format.
I'm happy to convert into int, we need add some custom decoder
https://developers.flow.com/http-api#tag/Accounts/paths/~1accounts~1%7Baddress%7D/get
@@ -4,7 +4,7 @@ import kotlinx.serialization.* | |||
import org.onflow.flow.infrastructure.Base64ByteArray | |||
|
|||
@Serializable | |||
data class FlowAddress private constructor(val bytes: ByteArray) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be public, I think we should get rid of flow jvm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Is it possible to add a unit test to sign a real tx to testnet chain ?
Closes: #???
Description
Looks like we will need to add iOS analog methods for the Crypto implementation added to replace functionality that was previously handled by Flow-JVM on Android.
For contributor use:
master
branchFiles changed
in the Github PR explorer