Block refactor #368
Block refactor #368
Conversation
nomos-core/src/block.rs
Outdated
| #[derive(Clone, Debug, Serialize, Deserialize)] | ||
| pub struct ExecutionZoneSidecar<Blob: Clone + Eq + Hash> { | ||
| blobs: IndexSet<Blob>, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Serialize, Deserialize)] | ||
| pub struct CoordinationLayerSidecar<Tx: Clone + Eq + Hash> { | ||
| transactions: IndexSet<Tx>, | ||
| } | ||
|
|
||
| /// A block | ||
| #[derive(Clone, Debug, Serialize, Deserialize)] | ||
| pub struct Block<TxId: Clone + Eq + Hash> { | ||
| pub struct Block<Tx: Clone + Eq + Hash, Blob: Clone + Eq + Hash> { | ||
| header: consensus_engine::Block, | ||
| transactions: IndexSet<TxId>, | ||
| beacon: RandomBeaconState, | ||
| coordination_layer_sidecar: CoordinationLayerSidecar<Tx>, | ||
| execution_zone_sidecar: ExecutionZoneSidecar<Blob>, |
There was a problem hiding this comment.
This are the most important changes. The other are just generics spreading away.
There was a problem hiding this comment.
This is inaccurate as it is. The base layer needs to have no knowledge of the existence of EZs at all.
The fields should, for now, be:
- cl_transactions: this is a list of transactions. The cl prefix denotes that they are interpreted by the Coordination Layer, and so the BL only knows it's a list.
- bl_blobs: this is a list of pointers to blobs (ie hashes probably, probably more things that we will need to figure out). Blobs belong to the BL, but they are not interpreted by either the BL or the CL. Only the EZs know about this.
Ideally a clean design does not have any coupling with the EZs here at all. The BL should not know of the existence of EZs.
There was a problem hiding this comment.
Makes sense, it is more naming/concept than an issue. But I had a mix in my head with everything.
There was a problem hiding this comment.
Yes, it's true. It's still the same. But I think it's important to keep the references to EZs out of the BL. The CL will be inevitably coupled with the BL since it provides global computation/verification.
Also to clarify, both transactions and blobs must be ordered. Especially in the case of blobs where it might be tempting to conceptualize them as a set, but in reality they must be ordered for the case in which two blobs of the same EZ are included in the same block.
There was a problem hiding this comment.
Yes, We use IndexSet which is an ordered set.
for the case in which two blobs of the same EZ are included in the same block
Why would the same blob be included twice?
There was a problem hiding this comment.
The CL will be inevitably coupled with the BL since it provides global computation/verification.
curious about this, why would we need a two-way coupling instead of requiring the CL to interact with the BL but not the other way around?
One thing is that if we want to price CL transactions differently we need a way to signal that to the BL
danielSanchezQ
force-pushed
the
block-blobs
branch
from
17f23bf
to
9af8609
Compare
| self.transactions.iter() | ||
| } | ||
|
|
||
| pub fn blobs(&self) -> impl Iterator<Item = &Blob> + '_ { |
There was a problem hiding this comment.
isn't a blob just a type of transaction? or do we mean cl transactions and data blobs?
There was a problem hiding this comment.
yes, transactions are cltransactions. Blobs just data blobs. I didn't wanted to include the distinction in the naming. But it may be worth it. cl_transactions and bl_blobs as Alvaro suggested?
There was a problem hiding this comment.
cl_transactions and bl_blobs as Alvaro suggested?
This might be clearer, at least until we have something visible for CL and EZ in the future.
Codecov ReportPatch has no changes to coverable lines. ❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. 📢 Thoughts on this report? Let us know!. |
Yeah. But I think they are necessary evil. I'm working on the da management. Fitting the types together without actually knowing them is useful. But ugly indeed 😂 |
Added
blobsto block