Skip to content
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

Remove separate validation array #1522

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Conversation

mkeeter
Copy link
Contributor

@mkeeter mkeeter commented Oct 28, 2024

Right now, we have two types with the same shape:

/// Results of validating a single block
#[derive(Copy, Clone, Eq, PartialEq, Debug)]
pub(crate) enum Validation {
    /// The block has no hash / context and is empty
    Empty,
    /// For an unencrypted block, the result is the hash
    Unencrypted(u64),
    /// For an encrypted block, the result is the tag + nonce
    Encrypted(crucible_protocol::EncryptionContext),
}
#[derive(Debug, PartialEq, Copy, Clone, Serialize, Deserialize)]
pub enum ReadBlockContext {
    Empty,
    Encrypted { ctx: EncryptionContext },
    Unencrypted { hash: u64 },
}

The DownstairsIO stores both of these types:

struct DownstairsIO {
    // other members elided
    data: Option<RawReadResponse>, // contains blocks: Vec<ReadBlockContext>
    pub hashes: Vec<Validation>,
}

By the time we write to data, the block contexts have already been validated, so storing them again in hashes adds unnecessary duplication (and they're guaranteed to be the same).

This PR removes the duplicate Validation hashes, using the RawReadResponse::blocks member instead.

There are small changes to GuestBlockRes::transfer_and_notify and Buffer::write_read_response, because we can't take the entire Option<RawReadResponse>; instead, we pass the &mut BytesMut separately, so it can be sent back to the host.

@mkeeter mkeeter force-pushed the mkeeter/consolidate-validation branch 2 times, most recently from fe4340a to ff1d036 Compare October 30, 2024 19:50
@mkeeter mkeeter force-pushed the mkeeter/consolidate-validation branch 4 times, most recently from ce825dd to 075cfb9 Compare November 5, 2024 20:54
@mkeeter mkeeter force-pushed the mkeeter/consolidate-validation branch from 075cfb9 to beb6b68 Compare November 5, 2024 21:38
@mkeeter mkeeter merged commit affcda4 into main Nov 5, 2024
16 checks passed
@mkeeter mkeeter deleted the mkeeter/consolidate-validation branch November 5, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants