Add enum for type, subtype and aclass and add serialize trait

[Ebolon]

I added three enums for type, subtype and aclass. The Kraken doc is not on par with the real api. But it should be easy to adjust. Additionally I added the Serialize trait. I like to cache the results without the need to map the values to new objects. Sadly it is not that easy to add the serialization feature from outside. I hope you are agreeing that it make sense to have this in this library.

[gmosx]

Hmm, it looks good. I may introduce a 'serialize' feature to the crate.

[Ebolon]

I added the possibility to serialize / deserialize the f64 strings as well to an f64 type. I hope this suites for your too.

[gmosx]

Tbh, I don't really like the f64 conversion stuff. Maybe the user would like to deserialize into e.g. rust_decimal::Decimal. Can you please revert the latest commit (and for example open a separate PR?) I would like to merge the enum stuff and consider f64, etc separately.

[Ebolon]

Yes of course. I have reverted it.

[gmosx]

Thanks for reverting. I have one more question. What will happen if Kraken adds a new e.g. LedgerType that is not part of the enum. How will serde handle this case?

[Ebolon]

This would result in a runtime error. There would be the possibility to use #[serde(other)] for a default value if there is no match. But it depends what the goal should be. Get notified by an exception that the protocol is not matching or silently accept it. I personally would vote to break on a mismatch.

[gmosx]

Are you sure you would like a runtime error (panic?) if the API returned a new string for e.g. ledgerType? Let's say I had an app that was displaying ledgers, having the ledgerType as a string would be more robust. The app could display the ledger it expects (or even the 'novel' ledger).

I think using the #[serde(other)] approach is the way to go.

[Ebolon]

I would handle an unknown enum value like this. Because the result has no value for me if the semantics are not defined.

...
let resp: Result<GetLedgersResponse, kraken_sdk_rest::error::Error> = kraken_client.get_ledgers().end(unix_timestamp).ofs(i).send().await;
if resp.is_err() {
    error!("{:#?}", resp.err().unwrap());
    break;
}

prints:

Error: FailedRequest {
    err: "error decoding response body: unknown variant `staking`, expected one of `trade`, `deposit`, `withdrawal`, `transfer`, `margin`, `rollover`, `spend`, `receive`, `settled`, `adjustment` at line 1 column 241",
    status: None,
}

But never the less I added an Unknown enum value. This result in to have a "validation" in a later step.

[Ebolon]

After giving it a second thought. I will have a look if I implement the enums as trait for the response. I think a enum with and unknown state is bad practice, because you would loose information in case of serializing and deserializing it.

[Ebolon]

My goal would be that the library providing enums so I do not handle with the strings (ledger types...) myself. How do you feel about the trait idea for optional enum typing? Maybe for other types as well.

[gmosx]

Hm, that's an interesting idea,thank you for refining this MR.

One alternative idea, what about if the 'Unknown' variant of the enum was a tuple struct with a string attribute of the 'unknown' value? That way no information would be lost? Thoughts?

[gmosx]

I will merge with minor edits.

[gmosx]

In a follow-up MR, I removed the trait. AFAICS it only adds noise, for no clear benefit. (Correct me if I am wrong).