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

feat(exex, primitives): serde bincode compatibility #11331

Merged
merged 16 commits into from
Sep 30, 2024

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Sep 30, 2024

Closes #11170

Blocked by alloy-rs/op-alloy#131

@shekhirin shekhirin force-pushed the alexey/exex-serde-bincode-compat branch from e7dfa77 to 29237fd Compare September 30, 2024 14:23
@shekhirin shekhirin force-pushed the alexey/exex-serde-bincode-compat branch from 407ce0e to 5cd1508 Compare September 30, 2024 15:14
@shekhirin shekhirin force-pushed the alexey/exex-serde-bincode-compat branch from 5cd1508 to c2062c6 Compare September 30, 2024 15:15
@shekhirin shekhirin force-pushed the alexey/exex-serde-bincode-compat branch from 7b1d402 to 3e27793 Compare September 30, 2024 15:40
@shekhirin
Copy link
Collaborator Author

fails with optimism feature, fixing

SEED=4 cargo t --quiet -p reth-primitives --features serde-bincode-compat,optimism block_body_bincode

running 1 test
block::serde_bincode_compat::tests::test_block_body_bincode_roundtrip --- FAILED

failures:

---- block::serde_bincode_compat::tests::test_block_body_bincode_roundtrip stdout ----
thread 'block::serde_bincode_compat::tests::test_block_body_bincode_roundtrip' panicked at crates/primitives/src/block.rs:1086:64:
called `Result::unwrap()` on an `Err` value: InvalidTagEncoding(32)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    block::serde_bincode_compat::tests::test_block_body_bincode_roundtrip

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 99 filtered out; finished in 0.09s

error: test failed, to rerun pass `-p reth-primitives --lib`SEED=4 cargo t --quiet -p reth-primitives --features serde-bincode-compat block_body_bincode 

running 1 test
.
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 96 filtered out; finished in 0.10s

@shekhirin
Copy link
Collaborator Author

fixed by alloy-rs/op-alloy#131

cargo t -p reth-primitives --features serde-bincode-compat,optimism --no-run                  
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.34s
  Executable unittests src/lib.rs (target/debug/deps/reth_primitives-5d9f9518ba4cbf81)hyperfine -r 100 -- "./target/debug/deps/reth_primitives-5d9f9518ba4cbf81 block_body_bincode"
Benchmark 1: ./target/debug/deps/reth_primitives-5d9f9518ba4cbf81 block_body_bincode
  Time (mean ± σ):      63.6 ms ±   6.9 ms    [User: 59.5 ms, System: 1.6 ms]
  Range (min … max):    59.1 ms … 113.2 ms    100 runs

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't super pretty but should do

Comment on lines +766 to +770
blocks: value
.blocks
.iter()
.map(|(block_number, block)| (*block_number, block.into()))
.collect(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could get rid of this by manually impl serialize but for now this is okay

@shekhirin shekhirin added this pull request to the merge queue Sep 30, 2024
Merged via the queue into main with commit d6113e1 Sep 30, 2024
36 checks passed
@shekhirin shekhirin deleted the alexey/exex-serde-bincode-compat branch September 30, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bincode support for ExExNotification
2 participants