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

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

Closed
wtdcode opened this issue Apr 27, 2024 · 8 comments
Closed
Labels
enhancement New feature or request

Comments

@wtdcode
Copy link
Contributor

wtdcode commented Apr 27, 2024

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 gakonst/ethers-rs#2667

@wtdcode wtdcode added the enhancement New feature or request label Apr 27, 2024
@onbjerg
Copy link
Member

onbjerg commented Apr 30, 2024

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

@wtdcode
Copy link
Contributor Author

wtdcode commented Apr 30, 2024

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

@wtdcode
Copy link
Contributor Author

wtdcode commented Apr 30, 2024

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

wtdcode commented Apr 30, 2024

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

wtdcode commented Apr 30, 2024

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

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

@prestwich prestwich closed this as not planned Won't fix, can't repro, duplicate, stale Apr 30, 2024
@wtdcode
Copy link
Contributor Author

wtdcode commented Apr 30, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants