From a69646e9e524dfa10c3972bd941ea24a798331b3 Mon Sep 17 00:00:00 2001 From: Ash Manning Date: Tue, 22 Oct 2024 17:09:16 +0800 Subject: [PATCH] Fixes for PR 52 --- src/messages.rs | 68 ++++++++------- src/proto.rs | 33 +++++++ src/server.rs | 17 +--- src/types.rs | 198 +++++++++++++++++++----------------------- src/validator/task.rs | 2 +- 5 files changed, 165 insertions(+), 153 deletions(-) diff --git a/src/messages.rs b/src/messages.rs index 26685c0..b67a663 100644 --- a/src/messages.rs +++ b/src/messages.rs @@ -1,20 +1,20 @@ -use bitcoin::script::{Instruction, Instructions}; -use bitcoin::{hashes::Hash, Amount, Opcode, Script, ScriptBuf, Transaction, TxOut}; use bitcoin::{ + hashes::Hash, opcodes::{ all::{OP_NOP5, OP_PUSHBYTES_1, OP_RETURN}, OP_TRUE, }, - script::PushBytesBuf, + script::{Instruction, Instructions, PushBytesBuf}, + Amount, Opcode, Script, ScriptBuf, Transaction, TxOut, }; use byteorder::{ByteOrder, LittleEndian}; -use nom::branch::alt; -use nom::bytes::complete::{tag, take}; -use nom::combinator::fail; -use nom::combinator::rest; -use nom::multi::many0; -use nom::IResult; -use sha2::{Digest, Sha256}; +use nom::{ + branch::alt, + bytes::complete::{tag, take}, + combinator::{fail, rest}, + multi::many0, + IResult, +}; use crate::types::SidechainNumber; @@ -160,27 +160,36 @@ impl M4AckBundles { } pub fn parse_coinbase_script(script: &Script) -> IResult<&[u8], CoinbaseMessage> { - let mut instructions = script.instructions(); - - // Return a nom parsing failure. Would be nice to include a message - // about what went wrong? Is that possible? - fn instruction_failure(instructions: Instructions) -> nom::Err> { - nom::Err::Failure(nom::error::Error { - input: instructions.as_script().as_bytes(), + fn instruction_failure<'a>( + err_msg: Option<&'static str>, + instructions: Instructions<'a>, + ) -> nom::Err> { + use nom::error::ContextError as _; + let input = instructions.as_script().as_bytes(); + let err = nom::error::Error { + input, code: nom::error::ErrorKind::Fail, - }) - } - - if instructions.next() != Some(Ok(Instruction::Op(OP_RETURN))) { - return Err(instruction_failure(instructions)); + }; + let err = match err_msg { + Some(err_msg) => nom::error::Error::add_context(input, err_msg, err), + None => err, + }; + nom::Err::Failure(err) } - + let mut instructions = script.instructions(); + let Some(Ok(Instruction::Op(OP_RETURN))) = instructions.next() else { + return Err(instruction_failure( + Some("expected OP_RETURN instruction"), + instructions, + )); + }; let Some(Ok(Instruction::PushBytes(data))) = instructions.next() else { - return Err(instruction_failure(instructions)); + return Err(instruction_failure( + Some("expected PushBytes instruction"), + instructions, + )); }; - let input = data.as_bytes(); - let (input, message_tag) = alt(( tag(M1_PROPOSE_SIDECHAIN_TAG), tag(M2_ACK_SIDECHAIN_TAG), @@ -379,12 +388,7 @@ impl TryFrom for ScriptBuf { } pub fn sha256d(data: &[u8]) -> [u8; 32] { - let mut hasher = Sha256::new(); - hasher.update(data); - let data_hash: [u8; 32] = hasher.finalize_reset().into(); - hasher.update(data_hash); - let data_hash: [u8; 32] = hasher.finalize().into(); - data_hash + bitcoin::hashes::sha256d::Hash::hash(data).to_byte_array() } pub fn m6_to_id(m6: &Transaction, previous_treasury_utxo_total: u64) -> [u8; 32] { diff --git a/src/proto.rs b/src/proto.rs index b38477f..08c95de 100644 --- a/src/proto.rs +++ b/src/proto.rs @@ -127,6 +127,14 @@ pub mod mainchain { let hex = bitcoin::consensus::encode::serialize_hex(value); Self { hex: Some(hex) } } + + pub fn encode_hex(value: &T) -> Self + where + T: hex::ToHex, + { + let hex = value.encode_hex(); + Self { hex: Some(hex) } + } } impl ReverseHex { @@ -166,6 +174,31 @@ pub mod mainchain { } } + impl From for SidechainDeclaration { + fn from(declaration: SidechainDeclarationV0) -> Self { + Self { + version: Some(sidechain_declaration::Version::V0(declaration)), + } + } + } + + impl From for SidechainDeclarationV0 { + fn from(declaration: crate::types::SidechainDeclaration) -> Self { + Self { + title: Some(declaration.title), + description: Some(declaration.description), + hash_id_1: Some(ConsensusHex::encode(&declaration.hash_id_1)), + hash_id_2: Some(ConsensusHex::encode_hex(&declaration.hash_id_2)), + } + } + } + + impl From for SidechainDeclaration { + fn from(declaration: crate::types::SidechainDeclaration) -> Self { + SidechainDeclarationV0::from(declaration).into() + } + } + impl From for Network { fn from(network: bitcoin::Network) -> Self { match network { diff --git a/src/server.rs b/src/server.rs index fcabd39..d106ab6 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,8 +1,5 @@ use std::sync::Arc; -use crate::proto::mainchain::{ - sidechain_declaration, SidechainDeclaration, SidechainDeclarationV0, -}; use crate::proto::mainchain::{ wallet_service_server::WalletService, BroadcastWithdrawalBundleRequest, BroadcastWithdrawalBundleResponse, CreateBmmCriticalDataTransactionRequest, @@ -350,16 +347,10 @@ impl ValidatorService for Validator { .map(|(data_hash, proposal)| SidechainProposal { sidechain_number: u8::from(proposal.sidechain_number) as u32, data: Some(proposal.data.clone()), - declaration: proposal.try_deserialize().ok().map(|(_, deserialized)| { - SidechainDeclaration { - version: Some(sidechain_declaration::Version::V0(SidechainDeclarationV0 { - title: Some(deserialized.title), - description: Some(deserialized.description), - hash_id_1: Some(ConsensusHex::encode(&deserialized.hash_id_1)), - hash_id_2: Some(ConsensusHex::encode(&deserialized.hash_id_2.to_vec())), - })), - } - }), + declaration: (&proposal) + .try_into() + .ok() + .map(|(_, declaration)| declaration.into()), data_hash: Some(ConsensusHex::encode(&data_hash)), vote_count: proposal.vote_count as u32, proposal_height: proposal.proposal_height, diff --git a/src/types.rs b/src/types.rs index 33a81b6..f3b1425 100644 --- a/src/types.rs +++ b/src/types.rs @@ -2,8 +2,10 @@ use std::num::TryFromIntError; use bitcoin::{Amount, BlockHash, OutPoint, TxOut, Work}; use hashlink::LinkedHashMap; -use miette::Result; +use miette::Diagnostic; +use nom::Finish; use serde::{Deserialize, Serialize}; +use thiserror::Error; pub type Hash256 = [u8; 32]; @@ -59,137 +61,119 @@ pub struct SidechainProposal { pub proposal_height: u32, } -impl SidechainProposal { - /// Deserialize the sidechain proposal, returning "raw" nom errors. This also means we're +#[derive(Debug, Error, Diagnostic)] +pub enum ParseSidechainProposalError { + #[error("Invalid UTF-8 sequence in title")] + #[diagnostic(code(sidechain_proposal::invalid_utf8_title))] + InvalidUtf8Title(std::str::Utf8Error), + + #[error("Invalid UTF-8 sequence in description")] + #[diagnostic(code(sidechain_proposal::invalid_utf8_description))] + InvalidUtf8Description(std::str::Utf8Error), + + #[error("Failed to deserialize sidechain proposal")] + #[diagnostic(code(sidechain_proposal::failed_to_deserialize))] + FailedToDeserialize(nom::error::Error>), + + #[error("Unknown sidechain proposal version: {0}")] + #[diagnostic(code(sidechain_proposal::unknown_version))] + UnknownVersion(u8), +} + +/// We know how to deserialize M1 v1 messages from BIP300 +/// https://github.com/bitcoin/bips/blob/master/bip-0300.mediawiki#m1----propose-sidechain +/// +/// 1-byte nSidechain +/// 4-byte nVersion +/// 1-byte title length +/// x-byte title +/// x-byte description +/// 32-byte hashID1f +/// 20-byte hashID2 +/// +/// Description length is total_data_length - length_of_all_other_fields +#[derive(Debug)] +pub struct SidechainDeclaration { + pub title: String, + pub description: String, + pub hash_id_1: [u8; 32], + pub hash_id_2: [u8; 20], +} + +impl SidechainDeclaration { + /// Parse the sidechain proposal, returning "raw" nom errors. This also means we're /// not validating that the title and description are valid UTF-8. This is probably possible /// with nom, but the author doesn't know how. It's fine to do this in the outer function, /// anyways. - fn try_deserialize_unvalidated( - &self, - ) -> nom::IResult<&[u8], (u8, UnvalidatedDeserializedSidechainProposalV1)> { - use nom::bytes::complete::tag; - use nom::bytes::complete::take; - use nom::number::complete::be_u8; - - let input = self.data.as_slice(); - let (input, sidechain_number) = be_u8(input)?; + fn try_parse( + input: &[u8], + ) -> nom::IResult<&[u8], (SidechainNumber, Self), ParseSidechainProposalError> { + use nom::{ + bytes::complete::{tag, take}, + number::complete::be_u8, + }; + fn failed_to_deserialize( + err: nom::Err>, + ) -> nom::Err { + err.to_owned() + .map(ParseSidechainProposalError::FailedToDeserialize) + } + let (input, sidechain_number) = be_u8(input).map_err(failed_to_deserialize)?; const VERSION_0: u8 = 0; - let (input, _) = tag(&[VERSION_0])(input)?; + let (input, _) = + tag(&[VERSION_0])(input).map_err(|_: nom::Err>| { + nom::Err::Error(ParseSidechainProposalError::UnknownVersion(input[0])) + })?; - let (input, title_length) = be_u8(input)?; - let (input, title) = take(title_length)(input)?; + let (input, title_length) = be_u8(input).map_err(failed_to_deserialize)?; + let (input, title_bytes) = take(title_length)(input).map_err(failed_to_deserialize)?; + let title = std::str::from_utf8(title_bytes) + .map_err(|err| nom::Err::Error(ParseSidechainProposalError::InvalidUtf8Title(err)))?; const HASH_ID_1_LENGTH: usize = 32; const HASH_ID_2_LENGTH: usize = 20; let description_length = input.len() - HASH_ID_1_LENGTH - HASH_ID_2_LENGTH; - let (input, description) = take(description_length)(input)?; + let (input, description_bytes) = + take(description_length)(input).map_err(failed_to_deserialize)?; + let description = std::str::from_utf8(description_bytes).map_err(|err| { + nom::Err::Error(ParseSidechainProposalError::InvalidUtf8Description(err)) + })?; - let (input, hash_id_1) = take(HASH_ID_1_LENGTH)(input)?; - let (input, hash_id_2) = take(HASH_ID_2_LENGTH)(input)?; + let (input, hash_id_1) = take(HASH_ID_1_LENGTH)(input).map_err(failed_to_deserialize)?; + let (input, hash_id_2) = take(HASH_ID_2_LENGTH)(input).map_err(failed_to_deserialize)?; let hash_id_1: [u8; HASH_ID_1_LENGTH] = hash_id_1.try_into().unwrap(); let hash_id_2: [u8; HASH_ID_2_LENGTH] = hash_id_2.try_into().unwrap(); - let parsed = UnvalidatedDeserializedSidechainProposalV1 { - title: title.to_vec(), - description: description.to_vec(), + let parsed = Self { + title: title.to_owned(), + description: description.to_owned(), hash_id_1, hash_id_2, }; - Ok((input, (sidechain_number, parsed))) + Ok((input, (sidechain_number.into(), parsed))) } +} - pub fn try_deserialize(&self) -> Result<(SidechainNumber, DeserializedSidechainProposalV1)> { - let (input, (sidechain_number, unvalidated)) = - self.try_deserialize_unvalidated() - .map_err(|err| match err { - nom::Err::Error(nom::error::Error { - code: nom::error::ErrorKind::Tag, - input, - }) => SidechainProposalError::UnknownVersion(input[0]), - _ => SidechainProposalError::FailedToDeserialize(err.to_owned()), - })?; +impl TryFrom<&SidechainProposal> for (SidechainNumber, SidechainDeclaration) { + type Error = ParseSidechainProposalError; + fn try_from(sidechain_proposal: &SidechainProposal) -> Result { + let (input, (sidechain_number, sidechain_proposal)) = + SidechainDeclaration::try_parse(&sidechain_proposal.data).finish()?; // One might think "this would be a good place to check there's no trailing bytes". // That doesn't work! Our parsing logic consumes _all_ bytes, because the length of the // `description` field is defined as the total length of the entire input minus // the length of all the other, known-length fields. We therefore _always_ read // the entire input. - if !input.is_empty() { - panic!("somehow ended up with trailing bytes") - } - - let sidechain_number = SidechainNumber::from(sidechain_number); - - let title = String::from_utf8(unvalidated.title) - .map_err(SidechainProposalError::InvalidUtf8Title)?; - - let description = String::from_utf8(unvalidated.description) - .map_err(SidechainProposalError::InvalidUtf8Description)?; - - Ok(( - sidechain_number, - DeserializedSidechainProposalV1 { - title, - description, - hash_id_1: unvalidated.hash_id_1, - hash_id_2: unvalidated.hash_id_2, - }, - )) + assert!(input.is_empty(), "somehow ended up with trailing bytes"); + Ok((sidechain_number, sidechain_proposal)) } } -/// We know how to deserialize M1 v1 messages from BIP300 -/// https://github.com/bitcoin/bips/blob/master/bip-0300.mediawiki#m1----propose-sidechain -/// -/// 1-byte nSidechain -/// 4-byte nVersion -/// 1-byte title length -/// x-byte title -/// x-byte description -/// 32-byte hashID1 -/// 20-byte hashID2 -/// -/// Description length is total_data_length - length_of_all_other_fields -#[derive(Debug)] -pub struct DeserializedSidechainProposalV1 { - pub title: String, - pub description: String, - pub hash_id_1: [u8; 32], - pub hash_id_2: [u8; 20], -} - -struct UnvalidatedDeserializedSidechainProposalV1 { - pub title: Vec, - pub description: Vec, - pub hash_id_1: [u8; 32], - pub hash_id_2: [u8; 20], -} -use miette::Diagnostic; -use thiserror::Error; - -#[derive(Debug, Error, Diagnostic)] -pub enum SidechainProposalError { - #[error("Invalid UTF-8 sequence in title")] - #[diagnostic(code(sidechain_proposal::invalid_utf8_title))] - InvalidUtf8Title(std::string::FromUtf8Error), - - #[error("Invalid UTF-8 sequence in description")] - #[diagnostic(code(sidechain_proposal::invalid_utf8_description))] - InvalidUtf8Description(std::string::FromUtf8Error), - - #[error("Failed to deserialize sidechain proposal")] - #[diagnostic(code(sidechain_proposal::failed_to_deserialize))] - FailedToDeserialize(nom::Err>>), - - #[error("Unknown sidechain proposal version: {0}")] - #[diagnostic(code(sidechain_proposal::unknown_version))] - UnknownVersion(u8), -} - #[derive(Debug, Deserialize, Serialize)] pub struct PendingM6id { pub m6id: Hash256, @@ -265,8 +249,8 @@ pub enum Event { #[cfg(test)] mod tests { - use crate::types::SidechainNumber; - use crate::types::SidechainProposal; + use crate::types::{SidechainNumber, SidechainProposal}; + use miette::Diagnostic as _; fn proposal(data: Vec) -> SidechainProposal { SidechainProposal { @@ -294,7 +278,7 @@ mod tests { 1, // hash_id_2 ]); - let result = sidechain_proposal.try_deserialize(); + let result = (&sidechain_proposal).try_into(); assert!(result.is_ok()); let (sidechain_number, deserialized) = result.unwrap(); @@ -332,7 +316,7 @@ mod tests { .concat(), ); - let result = sidechain_proposal.try_deserialize(); + let result: Result<(_, _), _> = (&sidechain_proposal).try_into(); assert!(result.is_err()); assert_eq!( format!("{}", result.unwrap_err().code().unwrap()), @@ -357,7 +341,7 @@ mod tests { .concat(), ); - let result = sidechain_proposal.try_deserialize(); + let result: Result<(_, _), _> = (&sidechain_proposal).try_into(); assert!(result.is_err()); assert_eq!( format!("{}", result.unwrap_err().code().unwrap()), @@ -373,7 +357,7 @@ mod tests { 0, // trailing byte ]); - let result = sidechain_proposal.try_deserialize(); + let result: Result<(_, _), _> = (&sidechain_proposal).try_into(); assert!(result.is_err()); assert_eq!( format!("{}", result.unwrap_err().code().unwrap()), diff --git a/src/validator/task.rs b/src/validator/task.rs index 09722a7..afae60b 100644 --- a/src/validator/task.rs +++ b/src/validator/task.rs @@ -1063,7 +1063,7 @@ mod tests { }; let (sidechain_number, deserialized) = - proposal.try_deserialize().expect("Failed to deserialize"); + (&proposal).try_into().expect("Failed to deserialize"); assert_eq!(sidechain_number, SidechainNumber(13)); assert_eq!(deserialized.description, "description");