Fixing convert number functions for Longs #56
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @matthappens and the rest of your teammates on |
| @@ -195,7 +195,7 @@ class InvoiceBuilder { | |||
| var invoiceUUID: String? = null | |||
| var amount: Int? = null | |||
| var receivingCurrency: InvoiceCurrency? = null | |||
| var expiration: Int? = null | |||
| var expiration: Number? = null | |||
There was a problem hiding this comment.
Why use Number instead of Long directly? I think we're going to need to be explicit about 64-bit ints to be consistent, right?
There was a problem hiding this comment.
converted all back to Int / Long, it's a bit clearer. during serialization, the values will be converted to their smallest possible representation, so Long fields might actually be serialized as small as a byte, but this detail is now better hidden inside of the getLong/getInt functions
| @@ -77,7 +77,7 @@ data class Invoice( | |||
| /** The currency of the invoice */ | |||
| val receivingCurrency: InvoiceCurrency, | |||
| /** The unix timestamp the UMA invoice expires */ | |||
| val expiration: Int, | |||
| val expiration: Number, | |||
There was a problem hiding this comment.
This one should be a Long then, right?
There was a problem hiding this comment.
good catch, it should. I'll switch amount to long as well, to cover msat
| @@ -141,12 +141,12 @@ data class Invoice( | |||
| when (bytes[offset].toInt()) { | |||
| 0 -> ib.receiverUma = bytes.getString(offset.valueOffset(), length) | |||
| 1 -> ib.invoiceUUID = bytes.getString(offset.valueOffset(), length) | |||
| 2 -> ib.amount = bytes.getNumber(offset.valueOffset(), length) | |||
| 2 -> ib.amount = bytes.getInt(offset.valueOffset(), length) | |||
There was a problem hiding this comment.
Should this be a long too? Could just see for units like msats, this could get pretty big. Not sure if @zhenlu was thinking of this as an int or long, though. I know it's a Long in UMA generally.
Long Serialization
There was an issue serializing longs in UMA Invoices -
most often, the expiration field is a long, since it represents a unix timestamp
This change converts the field to a long and handles serialization/deserialization of said long