paradigmxyz / reth

Modular, contributor-friendly and blazing-fast implementation of the Ethereum protocol, in Rust
https://reth.rs/
Apache License 2.0
3.99k stars 1.21k forks source link

chore(sdk): define helper trait `BlockPrimitives` #12721

Open emhane opened 1 day ago

emhane commented 1 day ago

Based on https://github.com/paradigmxyz/reth/pull/12647

klkvr commented 1 day ago
  • Fixes bug in stages. Bug: writing compact encoded signed transaction instead of the unsigned transaction to db

I believe we've been always writing signed transaction to both static files and database? https://github.com/paradigmxyz/reth/blob/166a2346dcd2a9c8d5fea6995a9fc5e6426c62bb/crates/storage/provider/src/providers/static_file/writer.rs#L543-L547 https://github.com/paradigmxyz/reth/blob/166a2346dcd2a9c8d5fea6995a9fc5e6426c62bb/crates/storage/db/src/tables/mod.rs#L295

emhane commented 1 day ago
  • Fixes bug in stages. Bug: writing compact encoded signed transaction instead of the unsigned transaction to db

I believe we've been always writing signed transaction to both static files and database?

https://github.com/paradigmxyz/reth/blob/166a2346dcd2a9c8d5fea6995a9fc5e6426c62bb/crates/storage/provider/src/providers/static_file/writer.rs#L543-L547

https://github.com/paradigmxyz/reth/blob/166a2346dcd2a9c8d5fea6995a9fc5e6426c62bb/crates/storage/db/src/tables/mod.rs#L295

TransactionSigned doesn't impl Compact though

klkvr commented 1 day ago

It does since https://github.com/paradigmxyz/reth/pull/12539

related discussion https://github.com/paradigmxyz/reth/pull/12694#discussion_r1851075736

emhane commented 1 day ago

then it should impl Compact, but it doesn't. before StaticFileProviderRW::append_transaction took TransactionSignedNoHash. @klkvr

klkvr commented 1 day ago

then it should impl Compact, but it doesn't

It does impl Compact https://github.com/paradigmxyz/reth/blob/e3702cfc87449294da061f02a571a23061b8ab50/crates/primitives/src/transaction/mod.rs#L1054

before StaticFileProviderRW::append_transaction took TransactionSignedNoHash

yep, but now it takes anything that implements Compact (should probably change that to N::SignedTx given that we have primitives on StaticFileProvider now)

sorry for the confusion with the 2 storage types, I was expecting the TransactionSignedNoHash to be gone by now

emhane commented 1 day ago

ok nvm, switching between a lot of branches to shrink scope, my bad

emhane commented 1 day ago

re-request review @klkvr