StratisBitcoinFullNode
StratisBitcoinFullNode copied to clipboard
[3.0.8.0] API models shouldn't use string to hold Value amounts
https://stratisplatformuk.visualstudio.com/StratisBitcoinFullNode/_workitems/edit/3904/
Re: "... currently MoneyFormatAttribute is causing us problems because Money.TryParse doesn't understand scientific notation ..."
I suggest that we improve consistency without changing the APIs in any way. The root issue seems to be that we (or external callers) are not always using the ToString method of the Money class to convert money values to strings. As a result we end up with a string that is not guaranteed to be parsable by the Money class.
Changes in this PR:
- Don't use
decimal.ToString. UseMoney.ToStringinstead. - Allow strings with scientific notation to be passed to
Money.TryParse(for external callers).
Code looks good, I don't really understand the issue it's fixing though? Unless we want sci notation to be allowed?
I think the issue is talking about the request models being passed to the controller?
The idea of this PR is to deal with the two issues (in a way that avoids changing the API):
- scientific notation being passed to us by external callers
- consistency in how we ourselves convert money values to strings.
As suggested by @codingupastorm , when we make the next version of the API, we can ensure that we use the same type for the same data - i.e. avoid passing money values as strings.
Alternatively adding these changes will give us backwards compatibility:
public override object ReadJson(JsonReader reader, Type objectType, object existingValue,
JsonSerializer serializer)
{
if (reader.Value is string strValue && Money.TryParse(strValue, out Money value))
return value.ToDecimal(MoneyUnit.BTC);
return (decimal)reader.Value;
}
public override bool CanRead => true;
public override bool CanConvert(Type objectType) => objectType == typeof(decimal);
Adding that code will make BtcDecimalJsonConverter accept quoted or unquoted amounts - including scientific notation.
[JsonConverter(typeof(BtcDecimalJsonConverter))]
public decimal? OpReturnAmount { get; set; }
after this we won't need MoneyFormat