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

CanonicalVoteExtension signature support #837

Merged
merged 1 commit into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/commands/ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl Runnable for InitCommand {
println!("{vote:?}");
let sign_vote_req = SignableMsg::from(Vote::try_from(vote).unwrap());
let to_sign = sign_vote_req
.signable_bytes(config.validator[0].chain_id.clone())
.canonical_bytes(config.validator[0].chain_id.clone())
.unwrap();

let _sig = chain.keyring.sign(None, &to_sign).unwrap();
Expand Down
6 changes: 6 additions & 0 deletions src/keyring/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,9 @@ impl From<ed25519::Signature> for Signature {
Self::Ed25519(sig)
}
}

impl From<Signature> for tendermint::Signature {
fn from(sig: Signature) -> tendermint::Signature {
sig.to_vec().try_into().expect("signature should be valid")
}
}
38 changes: 31 additions & 7 deletions src/privval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ impl SignableMsg {

/// Get the bytes representing a canonically encoded message over which a
/// signature is computed over.
pub fn signable_bytes(&self, chain_id: chain::Id) -> Result<Bytes, EncodeError> {
pub fn canonical_bytes(&self, chain_id: chain::Id) -> Result<Bytes, EncodeError> {
let mut bytes = BytesMut::new();

match self {
Self::Proposal(proposal) => {
let cp = proto::types::CanonicalProposal {
let canonical = proto::types::CanonicalProposal {
chain_id: chain_id.to_string(),
r#type: SignedMsgType::Proposal.into(),
height: proposal.height.into(),
Expand All @@ -67,24 +67,48 @@ impl SignableMsg {
timestamp: proposal.timestamp.map(Into::into),
};

cp.encode_length_delimited(&mut bytes)?;
canonical.encode_length_delimited(&mut bytes)?;
}
Self::Vote(vote) => {
let cv = proto::types::CanonicalVote {
let canonical = proto::types::CanonicalVote {
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(),
};
cv.encode_length_delimited(&mut bytes)?;
canonical.encode_length_delimited(&mut bytes)?;
}
}

Ok(bytes.into())
}

/// Get the bytes representing a vote extension if applicable.
pub fn extension_bytes(&self, chain_id: chain::Id) -> Result<Option<Bytes>, EncodeError> {
match self {
Self::Proposal(_) => Ok(None),
Self::Vote(vote) => {
// Only sign extension if actually present
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if vote.extension is empty, but vote extensions are enabled, the signature must be present.

if vote.extension.is_empty() {
return Ok(None);
}

let canonical = proto::types::CanonicalVoteExtension {
extension: vote.extension.clone(),
height: vote.height.into(),
round: vote.round.value().into(),
chain_id: chain_id.to_string(),
};

let mut bytes = BytesMut::new();
canonical.encode_length_delimited(&mut bytes)?;
Ok(Some(bytes.into()))
}
}
}

/// Parse the consensus state from the request.
pub fn consensus_state(&self) -> consensus::State {
match self {
Expand Down Expand Up @@ -275,7 +299,7 @@ mod tests {
#[test]
fn serialize_canonical_proposal() {
let signable_msg = SignableMsg::try_from(example_proposal()).unwrap();
let signable_bytes = signable_msg.signable_bytes(example_chain_id()).unwrap();
let signable_bytes = signable_msg.canonical_bytes(example_chain_id()).unwrap();
assert_eq!(
signable_bytes.as_ref(),
&[
Expand All @@ -290,7 +314,7 @@ mod tests {
#[test]
fn serialize_canonical_vote() {
let signable_msg = SignableMsg::try_from(example_vote()).unwrap();
let signable_bytes = signable_msg.signable_bytes(example_chain_id()).unwrap();
let signable_bytes = signable_msg.canonical_bytes(example_chain_id()).unwrap();
assert_eq!(
signable_bytes.as_ref(),
&[
Expand Down
22 changes: 17 additions & 5 deletions src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl Session {
}

/// Perform a digital signature operation
fn sign(&mut self, signable_msg: SignableMsg) -> Result<Response, Error> {
fn sign(&mut self, mut signable_msg: SignableMsg) -> Result<Response, Error> {
self.check_max_height(&signable_msg)?;

let registry = chain::REGISTRY.get();
Expand All @@ -139,13 +139,25 @@ impl Session {
return Ok(Response::error(signable_msg, remote_err));
}

let to_sign = signable_msg.signable_bytes(self.config.chain_id.clone())?;
// TODO(tarcieri): support for non-default public keys
let public_key = None;
let chain_id = self.config.chain_id.clone();

let canonical_msg = signable_msg.canonical_bytes(chain_id.clone())?;
let started_at = Instant::now();
let signature = chain.keyring.sign(public_key, &canonical_msg)?;
self.log_signing_request(&signable_msg, started_at).unwrap();

// TODO(ismail): figure out which key to use here instead of taking the only key
let signature = chain.keyring.sign(None, &to_sign)?;
// Add extension signature if there are any extensions defined
if let Some(extension_msg) = signable_msg.extension_bytes(chain_id)? {
let extension_sig = chain.keyring.sign(public_key, &extension_msg)?;

match &mut signable_msg {
SignableMsg::Vote(vote) => vote.extension_signature = Some(extension_sig.into()),
other => fail!(InvalidMessageError, "expected a vote type: {:?}", other),
}
}

self.log_signing_request(&signable_msg, started_at).unwrap();
Response::sign(signable_msg, signature)
}

Expand Down
6 changes: 3 additions & 3 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ fn handle_and_sign_proposal(key_type: KeyType) {
};

let signable_bytes = signable_msg
.signable_bytes(chain_id.parse().unwrap())
.canonical_bytes(chain_id.parse().unwrap())
.unwrap();

let prop = response
Expand Down Expand Up @@ -449,7 +449,7 @@ fn handle_and_sign_vote(key_type: KeyType) {
};

let signable_bytes = signable_msg
.signable_bytes(chain_id.parse().unwrap())
.canonical_bytes(chain_id.parse().unwrap())
.unwrap();

let vote_msg: proto::types::Vote = request
Expand Down Expand Up @@ -536,7 +536,7 @@ fn exceed_max_height(key_type: KeyType) {
};

let signable_bytes = signable_msg
.signable_bytes(chain_id.parse().unwrap())
.canonical_bytes(chain_id.parse().unwrap())
.unwrap();

let vote_msg = response
Expand Down
Loading