Create message_validator crate

[diegomrsantos]

Issue Addressed

Extracts validation from #147.
Related to #161.

Proposed Changes

Create a crate for validation that doesn't depend on other crates.

Additional Info

I added in common for now, but not sure it's necessary.

[dknopik]

why a submodule?

[dknopik]

nit: no need to specify the full type path

[dknopik]

Why a separate error instead of just returning the ValidationFailure?

[diegomrsantos]

Do we need to return ValidationFailure? We return the local Result and Error is an error in the validation process. Maybe we don't need the Error, I don't know if it can fail yet.

[diegomrsantos]

Maybe it should be like this?

pub enum Result {
    Accept,
    Reject(ValidationFailure),
    Ignore,
}

impl Validator {
    pub(crate) fn validate(
        &mut self,
        _message: SignedSSVMessage,
    ) -> Result {

        Accept
    }
}

[dknopik]

Aaah, now I also understand why you specified the path for the std Result type.

Well, we will need to report the validation result to the network crate via a queue. So validate should not return anything, right?

Nesting a validation failure into the Result also feels incorrect: we have a specific validation failure, and this maps to a way we handle the message. And this will be what we report to network, right?

[diegomrsantos]

something like this?

pub fn start_validator_service(validator: Validator) -> (Sender<SignedSSVMessage>, Receiver<ValidationResult>) {
    // Channel for receiving messages to validate.
    let (msg_tx, msg_rx) = mpsc::channel::<SignedSSVMessage>();
    // Channel for sending back validation results.
    let (result_tx, result_rx) = mpsc::channel::<ValidationResult>();

    spawn(move || {
       
        for signed_msg in msg_rx {
           
            let validation_result = validator.validate(signed_msg);
            
            if result_tx.send(validation_result).is_err() {
                break;
            }
        }
    });

    (msg_tx, result_rx)
}

[diegomrsantos]

But I believe we'd need to adapt it to go through the processor as well

[dknopik]

I don't think we need a centralized validator task, as there are no central resources to manage. Just create a closure, and send it to a processor queue via send_blocking.

[diegomrsantos]

Nesting a validation failure into the Result also feels incorrect

We might need it, cause the result can also be Accept. There's also a difference between Reject and Ignore.

[dknopik]

Yes, we need it, but we do not need to embed the validation failure into it, because we create the validation result (=the way we treat the message) from the validation failure, right? In other words, the failure variant determines whether to Ignore or Reject.

[diegomrsantos]

send_blocking doesn't work:

61  |     pub fn send_blocking<F: FnOnce() + Send + 'static>(
    |                             ^^^^^^^^ required by this bound in `Sender::send_blocking`

How about (ignore the Result for now)?

pub fn start_validator_service(
    validator: Validator,
) -> (Sender<SignedSSVMessage>, Receiver<Result>) {
    let (msg_tx, mut msg_rx) = mpsc::channel::<SignedSSVMessage>(100);
    let (result_tx, result_rx) = mpsc::channel::<Result>(100);

    validator
        .processor
        .urgent_consensus
        .send_async(
            async move {
                while let Some(signed_msg) = msg_rx.recv().await {
                    let result = validate(signed_msg);
                    // If the send fails, we can assume the receiver is dropped.
                    if result_tx.send(result).await.is_err() {
                        break;
                    }
                }
            },
            "validator_service",
        )
        .unwrap();

    (msg_tx, result_rx)
}