alloy-rs / core

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

Add custom serialization for Address #742

Closed jenpaff closed 2 months ago

jenpaff commented 2 months ago

Motivation

Partially closes https://github.com/foundry-rs/foundry/issues/8715 We want to serialize Address type in checksum format.

Solution

Solution is to implement a custom serialization function for Address type.

I ran into some issues: 1) The impl_serde macro conflicts with impl Serialize for Address (Address is wrapped with the wrap_fixed_bytes macro which calls impl_serde), so I added a workaround in impl_serde to bypass Address type. Another solution would be to just move the impl directly into the macro- if preferred.

2) There's another type EIP712Domain which uses the type Address, I wanted to double check that it is okay for this to also be serialized in checksum format, I added another unit test to make it explicit.

PR Checklist

zerosnacks commented 2 months ago
  1. Implementation looks good, I don't think the NOOP behavior in the macro is a problem but documenting it would be good. Perhaps there is a better way of handling this.
  2. It doesn't look like EIP712 enforces EIP-55, and nothing specifically relating to the verifyingContract field. From preliminary tests it looks like checksumming was already handled here so the resulting hash after the proposed changes should stay the same - worth double checking.

cc @DaniPopes please correct me if I'm wrong

Tip: for CI fmt task failing run: cargo +nightly fmt --all --check