Add serde traits to Auth

[RCasatta]

This would allow to ser/des RpcConfig downstream struct

[sgeisler]

Concept ACK, but we should discuss the representation.

[sgeisler]

I wonder if it would be worth it implementing the serde logic manually because none of the available tagging options feel right. The default, external tagging, would result in:

"auth_field": {
  "UserPass": ["user", "pass"]
}

or

"auth_field": {
  "CookieFile": "/path/to/file"
}

I think we should at least add variant renaming to snake_case. But ideally it would look like this imo:

"auth_field": {
  "user": "user",
  "pass": "pass"
}

or

"auth_field": {
  "cookie_file": "/path/to/file"
}

We could achieve this with untagged serialization and changing the tuple variants to struct variants, but that would be an API break and would make direct usage more cumbersome. We could add constructors to ease the pain, but the API break would remain.

[RCasatta]

@sgeisler I updated the PR to serialize in a more comprehensible way and at the same time trying to be non-breaking and easily upgradable

we may want to add #[serde(untagged)] but I slightly prefer the default

[sgeisler]

Without #[serde(untagged)] it still seems unnecessarily deeply nested 😕 I think with #[serde(untagged)] and the CookieFile variant becoming a named struct instead of a tuple struct that might be quite nice. If you feel strongly about it I'm ok with this too though.

Also, if we were feeling really fancy we'd avoid the clone on serializing by having two structs: one owned and one borrowed.

[RCasatta]

Without #[serde(untagged)] it still seems unnecessarily deeply nested 😕 I think with #[serde(untagged)] and the CookieFile variant becoming a named struct instead of a tuple struct that might be quite nice. If you feel strongly about it I'm ok with this too though.

I am not feeling strongly about it and I added #[serde(untagged)] (but I wouldn't say "unnecessarily deeply nested" since there are reasons to prefer it...)
I also added the named variant for Cookie

Also, if we were feeling really fancy we'd avoid the clone on serializing by having two structs: one owned and one borrowed.

I don't think the code complexity is worth it in this case since serialize/deserialize of this value is done rarely

[sgeisler]

utACK 4512d5d

but I wouldn't say "unnecessarily deeply nested" since there are reasons to prefer it...

Sorry for that wording, I just didn't understand it 😄 I guess there are some problems if a parent enum is untagged too and you have collisions in field names (just don't do that 😆 ). But since this is to be used in config (I suppose) and I as a user tend to get annoyed when I have to write seemingly redundant information I think this version is better.

I don't think the code complexity is worth it in this case since serialize/deserialize of this value is done rarely

Very good point.

[stevenroose]

Sorry for the delay guys.. It's a bit unfortunate that we have to clone stuff for this. I don't think it's too hard to implement serde manually without the need for an extra struct to copy into.

Even though the only problem with just doing this on our existing struct is that user and pass are a slice. Which is also not the end of the world:

#[derive(Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
enum Auth {
    None,
    UserPass(String, String),
    CookieFile(PathBuf),
}

which gives

{"auth":"none"}
{"auth":{"user_pass":["userr","passs"]}}
{"auth":{"cookie_file":"/"}}

[stevenroose]

Just for curiosity, I tried it manually. It ends up being quite verbose like always using the visitor etc. But it works without re-allocations for serializing: stevenroose@04dd65b

I'm fine also just merging this if that's preferred.

[RCasatta]

I am fine also with stevenroose/rust-bitcoincore-rpc@04dd65b