Skip to content

Commit

Permalink
Use tendermint::{Vote, Proposal} for SignMsg (#782)
Browse files Browse the repository at this point in the history 
Uses `tendermint-rs` domain types to assist in parsing and validating
that signable message types are well-formed.

This avoids duplicating validation logic within TMKMS.
  • Loading branch information
@tony-iqlusion
tony-iqlusion authored Oct 14, 2023
1 parent 720d1a8 commit a614d1a
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 144 deletions.
2 changes: 1 addition & 1 deletion src/commands/ledger.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl Runnable for InitCommand {
..Default::default()
};
println!("{vote:?}");
let sign_vote_req = SignableMsg::Vote(vote);
let sign_vote_req = SignableMsg::try_from(vote).unwrap();
let to_sign = sign_vote_req
.signable_bytes(config.validator[0].chain_id.clone())
.unwrap();
Expand Down
5 changes: 2 additions & 3 deletions src/rpc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ impl Request {
Self::SignProposal(proto::privval::SignProposalRequest {
proposal: Some(proposal),
chain_id,
}) => (SignableMsg::Proposal(proposal), chain_id),
}) => (SignableMsg::try_from(proposal)?, chain_id),
Self::SignVote(proto::privval::SignVoteRequest {
vote: Some(vote),
chain_id,
}) => (SignableMsg::Vote(vote), chain_id),
}) => (SignableMsg::try_from(vote)?, chain_id),
_ => fail!(
ErrorKind::InvalidMessageError,
"expected a signable message type: {:?}",
Expand All @@ -82,7 +82,6 @@ impl Request {
expected_chain_id
);

signable_msg.validate()?;
Ok(signable_msg)
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/session.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl Session {
/// doesn't exceed it
fn check_max_height(&mut self, signable_msg: &SignableMsg) -> Result<(), Error> {
if let Some(max_height) = self.config.max_height {
let height = signable_msg.height()?;
let height = signable_msg.height();

if height > max_height {
fail!(
Expand All @@ -175,8 +175,8 @@ impl Session {
chain: &Chain,
signable_msg: &SignableMsg,
) -> Result<Option<proto::privval::RemoteSignerError>, Error> {
let msg_type = signable_msg.msg_type()?;
let request_state = signable_msg.consensus_state()?;
let msg_type = signable_msg.msg_type();
let request_state = signable_msg.consensus_state();
let mut chain_state = chain.state.lock().unwrap();

match chain_state.update_consensus_state(request_state.clone()) {
Expand Down Expand Up @@ -232,8 +232,8 @@ impl Session {
signable_msg: &SignableMsg,
started_at: Instant,
) -> Result<(), Error> {
let msg_type = signable_msg.msg_type()?;
let request_state = signable_msg.consensus_state()?;
let msg_type = signable_msg.msg_type();
let request_state = signable_msg.consensus_state();

info!(
"[{}@{}] signed {:?}:{} at h/r/s {} ({} ms)",
Expand Down
199 changes: 67 additions & 132 deletions src/signing.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
//! Signed message support.

use crate::{
error::{Error, ErrorKind},
keyring::signature::Signature,
prelude::{ensure, fail, format_err},
rpc::Response,
};
use crate::{error::Error, keyring::signature::Signature, rpc::Response};
use bytes::{Bytes, BytesMut};
use prost::{EncodeError, Message as _};
use signature::Signer;
use tendermint::{block, chain, consensus};
use tendermint::{block, chain, consensus, vote, Proposal, Vote};
use tendermint_proto as proto;

/// Message codes.
Expand All @@ -27,76 +22,31 @@ const PRECOMMIT_CODE: SignedMsgCode = 0x02;
/// Code for precommits.
const PROPOSAL_CODE: SignedMsgCode = 0x20;

/// Size of a validator address.
const VALIDATOR_ADDR_SIZE: usize = 20;

/// Trait for signed messages.
#[derive(Debug)]
pub enum SignableMsg {
/// Proposals
Proposal(proto::types::Proposal),
Proposal(Proposal),

/// Votes
Vote(proto::types::Vote),
Vote(Vote),
}

impl SignableMsg {
/// Get the signed message type.
pub fn msg_type(&self) -> Result<SignedMsgType, Error> {
let res = match self {
Self::Proposal(_) => Some(SignedMsgType::Proposal),
Self::Vote(vote) => match vote.r#type {
PREVOTE_CODE => Some(SignedMsgType::Prevote),
PRECOMMIT_CODE => Some(SignedMsgType::Precommit),
_ => None,
},
};

Ok(res.ok_or_else(|| {
format_err!(ErrorKind::ProtocolError, "no message type for this request")
})?)
}

/// Get the block height.
pub fn height(&self) -> Result<block::Height, Error> {
pub fn msg_type(&self) -> SignedMsgType {
match self {
Self::Proposal(proposal) => Ok(proposal.height.try_into()?),
Self::Vote(vote) => Ok(vote.height.try_into()?),
Self::Proposal(_) => SignedMsgType::Proposal,
Self::Vote(vote) => vote.vote_type.into(),
}
}

/// Validate the message is well-formed.
pub fn validate(&self) -> Result<(), Error> {
// Ensure height is valid
self.height()?;

// Ensure consensus state is valid
self.consensus_state()?;

/// Get the block height.
pub fn height(&self) -> block::Height {
match self {
Self::Proposal(proposal) => {
ensure!(
proposal.pol_round >= -1,
ErrorKind::ProtocolError,
"negative pol_round"
);
}
Self::Vote(vote) => {
ensure!(
vote.validator_index >= 0,
ErrorKind::ProtocolError,
"negative validator index"
);

ensure!(
vote.validator_address.len() == VALIDATOR_ADDR_SIZE,
ErrorKind::ProtocolError,
"negative validator index"
);
}
Self::Proposal(proposal) => proposal.height,
Self::Vote(vote) => vote.height,
}

Ok(())
}

/// Sign the given message, returning a response with the signature appended.
Expand All @@ -112,52 +62,31 @@ impl SignableMsg {
/// Get the bytes representing a canonically encoded message over which a
/// signature is to be computed.
pub fn signable_bytes(&self, chain_id: chain::Id) -> Result<Bytes, EncodeError> {
fn canonicalize_block_id(
block_id: &proto::types::BlockId,
) -> Option<proto::types::CanonicalBlockId> {
if block_id.hash.is_empty() {
return None;
}

Some(proto::types::CanonicalBlockId {
hash: block_id.hash.clone(),
part_set_header: block_id.part_set_header.as_ref().map(|y| {
proto::types::CanonicalPartSetHeader {
total: y.total,
hash: y.hash.clone(),
}
}),
})
}

let mut bytes = BytesMut::new();

match self {
Self::Proposal(proposal) => {
let proposal = proposal.clone();
let block_id = proposal.block_id.as_ref().and_then(canonicalize_block_id);

let cp = proto::types::CanonicalProposal {
chain_id: chain_id.to_string(),
r#type: SignedMsgType::Proposal.into(),
height: proposal.height,
block_id,
pol_round: proposal.pol_round as i64,
round: proposal.round as i64,
height: proposal.height.into(),
block_id: proposal.block_id.map(Into::into),
pol_round: proposal
.pol_round
.map(|round| round.value().into())
.unwrap_or(-1),
round: proposal.round.value().into(),
timestamp: proposal.timestamp.map(Into::into),
};

cp.encode_length_delimited(&mut bytes)?;
}
Self::Vote(vote) => {
let vote = vote.clone();
let block_id = vote.block_id.as_ref().and_then(canonicalize_block_id);

let cv = proto::types::CanonicalVote {
r#type: vote.r#type,
height: vote.height,
round: vote.round as i64,
block_id,
r#type: vote.vote_type.into(),
height: vote.height.into(),
round: vote.round.value().into(),
block_id: vote.block_id.map(Into::into),
timestamp: vote.timestamp.map(Into::into),
chain_id: chain_id.to_string(),
};
Expand All @@ -171,7 +100,8 @@ impl SignableMsg {
/// Add a signature to this request, returning a response.
pub fn add_signature(self, sig: Signature) -> Result<Response, Error> {
match self {
Self::Proposal(mut proposal) => {
Self::Proposal(proposal) => {
let mut proposal = proto::types::Proposal::from(proposal);
proposal.signature = sig.to_vec();
Ok(Response::SignedProposal(
proto::privval::SignedProposalResponse {
Expand All @@ -180,7 +110,8 @@ impl SignableMsg {
},
))
}
Self::Vote(mut vote) => {
Self::Vote(vote) => {
let mut vote = proto::types::Vote::from(vote);
vote.signature = sig.to_vec();
Ok(Response::SignedVote(proto::privval::SignedVoteResponse {
vote: Some(vote),
Expand All @@ -205,38 +136,40 @@ impl SignableMsg {
}

/// Parse the consensus state from the request.
pub fn consensus_state(&self) -> Result<consensus::State, Error> {
let msg_type = self.msg_type()?;

let mut consensus_state = match self {
pub fn consensus_state(&self) -> consensus::State {
match self {
Self::Proposal(p) => consensus::State {
height: block::Height::try_from(p.height)?,
round: block::Round::from(p.round as u16),
step: 3,
block_id: p
.block_id
.clone()
.and_then(|block_id| block_id.try_into().ok()),
height: p.height,
round: p.round,
step: 0,
block_id: p.block_id,
},
Self::Vote(v) => consensus::State {
height: block::Height::try_from(v.height)?,
round: block::Round::from(v.round as u16),
step: 6,
block_id: v
.block_id
.clone()
.and_then(|block_id| block_id.try_into().ok()),
height: v.height,
round: v.round,
step: match v.vote_type {
vote::Type::Prevote => 1,
vote::Type::Precommit => 2,
},
block_id: v.block_id,
},
};
}
}
}

consensus_state.step = match msg_type {
SignedMsgType::Unknown => fail!(ErrorKind::InvalidMessageError, "unknown message type"),
SignedMsgType::Proposal => 0,
SignedMsgType::Prevote => 1,
SignedMsgType::Precommit => 2,
};
impl TryFrom<proto::types::Proposal> for SignableMsg {
type Error = tendermint::Error;

Ok(consensus_state)
fn try_from(proposal: proto::types::Proposal) -> Result<Self, Self::Error> {
Proposal::try_from(proposal).map(Self::Proposal)
}
}

impl TryFrom<proto::types::Vote> for SignableMsg {
type Error = tendermint::Error;

fn try_from(vote: proto::types::Vote) -> Result<Self, Self::Error> {
Vote::try_from(vote).map(Self::Vote)
}
}

Expand Down Expand Up @@ -300,18 +233,20 @@ impl From<proto::types::SignedMsgType> for SignedMsgType {
}
}

impl From<vote::Type> for SignedMsgType {
fn from(vote_type: vote::Type) -> SignedMsgType {
match vote_type {
vote::Type::Prevote => SignedMsgType::Prevote,
vote::Type::Precommit => SignedMsgType::Precommit,
}
}
}

impl TryFrom<SignedMsgCode> for SignedMsgType {
type Error = ();
type Error = Error;

fn try_from(code: SignedMsgCode) -> Result<Self, Self::Error> {
// TODO(tarcieri): use `TryFrom<i32>` impl for `proto::types::SignedMsgType`
match code {
UNKNOWN_CODE => Ok(Self::Unknown),
PREVOTE_CODE => Ok(Self::Prevote),
PRECOMMIT_CODE => Ok(Self::Precommit),
PROPOSAL_CODE => Ok(Self::Proposal),
_ => Err(()),
}
Ok(proto::types::SignedMsgType::try_from(code)?.into())
}
}

Expand Down Expand Up @@ -371,7 +306,7 @@ mod tests {

#[test]
fn sign_proposal() {
let signable_msg = SignableMsg::Proposal(example_proposal());
let signable_msg = SignableMsg::try_from(example_proposal()).unwrap();
let signable_bytes = signable_msg.signable_bytes(example_chain_id()).unwrap();
assert_eq!(
signable_bytes.as_ref(),
Expand All @@ -386,7 +321,7 @@ mod tests {

#[test]
fn sign_vote() {
let signable_msg = SignableMsg::Vote(example_vote());
let signable_msg = SignableMsg::try_from(example_vote()).unwrap();
let signable_bytes = signable_msg.signable_bytes(example_chain_id()).unwrap();
assert_eq!(
signable_bytes.as_ref(),
Expand Down
6 changes: 3 additions & 3 deletions tests/integration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ fn handle_and_sign_proposal(key_type: KeyType) {
signature: vec![],
};

let signable_msg = SignableMsg::Proposal(proposal.clone());
let signable_msg = SignableMsg::try_from(proposal.clone()).unwrap();

let request = proto::privval::SignProposalRequest {
proposal: Some(proposal),
Expand Down Expand Up @@ -434,7 +434,7 @@ fn handle_and_sign_vote(key_type: KeyType) {
extension_signature: vec![],
};

let signable_msg = SignableMsg::Vote(vote_msg.clone());
let signable_msg = SignableMsg::try_from(vote_msg.clone()).unwrap();

let vote = proto::privval::SignVoteRequest {
vote: Some(vote_msg),
Expand Down Expand Up @@ -521,7 +521,7 @@ fn exceed_max_height(key_type: KeyType) {
extension_signature: vec![],
};

let signable_msg = SignableMsg::Vote(vote_msg.clone());
let signable_msg = SignableMsg::try_from(vote_msg.clone()).unwrap();

let vote = proto::privval::SignVoteRequest {
vote: Some(vote_msg),
Expand Down

0 comments on commit a614d1a

Please sign in to comment.