gakonst / ethers-rs

Complete Ethereum & Celo library and wallet implementation in Rust. https://docs.rs/ethers
Apache License 2.0
2.49k stars 795 forks source link

Chain has Serialization/Deserialization mismatch #2038

Open plotchy opened 1 year ago

plotchy commented 1 year ago

Description Using serde_json to save serializations of Chains within files, and later deserializing the files back into Chain objects.

Ran into an issue with the Chain::BinanceSmartChain deserialization failing. Below is a test showing the issue:

#[tokio::test]
    async fn test_chain_deser() {
        let chain = Chain::Mainnet;
        let chain_str = serde_json::to_string(&chain).unwrap(); // "\"mainnet\""
        dbg!(&chain_str);
        let chain_deser: Chain = serde_json::from_str(&chain_str).unwrap(); // Mainnet
        dbg!(&chain_deser);
        assert_eq!(chain, chain_deser); // passes

        let chain = Chain::BinanceSmartChain;
        let chain_str = serde_json::to_string(&chain).unwrap(); // "\"bsc\""
        dbg!(&chain_str);
        let chain_deser: Chain = serde_json::from_str(&chain_str).unwrap(); // panic. "bsc" is not a valid chain
        assert_eq!(chain, chain_deser); // not reached
    }

Here is the output of the test:

running 1 test
[src/lib.rs:288] &chain_str = "\"mainnet\""
[src/lib.rs:290] &chain_deser = Mainnet
[src/lib.rs:295] &chain_str = "\"bsc\""
thread 'tests::test_chain_deser' panicked at 'called `Result::unwrap()` on an `Err` value: Error("unknown variant `bsc`, expected one of `mainnet`, `morden`, `ropsten`, `rinkeby`, `goerli`, `kovan`, `sepolia`, `optimism`, `optimism_kovan`, `optimism_goerli`, `arbitrum`, `arbitrum_testnet`, `arbitrum_goerli`, `arbitrum_nova`, `cronos`, `cronos_testnet`, `rsk`, `binance_smart_chain`, `binance_smart_chain_testnet`, `poa`, `sokol`, `x_dai`, `polygon`, `polygon_mumbai`, `fantom`, `fantom_testnet`, `moonbeam`, `moonbeam_dev`, `moonriver`, `moonbase`, `dev`, `anvil_hardhat`, `evmos`, `evmos_testnet`, `chiado`, `oasis`, `emerald`, `emerald_testnet`, `avalanche`, `avalanche_fuji`, `celo`, `celo_alfajores`, `celo_baklava`, `aurora`, `aurora_testnet`", line: 1, column: 5)', src/lib.rs:296:67

Looks like it wants to see "binance_smart_chain" I imagine this occurs similarly for the other #[strum(serialize = ...)] Chains as well. link to Chain enum

mattsse commented 1 year ago

idk why strum serialize serializes as str as escaped string

@DaniPopes mind taking a look?

plotchy commented 1 year ago

FYI Adding in a line for a serde alias lets the test pass.

    #[strum(serialize = "bsc")]
    #[serde(alias = "bsc")]
    BinanceSmartChain = 56,

Output:

running 1 test
[src/lib.rs:288] &chain_str = "\"mainnet\""
[src/lib.rs:290] &chain_deser = Mainnet
[src/lib.rs:295] &chain_str = "\"bsc\""
[src/lib.rs:297] &chain_deser = BinanceSmartChain
test tests::test_chain_deser ... ok
DaniPopes commented 1 year ago

idk why strum serialize serializes as str as escaped string

that's just serde_json adding quotes for json and dbg! using Debug on a str escaping it

What ethers version / ref are you using? @plotchy

plotchy commented 1 year ago

@DaniPopes ethers-core v1.0.2 (https://github.com/gakonst/ethers-rs#0187bedd)

DaniPopes commented 1 year ago

Ok that's latest commit on master, but this has been a thing forever: the problem here is that we derive Deserialize which uses snake_case while everything else uses kebab-case and strum aliases.

I'm not sure how to fix this other than adding each and every kebab-case + alias to serde too, or a custom Deserialize impl with manual matching for snake_case + fallback to String::deserialize -> FromStr... @mattsse

plotchy commented 1 year ago

Revisiting this from a long time ago. Noticing your comment in https://github.com/gakonst/ethers-rs/pull/2270

Doesn't fix it because you still need to replace kebab-case to snake_case

for my needs that fix worked great. Feel free to keep this open as an issue to track that if you'd like, but just wanted to let you know im satisfied with the patch