-
Notifications
You must be signed in to change notification settings - Fork 35
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
Version 1.6.2204 #85
Version 1.6.2204 #85
Conversation
MrStahlfelge
commented
Jan 26, 2022
- ErgoPay implementation
- Portuguese and Italian translation added
- Payment request URI scheme changed to ergo:
- Spanish translation improved
- Appkit upgraded to 4.0.6
- Small bug fixes and improvements
…ansactionInfo data class #79
… name/decimals when available #79
…rgoPaySigningUiLogic #79
…w things … (#81) * Fixed a sentence that didnt fit properly + translated the new things added * Remove untranslatable, run convertIos Co-authored-by: Benjamin Schulte <[email protected]>
* Android * Fixed the word "única" + changed "tx" to "transação" since the abbreviation doesn't make sense in PT * Portuguese activated for and converted to iOS Co-authored-by: Benjamin Schulte <[email protected]>
…equest differs from signing addresses, received no reducedTx but message #79
…ltiple addresses #79
Ergopay reference implementation
* Italian translation * Fixed Italian translation for Android * Converted Italian for iOS Co-authored-by: Benjamin Schulte <[email protected]>
android/src/main/java/org/ergoplatform/android/transactions/ErgoPaySigningFragment.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/org/ergoplatform/android/transactions/SubmitTransactionViewModel.kt
Show resolved
Hide resolved
.map { Base64Coder.decode(it.asString) } | ||
} else null | ||
val inputs = jsonTree.get(JSON_FIELD_INPUTS)?.asJsonArray?.toList() | ||
?.map { Base64Coder.decode(it.asString) } |
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.
Such code fragments are good candidates for encapsulation into extension methods.
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.
What would you suggest to encapsulate here?
Unfortunately Gson is not very Kotlin-friendly anyway. Using kotlinx-serialization would make better code here. But we already have Gson pulled in, and pulling in a second JSON parser is something I want to avoid.
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.
I keep something like this in mind
jsonTree.decodeBase64StringList(JSON_FIELD_INPUTS)
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.
Changed it, although I am not sure as this is a very specific method
|
||
/** | ||
* EIP-0020 ErgoPaySigningRequest | ||
* everything is optional, but it should have either reducedTx or message |
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.
this requirement can be checked in the constructor.
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.
I had the wrong assumption that data classes can't hold any code, turned out it is possible to have such a check. Will add.
val message: String? = null, | ||
val messageSeverity: MessageSeverity = MessageSeverity.NONE, | ||
val replyToUrl: String? = null | ||
) |
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.
Please add documentation to all classes, methods and properties in this file.
private const val PARAM_DELIMITER = "&" | ||
private const val RECIPIENT_PARAM_PREFIX = "address=" | ||
private const val AMOUNT_PARAM_PREFIX = "amount=" | ||
private const val TOKEN_PARAM_PREFIX = "token-" |
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.
why -
in the prefix?
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.
Token prefix is token-<ErgoID>=
, the -
is the separator between the prefix and the actual id. (https://github.com/ergoplatform/eips/blob/master/eip-0025.md#format)
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.
Please put this answer to the doc comment for TOKEN_PARAM_PREFIX