Conversation
blob/src/p2p.rs
Outdated
| let blob_topic = | ||
| <<Block::Header as HeaderT>::Hashing as HashT>::hash("blob_topic".as_bytes()); | ||
| let eval_claims_topic = | ||
| <<Block::Header as HeaderT>::Hashing as HashT>::hash(EVAL_CLAIMS_TOPIC); |
There was a problem hiding this comment.
Might be good to have 'blob_topic' as a const too, so we have to disparity
There was a problem hiding this comment.
added a separate const for blob_topic in latest commit
| maybe_eval = eval_claims_cmd_receiver.recv().fuse() => { | ||
| match maybe_eval { | ||
| Ok(msg) => { | ||
| gossip_engine.gossip_message(eval_claims_topic, msg.encode(), false) | ||
| }, | ||
| _ => break, | ||
| } |
There was a problem hiding this comment.
Would it make sense to have the eval_claims_cmd_receiver in its own spawned task ?
There was a problem hiding this comment.
added as a spawned task in latest commit
ToufeeqP
left a comment
ToufeeqP
left a comment
There was a problem hiding this comment.
@vbhattaccmu Were you able to test light-nodes validating the eval_proof with this setup? We need to change the key definition on the publishing side, or else change the key itselfo
blob/src/rpc.rs
Outdated
| ) { | ||
| if let Some(eval_sender) = friends.externalities.eval_claims_cmd_sender() { | ||
| let eval_msg = avail_blob::types::EvalClaimsMessage { | ||
| block_hash: finalized_block_hash.into(), |
There was a problem hiding this comment.
We shouldn't be publishing the eval data during blob processing/gossip, instead it should be done either during block production or import, as the block_hash we refer here is of the block in which the blob is included, and that is available only after we have constructed the block/imported it. The finalised block_hash is for a different purpose, and it can be different for different nodes based on their network at the time of their processing.
There was a problem hiding this comment.
have updated eval data publishing after block import.
| #[derive(Debug, Clone, PartialEq, Eq, Encode, Decode, TypeInfo, Serialize, Deserialize)] | ||
| pub struct EvalClaimsMessage { | ||
| /// Block hash this eval data belongs to | ||
| pub block_hash: H256, |
There was a problem hiding this comment.
Maybe we can use the app_id instead of block_hash, which enables us to publish the eval data during blob processing & also lightnodes can filter whether to validate the eval_proof based on app_id.
There was a problem hiding this comment.
using app_id instead of block_hash in latest commit. Light nodes are caching eval messages keyed by blob_hash and can filter per app_id.
There was a problem hiding this comment.
I think you forgot to push it?
not yet fully. will need to test |
3 participants
Pull Request type
Please add the labels corresponding to the type of changes your PR introduces:
Description
Related Issues
Testing Performed
Checklist
cargo test.cargo fmt.cargo build --releaseandcargo build --release --features runtime-benchmarks.cargo clippy.