From f710de88abea8c59e2d74c54b30d73955bd692e0 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 6 Apr 2023 21:30:51 +0200 Subject: [PATCH] feat(rs-drive-abci): vote extensions --- packages/rs-drive-abci/src/abci/error.rs | 28 +++- packages/rs-drive-abci/src/abci/handlers.rs | 77 +++++++++ packages/rs-drive-abci/src/abci/mod.rs | 3 + .../src/abci/signature_verifier.rs | 60 +++++++ packages/rs-drive-abci/src/abci/withdrawal.rs | 151 ++++++++++++++++++ packages/rs-drive-abci/src/block.rs | 7 +- .../src/execution/commit_validation.rs | 25 +-- .../rs-drive-abci/src/execution/engine.rs | 87 ++++++++-- 8 files changed, 398 insertions(+), 40 deletions(-) create mode 100644 packages/rs-drive-abci/src/abci/signature_verifier.rs create mode 100644 packages/rs-drive-abci/src/abci/withdrawal.rs diff --git a/packages/rs-drive-abci/src/abci/error.rs b/packages/rs-drive-abci/src/abci/error.rs index d85ac50589e..0f18f4a25e6 100644 --- a/packages/rs-drive-abci/src/abci/error.rs +++ b/packages/rs-drive-abci/src/abci/error.rs @@ -1,5 +1,5 @@ +use super::signature_verifier::SignatureError; use crate::validator_set::ValidatorSetError; -use dpp::bls_signatures::Error as BLSError; /// Error returned within ABCI server #[derive(Debug, thiserror::Error)] @@ -7,6 +7,19 @@ pub enum AbciError { /// Invalid system state #[error("invalid state: {0}")] InvalidState(String), + /// Request does not match currently processed block + #[error("request does not match current block: {0}")] + RequestForWrongBlockReceived(String), + /// Withdrawal transactions mismatch + #[error("vote extensions mismatch: got {got:?}, expected {expected:?}")] + #[allow(missing_docs)] + VoteExtensionMismatchReceived { got: String, expected: String }, + /// Vote extensions signature is invalid + #[error("one of vote extension signatures is invalid")] + VoteExtensionsSignatureInvalid, + /// Cannot load withdrawal transactions + #[error("cannot load withdrawal transactions: {0}")] + WithdrawalTransactionsDBLoadError(String), /// Wrong finalize block received #[error("finalize block received before processing from Tenderdash: {0}")] FinalizeBlockReceivedBeforeProcessing(String), @@ -26,9 +39,9 @@ pub enum AbciError { #[error("tenderdash: {0}")] Tenderdash(#[from] tenderdash_abci::Error), - /// Error occurred during bls signature verification - #[error("bls error set: {0}")] - BLSError(#[from] BLSError), + /// Error occurred during signature verification + #[error("signature error: {0}")] + SignatureError(#[from] SignatureError), /// Error occurred during validator set creation #[error("validator set: {0}")] @@ -38,3 +51,10 @@ pub enum AbciError { #[error("generic with code: {0}")] GenericWithCode(u32), } + +// used by `?` operator +impl From for String { + fn from(value: AbciError) -> Self { + value.to_string() + } +} diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index 25c3cb5e1b3..1381c208cef 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -35,6 +35,7 @@ use crate::abci::server::AbciApplication; use crate::rpc::core::CoreRPCLike; use drive::fee::credits::SignedCredits; +use tenderdash_abci::proto::abci::response_verify_vote_extension::VerifyStatus; use tenderdash_abci::proto::abci::tx_record::TxAction; use tenderdash_abci::proto::abci::{ self as proto, RequestExtendVote, ResponseException, ResponseExtendVote, @@ -49,6 +50,9 @@ use crate::error::execution::ExecutionError; use crate::error::Error; use crate::execution::engine::BlockExecutionOutcome; +use super::withdrawal::WithdrawalTxs; +use super::AbciError; + impl<'a, C> tenderdash_abci::Application for AbciApplication<'a, C> where C: CoreRPCLike, @@ -181,12 +185,59 @@ where let response = ResponseProcessProposal { app_hash: app_hash.to_vec(), tx_results, + status: proto::response_process_proposal::ProposalStatus::Accept.into(), ..Default::default() }; Ok(response) } + fn extend_vote( + &self, + request: proto::RequestExtendVote, + ) -> Result { + let transaction_guard = self.transaction.read().unwrap(); + let transaction = transaction_guard.as_ref().unwrap(); + + self.must_match(request.height, request.round, request.hash)?; + + let withdrawals = WithdrawalTxs::load(Some(transaction), &self.platform.drive)?; + + Ok(proto::ResponseExtendVote { + vote_extensions: withdrawals.to_vec(), + }) + } + + fn verify_vote_extension( + &self, + request: proto::RequestVerifyVoteExtension, + ) -> Result { + let transaction_guard = self.transaction.read().unwrap(); + let transaction = transaction_guard.as_ref().unwrap(); + + self.must_match(request.height, request.round, request.hash)?; + + let got: WithdrawalTxs = request.vote_extensions.into(); + let expected = WithdrawalTxs::load(Some(transaction), &self.platform.drive)?; + + if let Err(err) = self.platform.check_withdrawals(&got, &expected, None) { + tracing::error!( + method = "verify_vote_extension", + ?got, + ?expected, + ?err, + "vote extension mismatch" + ); + Ok(proto::ResponseVerifyVoteExtension { + status: VerifyStatus::Reject.into(), + }) + } else { + Ok(proto::ResponseVerifyVoteExtension { + status: VerifyStatus::Accept.into(), + }) + } + } + fn finalize_block( &self, request: RequestFinalizeBlock, @@ -198,6 +249,7 @@ where "trying to finalize block without a current transaction", ), ))?; + self.platform .finalize_block_proposal(request, transaction)?; @@ -798,3 +850,28 @@ where // } // } // } +impl<'a, C> AbciApplication<'a, C> { + /// Check if current state (round/height/hash) matches received message. + fn must_match(&self, height: i64, round: i32, hash: Vec) -> Result<(), Error> { + let guarded_block_execution_context = self.platform.block_execution_context.read().unwrap(); + let block_execution_context = + guarded_block_execution_context + .as_ref() + .ok_or(Error::Execution(ExecutionError::CorruptedCodeExecution( + "block execution context must be set in block begin handler", + )))?; + + let block_state_info = &block_execution_context.block_state_info; + + if !block_state_info.matches(height as u64, round as u32, hash)? { + Err(Error::from(AbciError::RequestForWrongBlockReceived( + format!( + "received request for height: {} rount: {}, expected height: {} round: {}", + height, round, block_state_info.height, block_state_info.round + ), + ))) + } else { + Ok(()) + } + } +} diff --git a/packages/rs-drive-abci/src/abci/mod.rs b/packages/rs-drive-abci/src/abci/mod.rs index 52afcd51692..7b6e8e98dcf 100644 --- a/packages/rs-drive-abci/src/abci/mod.rs +++ b/packages/rs-drive-abci/src/abci/mod.rs @@ -15,6 +15,9 @@ pub mod mimic; #[cfg(any(feature = "server", test))] mod server; +pub mod signature_verifier; +pub mod withdrawal; + pub use error::AbciError; #[cfg(feature = "server")] pub use server::start; diff --git a/packages/rs-drive-abci/src/abci/signature_verifier.rs b/packages/rs-drive-abci/src/abci/signature_verifier.rs new file mode 100644 index 00000000000..53759b2966a --- /dev/null +++ b/packages/rs-drive-abci/src/abci/signature_verifier.rs @@ -0,0 +1,60 @@ +//! Signature verification + +use dpp::bls_signatures::Serialize; +use tenderdash_abci::proto::types::VoteExtension; + +/// Errors occured during signature verification +#[derive(Debug, thiserror::Error)] +pub enum SignatureError { + /// Error occurred during bls signature verification + #[error("bls error set: {0}")] + BLSError(#[from] dpp::bls_signatures::Error), +} +/// SignatureVerifier can be used to verify a BLS signature. +pub trait SignatureVerifier { + /// Verify all signatures using provided public key. + /// + /// ## Return value + /// + /// * Ok(true) when all signatures are correct + /// * Ok(false) when at least one signature is invalid + /// * Err(e) on error + fn verify_signature( + &self, + quorum_public_key: dpp::bls_signatures::PublicKey, + ) -> Result; +} + +impl SignatureVerifier for Vec { + fn verify_signature( + &self, + quorum_public_key: dpp::bls_signatures::PublicKey, + ) -> Result { + for tx in self { + if !tx.verify_signature(quorum_public_key)? { + return Ok(false); + } + } + + Ok(true) + } +} + +impl SignatureVerifier for VoteExtension { + fn verify_signature( + &self, + _quorum_public_key: dpp::bls_signatures::PublicKey, + ) -> Result { + let signature = &self.signature; + + // We could have received a fake commit, so signature validation needs to be returned if error as a simple validation result + let _signature = match dpp::bls_signatures::Signature::from_bytes(signature.as_slice()) { + Ok(signature) => signature, + Err(e) => return Err(SignatureError::from(e)), + }; + // TODO: implement correct signature verification for VoteExtension. It uses CanonicalVoteExtension. + // For now, we just return `true` + // Ok(quorum_public_key.verify(signature, &self.extension)) + Ok(true) + } +} diff --git a/packages/rs-drive-abci/src/abci/withdrawal.rs b/packages/rs-drive-abci/src/abci/withdrawal.rs new file mode 100644 index 00000000000..6a7a8f7c49a --- /dev/null +++ b/packages/rs-drive-abci/src/abci/withdrawal.rs @@ -0,0 +1,151 @@ +//! Withdrawal transactions definitions and processing + +use dpp::bls_signatures::{self}; +use drive::{ + drive::{batch::DriveOperation, block_info::BlockInfo, Drive}, + fee::result::FeeResult, + query::TransactionArg, +}; +use std::fmt::Display; +use tenderdash_abci::proto::{ + abci::ExtendVoteExtension, + types::{VoteExtension, VoteExtensionType}, +}; + +use super::{ + signature_verifier::{SignatureError, SignatureVerifier}, + AbciError, +}; + +const MAX_WITHDRAWAL_TXS: u16 = 16; + +/// Collection of withdrawal transactions processed at some height/round +#[derive(Debug)] +pub struct WithdrawalTxs<'a> { + inner: Vec, + drive_operations: Vec>, +} + +impl<'a> WithdrawalTxs<'a> { + /// Load pending withdrawal transactions from database + pub fn load(transaction: TransactionArg, drive: &Drive) -> Result { + let mut drive_operations = Vec::::new(); + + let inner = drive + .dequeue_withdrawal_transactions(MAX_WITHDRAWAL_TXS, transaction, &mut drive_operations) + .map_err(|e| AbciError::WithdrawalTransactionsDBLoadError(e.to_string()))? + .into_iter() + .map(|(_k, v)| VoteExtension { + r#type: VoteExtensionType::ThresholdRecover.into(), + extension: v, + signature: Default::default(), + }) + .collect::>(); + + Ok(Self { + drive_operations, + inner, + }) + } + + /// Basic validation of withdrawals. + /// + /// TODO: validate signature, etc. + pub fn validate(&self) -> Result<(), AbciError> { + if self.drive_operations.len() != self.inner.len() { + return Err(AbciError::InvalidState(format!( + "num of drive operations {} must match num of withdrawal transactions {}", + self.drive_operations.len(), + self.inner.len(), + ))); + } + + Ok(()) + } + + /// Finalize operations related to this withdrawal, as part of FinalizeBlock logic. + /// + /// Deletes withdrawal transactions that were executed. + pub fn finalize( + &self, + transaction: TransactionArg, + drive: &Drive, + block_info: &BlockInfo, + ) -> Result { + self.validate()?; + // TODO: Do we need to do sth with withdrawal txs to actually execute them? + // FIXME: check if this is correct, esp. "apply" arg + drive + .apply_drive_operations(self.drive_operations.clone(), true, block_info, transaction) + .map_err(|e| AbciError::WithdrawalTransactionsDBLoadError(e.to_string())) + } +} + +impl<'a> WithdrawalTxs<'a> { + /// Convert withdrawal transactions to vector of ExtendVoteExtension + pub fn to_vec(self) -> Vec { + self.inner + .into_iter() + .map(|v| ExtendVoteExtension { + r#type: v.r#type, + extension: v.extension, + }) + .collect::>() + } +} + +impl<'a> Display for WithdrawalTxs<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_fmt(format_args!("txs:["))?; + for item in &self.inner { + f.write_fmt(format_args!( + "tx:{},sig:{}\n", + hex::encode(&item.extension), + hex::encode(&item.signature) + ))?; + } + f.write_str("]\n")?; + Ok(()) + } +} +impl<'a> From> for WithdrawalTxs<'a> { + fn from(value: Vec) -> Self { + WithdrawalTxs { + inner: value + .into_iter() + .map(|v| VoteExtension { + r#type: v.r#type, + extension: v.extension, + signature: Default::default(), + }) + .collect::>(), + drive_operations: Vec::::new(), + } + } +} + +impl<'a> From<&Vec> for WithdrawalTxs<'a> { + fn from(value: &Vec) -> Self { + WithdrawalTxs { + inner: value.clone(), + drive_operations: Vec::::new(), + } + } +} + +impl<'a> PartialEq for WithdrawalTxs<'a> { + /// Two sets of withdrawal transactions are equal if all their inner raw transactions are equal. + /// Note we don't compare `drive_operations`. + fn eq(&self, other: &Self) -> bool { + self.inner.eq(&other.inner) + } +} + +impl<'a> SignatureVerifier for WithdrawalTxs<'a> { + fn verify_signature( + &self, + quorum_public_key: bls_signatures::PublicKey, + ) -> Result { + self.inner.verify_signature(quorum_public_key) + } +} diff --git a/packages/rs-drive-abci/src/block.rs b/packages/rs-drive-abci/src/block.rs index bff3bcbf422..05e5b9bc05a 100644 --- a/packages/rs-drive-abci/src/block.rs +++ b/packages/rs-drive-abci/src/block.rs @@ -28,13 +28,12 @@ // use crate::abci::AbciError; -use drive::drive::block_info::BlockInfo; -use drive::fee::epoch::EpochIndex; -use drive::fee_pools::epochs::Epoch; - use crate::error::Error; use crate::execution::block_proposal::BlockProposal; use crate::execution::fee_pools::epoch::EpochInfo; +use drive::drive::block_info::BlockInfo; +use drive::fee::epoch::EpochIndex; +use drive::fee_pools::epochs::Epoch; /// Block info pub struct BlockStateInfo { diff --git a/packages/rs-drive-abci/src/execution/commit_validation.rs b/packages/rs-drive-abci/src/execution/commit_validation.rs index d4a6f3d3ad0..21c2ba9087e 100644 --- a/packages/rs-drive-abci/src/execution/commit_validation.rs +++ b/packages/rs-drive-abci/src/execution/commit_validation.rs @@ -1,13 +1,10 @@ use crate::abci::AbciError; -use crate::error::execution::ExecutionError; use crate::error::Error; use crate::platform::Platform; use crate::rpc::core::CoreRPCLike; -use dashcore_rpc::dashcore_rpc_json::QuorumHash; use dpp::bls_signatures; use dpp::bls_signatures::Serialize; use dpp::validation::SimpleValidationResult; -use drive::grovedb::Transaction; use tenderdash_abci::proto::abci::CommitInfo; use tenderdash_abci::proto::types::BlockId; @@ -31,28 +28,18 @@ where &self, commit: CommitInfo, block_id: BlockId, + quorum_public_key: bls_signatures::PublicKey, ) -> Result, Error> { let signature = commit.block_signature; // We could have received a fake commit, so signature validation needs to be returned if error as a simple validation result let signature = match bls_signatures::Signature::from_bytes(signature.as_slice()) { Ok(signature) => signature, - Err(e) => return Ok(SimpleValidationResult::new_with_error(e.into())), + Err(e) => { + return Ok(SimpleValidationResult::new_with_error( + AbciError::SignatureError(e.into()), + )) + } }; - let public_key = self - .core_rpc - .get_quorum_info( - self.config.quorum_type.clone(), - &QuorumHash { - 0: commit.quorum_hash, - }, - Some(false), - )? - .quorum_public_key; - let public_key = match bls_signatures::PublicKey::from_bytes(public_key.as_slice()) { - Ok(public_key) => public_key, - Err(e) => return Ok(SimpleValidationResult::new_with_error(e.into())), - }; - // todo: public_key.verify(signature, ) Ok(SimpleValidationResult::default()) diff --git a/packages/rs-drive-abci/src/execution/engine.rs b/packages/rs-drive-abci/src/execution/engine.rs index 951bfb98854..252bf8420d9 100644 --- a/packages/rs-drive-abci/src/execution/engine.rs +++ b/packages/rs-drive-abci/src/execution/engine.rs @@ -14,6 +14,8 @@ use drive::fee::result::FeeResult; use drive::grovedb::{Transaction, TransactionArg}; use tenderdash_abci::proto::abci::{ExecTxResult, RequestFinalizeBlock}; +use crate::abci::signature_verifier::{SignatureError, SignatureVerifier}; +use crate::abci::withdrawal::WithdrawalTxs; use crate::abci::AbciError; use crate::block::{BlockExecutionContext, BlockStateInfo}; use crate::error::execution::ExecutionError; @@ -323,6 +325,45 @@ where state_cache.last_committed_block_info = Some(block_info.clone()); } + /// check if received withdrawal transactions are correct and match our withdrawal txs + pub fn check_withdrawals( + &self, + received_withdrawals: &WithdrawalTxs, + our_withdrawals: &WithdrawalTxs, + quorum_public_key: Option, + ) -> Result<(), AbciError> { + if received_withdrawals.ne(&our_withdrawals) { + return Err(AbciError::VoteExtensionMismatchReceived { + got: received_withdrawals.to_string(), + expected: our_withdrawals.to_string(), + }); + } + + // Now, verify signature + if let Some(public_key) = quorum_public_key { + match received_withdrawals.verify_signature(public_key) { + Ok(true) => return Ok(()), + Ok(false) => return Err(AbciError::VoteExtensionsSignatureInvalid), + Err(e) => return Err(e.into()), + } + } + + Ok(()) + } + // Retreieve quorum public key + fn get_quorum_key(&self, quorum_hash: Vec) -> Result { + let public_key = self + .core_rpc + .get_quorum_info( + self.config.quorum_type.clone(), + &QuorumHash { 0: quorum_hash }, + Some(false), + )? + .quorum_public_key; + + bls_signatures::PublicKey::from_bytes(public_key.as_slice()) + .map_err(|e| AbciError::from(SignatureError::from(e)).into()) + } /// Finalize the block, this first involves validating it, then if valid /// it is committed to the state pub fn finalize_block_proposal( @@ -371,28 +412,45 @@ where } // Next we need to verify that the signature returned from the quorum is valid + let commit = if let Some(commit) = commit { + commit + } else { + validation_result.add_error(AbciError::WrongFinalizeBlockReceived(format!( + "received a block for h: {} r: {} without a commit", + height, round + ))); + return Ok(validation_result.into()); + }; - if let Some(commit) = commit { - if let Some(block_id) = block_id { - let result = self.validate_commit(commit, block_id)?; - if !result.is_valid() { - return Ok(validation_result.into()); - } - } else { - validation_result.add_error(AbciError::WrongFinalizeBlockReceived(format!( - "received a block for h: {} r: {} without a block id", - height, round - ))); + let quorum_public_key = self.get_quorum_key(commit.quorum_hash.clone())?; + + if let Some(block_id) = block_id { + let result = self.validate_commit(commit.clone(), block_id, quorum_public_key)?; + if !result.is_valid() { return Ok(validation_result.into()); } } else { validation_result.add_error(AbciError::WrongFinalizeBlockReceived(format!( - "received a block for h: {} r: {} without a commit", + "received a block for h: {} r: {} without a block id", height, round ))); return Ok(validation_result.into()); } + // Verify vote extensions + let received_withdrawals = WithdrawalTxs::from(&commit.threshold_vote_extensions); + let our_withdrawals = WithdrawalTxs::load(Some(transaction), &self.drive) + .map_err(|e| AbciError::WithdrawalTransactionsDBLoadError(e.to_string()))?; + + if let Err(e) = self.check_withdrawals( + &received_withdrawals, + &our_withdrawals, + Some(quorum_public_key), + ) { + validation_result.add_error(e); + return Ok(validation_result.into()); + } + // Next let's check that the hash received is the same as the hash we expect if height == self.config.abci.genesis_height { @@ -419,8 +477,11 @@ where None }; - // At the end we update the state cache let block_info = block_state_info.to_block_info(epoch_info.current_epoch_index); + // Finalize withdrawal processing + our_withdrawals.finalize(Some(transaction), &self.drive, &block_info)?; + + // At the end we update the state cache self.update_state_cache_and_quorums(block_info);