dedup: TransactionList and BlockResponse #444
Conversation
yash-atreya
requested review from
DaniPopes,
gakonst,
mattsse,
onbjerg,
prestwich and
Evalir
as code owners
| /// A block response | ||
| #[derive(serde::Serialize, serde::Deserialize, Debug, Clone)] | ||
| pub struct BlockResponse<N: Network> { | ||
| #[serde(flatten)] | ||
| header: N::HeaderResponse, | ||
| transactions: TransactionList<N::TransactionResponse>, |
There was a problem hiding this comment.
we should probably do the following while we're here:
- delete this block response as unused and close [Bug] Dedupe
alloy_consensus::BlockResponse<N>andalloy_rpc_types::Block#441 - make
TransactionListgeneric overTif it's not already
There was a problem hiding this comment.
TransactionList is already generic over T, I was removing it in favour of BlockTransactions (which does have a generic) from rpc-types.
Also if we're removing BlockResponse we can also remove TransactionList as it is the only place it is used
There was a problem hiding this comment.
BlockTransactions is not currently generic, and should be for reuse in other networks
There was a problem hiding this comment.
I'll leave TransactionsList as is but would like to change the Hydrated enum value to Full to be consistent with BlockTransactions
There was a problem hiding this comment.
i feel like we're having a miscommunication here so i'm gonna step back
- the 2 types in alloy-network lib.rs should be deleted as they're duplicative
- the types in alloy-rpc-types should have generics added with defaults
There was a problem hiding this comment.
Sounds great! That's ideal.
yash-atreya
changed the title
* dedup: TransactionList * dedup: BlockResponse in `network` * fix: ci * add: generic to BlockTransactions
Motivation
Ref #440 and #441
Solution
Removed
TransactionListfrom alloy_networkPR Checklist