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

Clean up decoding of known-len component of messages between bootstrap client/server #3881

Merged
merged 29 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
3519398
Use dedicated deserializer for client-binder msg receive
Ben-PH Apr 28, 2023
125c1f3
Use helper method to decode data in known-len component of msg frm cl…
Ben-PH Apr 28, 2023
e64dd15
In client-binder, use helper method instead of a dedicated deser-struct
Ben-PH Apr 28, 2023
7ae357b
Remove redundant TODO
Ben-PH Apr 28, 2023
ca3a5c6
Bootstrap/constify (#3883)
Ben-PH Apr 28, 2023
03321ee
Use timeout-updating read-loop. Use peek_exact_timeout helper
Ben-PH Apr 28, 2023
ad38081
Send message to client in single write_all
Ben-PH Apr 28, 2023
d7c46a0
Document new helper methods
Ben-PH Apr 28, 2023
89b5c60
Make the servers `send_timeout` more readable
Ben-PH Apr 28, 2023
4aff56a
Revert the use of peek to a read-exact
Ben-PH May 3, 2023
041662c
Use traits to implement read_exact without code duplication
Ben-PH May 3, 2023
4c7a5b5
Remove unused peek implementation
Ben-PH May 3, 2023
01fa710
Comment the new methods
Ben-PH May 3, 2023
d006490
Clear clippy lints
Ben-PH May 3, 2023
472ed63
Client sends message in a single write
Ben-PH May 3, 2023
9c6055c
Implement and use the traits for the server binder
Ben-PH May 3, 2023
5676e9b
Module rearrangement
Ben-PH May 3, 2023
17eafbd
Merge remote-tracking branch 'origin/testnet_22' into bootstrap/use-r…
Ben-PH May 3, 2023
93c43a3
`cargo fmt --all`
Ben-PH May 3, 2023
c92b93d
Self review
Ben-PH May 3, 2023
35a5c2d
Merge remote-tracking branch 'origin/testnet_22' into bootstrap/messa…
Ben-PH May 3, 2023
497e76e
Merge remote-tracking branch 'origin/bootstrap/use-read-loop' into bo…
Ben-PH May 3, 2023
9e5abd5
Delete file that somehow missed the deletion-memo
Ben-PH May 4, 2023
9f8bd55
Merge branch 'testnet_23' into bootstrap/message-deser
Ben-PH May 4, 2023
54aa34c
Clear clippy lints
Ben-PH May 4, 2023
9b867ed
Merge branch 'testnet_23' into bootstrap/message-deser
Ben-PH May 4, 2023
0102c2e
rename peek and friends to reflect the reversion of peek usage
Ben-PH May 4, 2023
3f7b7b4
Use read_exact_timeout (should have been included in use-read-loop)
Ben-PH May 4, 2023
ff7a558
Use an in-file constant for compile-time known values
Ben-PH May 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 29 additions & 13 deletions massa-bootstrap/src/client_binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ pub struct BootstrapClientBinder {
cfg: BootstrapClientConfig,
}

/// The known-length component of a message to be received.
struct ServerMessageLeader {
sig: Signature,
msg_len: u32,
}

impl BootstrapClientBinder {
/// Creates a new `WriteBinder`.
///
Expand Down Expand Up @@ -69,7 +75,6 @@ impl BootstrapClientBinder {
Ok(())
}

// TODO: use a proper (de)serializer: https://github.com/massalabs/massa/pull/3745#discussion_r1169733161
/// Reads the next message.
pub fn next_timeout(
&mut self,
Expand All @@ -85,18 +90,7 @@ impl BootstrapClientBinder {
// TODO: Backoff spin of some sort
}

// construct the signature from the peek
let sig_array = peek_buff.as_slice()[0..SIGNATURE_SIZE_BYTES]
.try_into()
.expect("logic error in array manipulations");
let sig = Signature::from_bytes(&sig_array)?;

// construct the message len from the peek
let msg_len = u32::from_be_bytes_min(
&peek_buff[SIGNATURE_SIZE_BYTES..],
self.cfg.max_bootstrap_message_size,
)?
.0;
let ServerMessageLeader { sig, msg_len } = self.decode_msg_leader(&peek_buff)?;

// Update this bindings "most recently received" message hash, retaining the replaced value
let message_deserializer = BootstrapServerMessageDeserializer::new((&self.cfg).into());
Expand Down Expand Up @@ -189,4 +183,26 @@ impl BootstrapClientBinder {
self.duplex.write_all(&msg_bytes)?;
Ok(())
}

/// We are using this instead of of our library deserializer as the process is relatively straight forward
/// and makes error-type management cleaner
fn decode_msg_leader(&self, peek_buff: &[u8]) -> Result<ServerMessageLeader, BootstrapError> {
// todo: make the byte-count for the message len known at compile time so an array can be used
if peek_buff.len() != SIGNATURE_SIZE_BYTES + self.size_field_len {
panic!("todo: make the peek-buff length a compile-time constant");
}

let sig_array = peek_buff[0..SIGNATURE_SIZE_BYTES]
.try_into()
.expect("logic error in array manipulations");
let sig = Signature::from_bytes(&sig_array)?;

// construct the message len from the peek
let msg_len = u32::from_be_bytes_min(
&peek_buff[SIGNATURE_SIZE_BYTES..],
self.cfg.max_bootstrap_message_size,
)?
.0;
Ok(ServerMessageLeader { sig, msg_len })
}
}
64 changes: 43 additions & 21 deletions massa-bootstrap/src/server_binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ use std::{
};
use tracing::error;

/// The known-length component of a message to be received.
struct ClientMessageLeader {
received_prev_hash: Option<Hash>,
msg_len: u32,
}

/// Bootstrap server binder
pub struct BootstrapServerBinder {
max_bootstrap_message_size: u32,
Expand Down Expand Up @@ -168,7 +174,6 @@ impl BootstrapServerBinder {
})
}

// TODO: use a proper (de)serializer: https://github.com/massalabs/massa/pull/3745#discussion_r1169733161
/// Writes the next message.
pub fn send_timeout(
&mut self,
Expand Down Expand Up @@ -229,27 +234,11 @@ impl BootstrapServerBinder {
while self.duplex.peek(&mut peek_buf)? < peek_len {
// TODO: backoff spin of some sort
}
// construct prev-hash from peek
let received_prev_hash = {
if self.prev_message.is_some() {
Some(Hash::from_bytes(
peek_buf[..HASH_SIZE_BYTES]
.try_into()
.expect("bad slice logic"),
))
} else {
None
}
};

// construct msg-len from peek
let msg_len = {
u32::from_be_bytes_min(
&peek_buf[HASH_SIZE_BYTES..],
self.max_bootstrap_message_size,
)?
.0
};
let ClientMessageLeader {
received_prev_hash,
msg_len,
} = self.decode_message_leader(&peek_buf)?;

// read message, and discard the peek
let mut msg_bytes = vec![0u8; peek_len + (msg_len as usize)];
Expand Down Expand Up @@ -287,4 +276,37 @@ impl BootstrapServerBinder {

Ok(msg)
}

/// We are using this instead of of our library deserializer as the process is relatively straight forward
/// and makes error-type management cleaner
fn decode_message_leader(
&self,
peek_buf: &[u8],
) -> Result<ClientMessageLeader, BootstrapError> {
// construct prev-hash from peek
let received_prev_hash = {
if self.prev_message.is_some() {
Some(Hash::from_bytes(
peek_buf[..HASH_SIZE_BYTES]
.try_into()
.expect("bad slice logic"),
))
} else {
None
}
};

// construct msg-len from peek
let msg_len = {
u32::from_be_bytes_min(
&peek_buf[HASH_SIZE_BYTES..],
self.max_bootstrap_message_size,
)?
.0
};
Ok(ClientMessageLeader {
received_prev_hash,
msg_len,
})
}
}