alloy-rs / core

High-performance, well-tested & documented core libraries for Ethereum, in Rust
https://alloy.rs
Apache License 2.0
790 stars 155 forks source link

[Feature] Approach for migrating `DynSolValue` from `ethers-rs` to `alloy/core` #612

Closed wtdcode closed 7 months ago

wtdcode commented 7 months ago

Component

dyn-abi

Describe the feature you would like

Currently, there is no straightforward way to implement (De)/Serialize for DynSolValue. #[serde(remote = ...)] doesn't work due to Vec<DynSolValue> presents. This prevents me from migrating from ethers-rs to alloy/core because the original implementation from ethabi allows Token to be (de)serializable.

I would like to ask how to achieve it without implementing (De)/Serialize by my own, which is painful and error-prone.

Additional context

See #611 for the alternative approach. Since deriving is not accepted, I want to keep this issue open because of https://github.com/gakonst/ethers-rs/issues/2667

onbjerg commented 7 months ago

Hey @wtdcode, can you explain your use case a bit?

wtdcode commented 7 months ago

Hey @wtdcode, can you explain your use case a bit?

I need to serialize this to cross process boundaries (either shared memory or HTTP etc) and this was doable with previous ethers-rs.

DaniPopes commented 7 months ago

The canonical representation of dynsolvalue is the ABI encoded bytes, and alongside the type name it can be losslessly encoded and decoded

wtdcode commented 7 months ago

The canonical representation of dynsolvalue is the ABI encoded bytes, and alongside the type name it can be losslessly encoded and decoded

If it really the canonical format, it is still possible to implement (De)/Serialize for DynSolValue, no? Without this, it is super inconvenient because of the type bounds. Say you can indeed erase the types when passing messages but then you have to create another struct (I'm currently doing this) to hold the typed values. Imagine something like:

fn send_message<T: Serialize>(msg: T);
fn recv_message<T: Desrialize>(...) -> T;

We could have everything in one struct:

#[derive(Serialize, Deserialize)]
struct Message {
    pub x: u64,
    pub y: JsonAbi,
    pub z: Vec<DynSolValue> // We can't have this unfortunately
}

But now I have to do

#[derive(Serialize, Deserialize)]
struct Message {
    pub x: u64,
    pub y: JsonAbi,
    pub z: Vec<u8>
}

struct DecodedMessage {
    pub x: u64,
    pub y: JsonAbi,
    pub z: Vec<DynSolValue>
}

which is really not elegant and doesn't fit all my use cases because this solution implicitly requires two sides know ABI in advance.

It doesn’t work if passing arbitrary values without abi negotiated.

wtdcode commented 7 months ago

I'm just feeling confused, serde inherently just tells compilers the layout of an object, pretty like how Debug trait is derived. It by itself should not bind to any representations. For example, if the structs including DynSolValue are derived with Serialize, I can serialize it to any formats I feel happy with, including but not limited to json, yaml, toml etc. If there were really some crates like serde_evm_abi (not sure if it's there), we can encode any struct derived Serialize to evm abi bytes. I mean:

The serde derives has nothing to do with the canonical formats. serde by design decouples the output format and object layouts.

For example, in foundry-rs/compilers, this corresponds to the json abi, but actually you can also call serde_yaml::to_writer with that object even if the canonical format is obviously json.

In addition, as you point out, if the abi encoded bytes is the canonical, personally DynSolValue should be indeed derived with (De)/Serialize so that we could have something like serde_evm_abi to do the job without creating duplicate code.

Hope it clarifies my thoughts.

wtdcode commented 7 months ago

Attach a rather simple sample to illustrate this:

use alloy_json_abi::JsonAbi;

fn main() {
    let abi = r#"
[{"constant":true,"inputs":[],"name":"name","outputs":[{"name":"","type":"string"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"guy","type":"address"},{"name":"wad","type":"uint256"}],"name":"approve","outputs":[{"name":"","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"totalSupply","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"src","type":"address"},{"name":"dst","type":"address"},{"name":"wad","type":"uint256"}],"name":"transferFrom","outputs":[{"name":"","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[{"name":"wad","type":"uint256"}],"name":"withdraw","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":true,"inputs":[],"name":"decimals","outputs":[{"name":"","type":"uint8"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"name":"","type":"address"}],"name":"balanceOf","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"symbol","outputs":[{"name":"","type":"string"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"dst","type":"address"},{"name":"wad","type":"uint256"}],"name":"transfer","outputs":[{"name":"","type":"bool"}],"payable":false,"stateMutability":"nonpayable","type":"function"},{"constant":false,"inputs":[],"name":"deposit","outputs":[],"payable":true,"stateMutability":"payable","type":"function"},{"constant":true,"inputs":[{"name":"","type":"address"},{"name":"","type":"address"}],"name":"allowance","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"payable":true,"stateMutability":"payable","type":"fallback"},{"anonymous":false,"inputs":[{"indexed":true,"name":"src","type":"address"},{"indexed":true,"name":"guy","type":"address"},{"indexed":false,"name":"wad","type":"uint256"}],"name":"Approval","type":"event"},{"anonymous":false,"inputs":[{"indexed":true,"name":"src","type":"address"},{"indexed":true,"name":"dst","type":"address"},{"indexed":false,"name":"wad","type":"uint256"}],"name":"Transfer","type":"event"},{"anonymous":false,"inputs":[{"indexed":true,"name":"dst","type":"address"},{"indexed":false,"name":"wad","type":"uint256"}],"name":"Deposit","type":"event"},{"anonymous":false,"inputs":[{"indexed":true,"name":"src","type":"address"},{"indexed":false,"name":"wad","type":"uint256"}],"name":"Withdrawal","type":"event"}]
    "#;

    let abi: JsonAbi = serde_json::from_str(&abi).unwrap();

    let yaml_abi = serde_yaml::to_string(&abi).unwrap();

    println!("YAML ABI:\n{}", yaml_abi);
}

Output:

YAML ABI:
- type: fallback
  stateMutability: payable
- type: function
  name: allowance
  inputs:
  - name: ''
    type: address
  - name: ''
    type: address
  outputs:
  - name: ''
    type: uint256
  stateMutability: view
- type: function
  name: approve
  inputs:
  - name: guy
    type: address
  - name: wad
    type: uint256
  outputs:
  - name: ''
    type: bool
  stateMutability: nonpayable
...
prestwich commented 7 months ago

The serde derives has nothing to do with the canonical formats. serde by design decouples the output format and object layouts.

We know this. I think we're miscommunicating about what formats we value and why we implement Serialize for anything at all. alloy-core exists to implement standard data formats that are used across many different libraries and applications across the ethereum ecosystem. E.g. ABI encoding, ABI interface specification JSON, RLP, EIP-712 eth_signTypedData objects, signature vrs, etc.

Yes, you are free to make a YAML representation using the Serialize trait, but it won't interoperate with any other system. We add the Serialize derives to target specific industry-standard data formats that are broadly used. We're not trying to make new data formats.

Because there is no industry-standard format for DynSolValue values (as they're an alloy implementation detail), we won't be adding Serialize derives to it. If you want to add application-specific serde logic for DynSolValue you can pretty easily do that with serde(with = "...") and a custom (de)serialize module

wtdcode commented 7 months ago

Could you show me how to do that with serde(with=…) ?

Btw, thanks to @DaniPopes, it has canonical format, no?

Lastly, it is still an open issue. It's a bit offensive to close it without any concrete fix or workarounds.


From: James Prestwich @.> Sent: Wednesday, May 1, 2024 12:42:00 AM To: alloy-rs/core @.> Cc: lazymio @.>; Mention @.> Subject: Re: [alloy-rs/core] [Feature] Approach for migrating DynSolValue from ethers-rs to alloy/core (Issue #612)

The serde derives has nothing to do with the canonical formats. serde by design decouples the output format and object layouts.

We know this. I think we're miscommunicating about what formats we value and why we implement Serialize for anything at all. alloy-core exists to implement standard data formats that are used across many different libraries and applications across the ethereum ecosystem. E.g. ABI encoding, ABI interface specification JSON, RLP, EIP-712 eth_signTypedData objects, signature vrs, etc.

Yes, you are free to make a YAML representation using the Serialize trait, but it won't interoperate with any other system. We add the Serialize derives to target specific industry-standard data formats that are broadly used. We're not trying to make new data formats.

Because there is no industry-standard format for DynSolValue values (as they're an alloy implementation detail), we won't be adding Serialize derives to it. If you want to add application-specific serde logic for DynSolValue you can pretty easily do that with serde(with = "...") and a custom (de)serialize module

― Reply to this email directly, view it on GitHubhttps://github.com/alloy-rs/core/issues/612#issuecomment-2085935739, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHJULOZ2ZZPWSBJXANOLWSDY77CVRAVCNFSM6AAAAABG4IFAVSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBVHEZTKNZTHE. You are receiving this because you were mentioned.Message ID: @.***>