Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chain has Serialization/Deserialization mismatch #2038

Open
plotchy opened this issue Jan 10, 2023 · 6 comments
Open

Chain has Serialization/Deserialization mismatch #2038

plotchy opened this issue Jan 10, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@plotchy
Copy link
Contributor

plotchy commented Jan 10, 2023

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

@plotchy plotchy added the bug Something isn't working label Jan 10, 2023
@mattsse
Copy link
Collaborator

mattsse commented Jan 10, 2023

idk why strum serialize serializes as str as escaped string

@DaniPopes mind taking a look?

@plotchy
Copy link
Contributor Author

plotchy commented Jan 10, 2023

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
Copy link
Collaborator

DaniPopes commented Jan 10, 2023

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
Copy link
Contributor Author

plotchy commented Jan 10, 2023

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

@DaniPopes
Copy link
Collaborator

DaniPopes commented Jan 10, 2023

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
Copy link
Contributor Author

plotchy commented Oct 3, 2023

Revisiting this from a long time ago. Noticing your comment in #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants