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
Add pieces verification to farmers. #725
Changes from 4 commits
926372a
1ea616b
7743d25
673fb47
3d901b5
ef4f9cf
22011d8
79b59b2
8ec51dc
fb1687f
a34268a
7ead01d
25982e1
e4ae0ce
7d1cc50
c866ea7
790f371
8538361
7d54abb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
use crate::RpcClient; | ||
use subspace_archiving::archiver::is_piece_valid; | ||
use subspace_core_primitives::{ | ||
FlatPieces, PieceIndex, MERKLE_NUM_LEAVES, PIECE_SIZE, RECORD_SIZE, | ||
}; | ||
use thiserror::Error; | ||
|
||
/// Pieces verification errors. | ||
#[derive(Error, Debug)] | ||
pub enum PiecesVerificationError { | ||
/// Invalid pieces data provided | ||
#[error("Invalid pieces data provided")] | ||
InvalidRawData, | ||
/// Pieces verification failed. | ||
#[error("Pieces verification failed.")] | ||
InvalidPieces, | ||
/// RPC client failed. | ||
#[error("RPC client failed. jsonrpsee error: {0}")] | ||
RpcError(Box<dyn std::error::Error + Send + Sync>), | ||
/// RPC client returned empty records_root. | ||
#[error("RPC client returned empty records_root.")] | ||
NoRecordsRootFound, | ||
} | ||
|
||
//TODO: Optimize: parallel execution, batch execution. | ||
/// Verifies pieces against the blockchain. | ||
pub async fn verify_pieces_at_blockchain<RC: RpcClient>( | ||
verification_client: &RC, | ||
piece_indexes: &[PieceIndex], | ||
pieces: &FlatPieces, | ||
) -> Result<(), PiecesVerificationError> { | ||
if piece_indexes.len() != (pieces.len() / PIECE_SIZE) { | ||
shamil-gadelshin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return Err(PiecesVerificationError::InvalidRawData); | ||
} | ||
for (index, piece) in pieces.as_pieces().enumerate() { | ||
let piece_index = piece_indexes[index]; | ||
shamil-gadelshin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
let segment_index: u64 = piece_index / MERKLE_NUM_LEAVES as u64; | ||
let position: u64 = piece_index % MERKLE_NUM_LEAVES as u64; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Please, verify the new algorithm. |
||
|
||
let root = verification_client | ||
.records_root(segment_index) | ||
.await | ||
.map_err(|err| PiecesVerificationError::RpcError(err))? | ||
.ok_or(PiecesVerificationError::NoRecordsRootFound)?; | ||
|
||
if !is_piece_valid(piece, root, position as usize, RECORD_SIZE as usize) { | ||
return Err(PiecesVerificationError::InvalidPieces); | ||
} | ||
} | ||
|
||
Ok(()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should take a vector of segment indexes and return a vector of roots to improve efficiency, otherwise there might be too many requests necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sure. However, it seems vulnerable to a large indexes vector. It makes sense to introduce
MAX_SEGMENT_INDEXES_PER_REQUEST
const to limit both farmer and node sides.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly. Requester needs to limit number of roots requested or else response will not fit into max size configured in RPC server and response will fail. For public RPC endpoints we also may want to limit this, so yeah, makes sense to have certain limits on both sides.