Skip to content

Commit

Permalink
[aptos-crypto] Remove expect from signing message
Browse files Browse the repository at this point in the history
  • Loading branch information
gregnazario committed Sep 21, 2022
1 parent ea67037 commit 18bbf8f
Show file tree
Hide file tree
Showing 63 changed files with 361 additions and 220 deletions.
2 changes: 1 addition & 1 deletion api/src/tests/transaction_vector_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ fn sign_transaction(raw_txn: RawTransaction) -> serde_json::Value {
let private_key = Ed25519PrivateKey::generate_for_testing();
let public_key = Ed25519PublicKey::from(&private_key);

let signature = private_key.sign(&raw_txn);
let signature = private_key.sign(&raw_txn).unwrap();
let txn = SignedTransaction::new(raw_txn.clone(), public_key, signature);

let mut raw_txn_json_out = Vec::new();
Expand Down
12 changes: 9 additions & 3 deletions api/src/tests/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ async fn test_multi_ed25519_signed_transaction() {
.expiration_timestamp_secs(u64::MAX) // set timestamp to max to ensure static raw transaction
.build();

let signature = private_key.sign(&raw_txn);
let signature = private_key.sign(&raw_txn).unwrap();
let txn = SignedTransaction::new_multisig(raw_txn, public_key, signature.clone());

let body = bcs::to_bytes(&txn).unwrap();
Expand Down Expand Up @@ -530,14 +530,20 @@ async fn test_signing_message_with_payload(
signing_msg.to_string(),
format!(
"0x{}",
hex::encode(&txn.clone().into_raw_transaction().signing_message())
hex::encode(
&txn.clone()
.into_raw_transaction()
.signing_message()
.unwrap()
)
)
);

let sig = context
.root_account()
.private_key()
.sign_arbitrary_message(signing_msg.inner());
.sign_arbitrary_message(signing_msg.inner())
.unwrap();
let expected_sig = match txn.authenticator() {
TransactionAuthenticator::Ed25519 {
public_key: _,
Expand Down
23 changes: 18 additions & 5 deletions api/src/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,16 +1212,29 @@ impl TransactionsApi {
})?;

let raw_message = match request.secondary_signers {
Some(secondary_signer_addresses) => {
signing_message(&RawTransactionWithData::new_multi_agent(
Some(secondary_signer_addresses) => signing_message(
&RawTransactionWithData::new_multi_agent(
raw_txn,
secondary_signer_addresses
.into_iter()
.map(|v| v.into())
.collect(),
))
}
None => raw_txn.signing_message(),
),
)
.context("Invalid transaction to generate signing message")
.map_err(|err| {
BasicError::bad_request_with_code(err, AptosErrorCode::InvalidInput, &ledger_info)
})?,
None => raw_txn
.signing_message()
.context("Invalid transaction to generate signing message")
.map_err(|err| {
BasicError::bad_request_with_code(
err,
AptosErrorCode::InvalidInput,
&ledger_info,
)
})?,
};

BasicResponse::try_from_json((
Expand Down
3 changes: 2 additions & 1 deletion api/test-context/src/test_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,8 @@ impl TestContext {

let sig = account
.private_key()
.sign_arbitrary_message(signing_msg.inner());
.sign_arbitrary_message(signing_msg.inner())
.unwrap();

request["signature"] = json!({
"type": "ed25519_signature",
Expand Down
7 changes: 5 additions & 2 deletions aptos-move/e2e-move-tests/tests/rotate_auth_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,11 @@ pub fn assert_successful_payload_key_rotation<
// sign the rotation message by the current private key and the new private key
let signature_by_curr_privkey = current_account
.privkey
.sign_arbitrary_message(&rotation_msg.clone().unwrap());
let signature_by_new_privkey = new_private_key.sign_arbitrary_message(&rotation_msg.unwrap());
.sign_arbitrary_message(&rotation_msg.clone().unwrap())
.unwrap();
let signature_by_new_privkey = new_private_key
.sign_arbitrary_message(&rotation_msg.unwrap())
.unwrap();

assert_success!(harness.run_transaction_payload(
&current_account,
Expand Down
10 changes: 6 additions & 4 deletions consensus/consensus-types/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl Block {
quorum_cert: QuorumCert,
validator_signer: &ValidatorSigner,
failed_authors: Vec<(Round, Author)>,
) -> Self {
) -> anyhow::Result<Self> {
let block_data = BlockData::new_proposal(
payload,
validator_signer.author(),
Expand All @@ -215,9 +215,11 @@ impl Block {
pub fn new_proposal_from_block_data(
block_data: BlockData,
validator_signer: &ValidatorSigner,
) -> Self {
let signature = validator_signer.sign(&block_data);
Self::new_proposal_from_block_data_and_signature(block_data, signature)
) -> anyhow::Result<Self> {
let signature = validator_signer.sign(&block_data)?;
Ok(Self::new_proposal_from_block_data_and_signature(
block_data, signature,
))
}

pub fn new_proposal_from_block_data_and_signature(
Expand Down
32 changes: 21 additions & 11 deletions consensus/consensus-types/src/block_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ fn test_nil_block() {
nil_block_qc,
&signer,
Vec::new(),
);
)
.unwrap();
assert_eq!(nil_block_child.is_nil_block(), false);
assert_eq!(nil_block_child.round(), 2);
assert_eq!(nil_block_child.parent_id(), nil_block.id());
Expand All @@ -97,7 +98,8 @@ fn test_block_relation() {
quorum_cert,
&signer,
Vec::new(),
);
)
.unwrap();
assert_eq!(next_block.round(), 1);
assert_eq!(genesis_block.is_parent_of(&next_block), true);
assert_eq!(
Expand Down Expand Up @@ -127,9 +129,10 @@ fn test_same_qc_different_authors() {
genesis_qc.clone(),
signer,
Vec::new(),
);
)
.unwrap();

let signature = signer.sign(genesis_qc.ledger_info().ledger_info());
let signature = signer.sign(genesis_qc.ledger_info().ledger_info()).unwrap();
let mut ledger_info_altered = LedgerInfoWithPartialSignatures::new(
genesis_qc.ledger_info().ledger_info().clone(),
PartialSignatures::empty(),
Expand All @@ -149,7 +152,8 @@ fn test_same_qc_different_authors() {
genesis_qc_altered,
signer,
Vec::new(),
);
)
.unwrap();

let block_round_1_same = Block::new_proposal(
payload,
Expand All @@ -158,7 +162,8 @@ fn test_same_qc_different_authors() {
genesis_qc,
signer,
Vec::new(),
);
)
.unwrap();

assert_ne!(block_round_1.id(), block_round_1_altered.id());
assert_eq!(block_round_1.id(), block_round_1_same.id());
Expand Down Expand Up @@ -188,7 +193,8 @@ fn test_block_metadata_bitvec() {
genesis_qc,
&signers[0],
Vec::new(),
);
)
.unwrap();
let block_metadata_1 = block_1.new_block_metadata(&validators);
assert_eq!(signers[0].author(), block_metadata_1.proposer());
assert_eq!(
Expand All @@ -202,9 +208,11 @@ fn test_block_metadata_bitvec() {
votes_1
.iter()
.zip(
validators
.iter()
.zip(signers.iter().map(|signer| signer.sign(&ledger_info))),
validators.iter().zip(
signers
.iter()
.map(|signer| signer.sign(&ledger_info).unwrap()),
),
)
.for_each(|(&voted, (&address, signature))| {
if voted {
Expand All @@ -225,7 +233,8 @@ fn test_block_metadata_bitvec() {
qc_1,
&signers[1],
Vec::new(),
);
)
.unwrap();
let block_metadata_2 = block_2.new_block_metadata(&validators);
assert_eq!(signers[1].author(), block_metadata_2.proposer());
let raw_bytes: Vec<u8> = BitVec::from(votes_1).into();
Expand Down Expand Up @@ -258,6 +267,7 @@ fn test_failed_authors_well_formed() {
&signer,
failed_authors,
)
.unwrap()
};

assert!(create_block(1, vec![]).verify_well_formed().is_ok());
Expand Down
2 changes: 1 addition & 1 deletion consensus/consensus-types/src/block_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ prop_compose! {
parent_qc,
&signer,
Vec::new(),
)
).unwrap()
}
}

Expand Down
12 changes: 8 additions & 4 deletions consensus/consensus-types/src/experimental/commit_vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::common::{Author, Round};
use anyhow::Context;
use aptos_crypto::bls12381;
use aptos_crypto::{bls12381, CryptoMaterialError};
use aptos_types::{
block_info::BlockInfo, ledger_info::LedgerInfo, validator_signer::ValidatorSigner,
validator_verifier::ValidatorVerifier,
Expand Down Expand Up @@ -43,9 +43,13 @@ impl CommitVote {
author: Author,
ledger_info_placeholder: LedgerInfo,
validator_signer: &ValidatorSigner,
) -> Self {
let signature = validator_signer.sign(&ledger_info_placeholder);
Self::new_with_signature(author, ledger_info_placeholder, signature)
) -> Result<Self, CryptoMaterialError> {
let signature = validator_signer.sign(&ledger_info_placeholder)?;
Ok(Self::new_with_signature(
author,
ledger_info_placeholder,
signature,
))
}

/// Generates a new CommitProposal using a signature over the specified ledger_info
Expand Down
15 changes: 11 additions & 4 deletions consensus/consensus-types/src/timeout_2chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use crate::{common::Author, quorum_cert::QuorumCert};
use anyhow::ensure;
use aptos_crypto::bls12381;
use aptos_crypto::{bls12381, CryptoMaterialError};
use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher};
use aptos_types::account_address::AccountAddress;
use aptos_types::aggregate_signature::{AggregateSignature, PartialSignatures};
Expand Down Expand Up @@ -53,7 +53,10 @@ impl TwoChainTimeout {
&self.quorum_cert
}

pub fn sign(&self, signer: &ValidatorSigner) -> bls12381::Signature {
pub fn sign(
&self,
signer: &ValidatorSigner,
) -> Result<bls12381::Signature, CryptoMaterialError> {
signer.sign(&self.signing_format())
}

Expand Down Expand Up @@ -409,7 +412,7 @@ mod tests {
PartialSignatures::empty(),
);
for signer in &signers[0..num_of_signature] {
let signature = signer.sign(ledger_info.ledger_info());
let signature = signer.sign(ledger_info.ledger_info()).unwrap();
ledger_info.add_signature(signer.author(), signature);
}
QuorumCert::new(
Expand All @@ -428,7 +431,11 @@ mod tests {
let mut tc_with_partial_sig =
TwoChainTimeoutWithPartialSignatures::new(timeouts[0].clone());
for (timeout, signer) in timeouts.iter().zip(&signers) {
tc_with_partial_sig.add(signer.author(), timeout.clone(), timeout.sign(signer));
tc_with_partial_sig.add(
signer.author(),
timeout.clone(),
timeout.sign(signer).unwrap(),
);
}

let tc_with_sig = tc_with_partial_sig
Expand Down
2 changes: 1 addition & 1 deletion consensus/consensus-types/src/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl Vote {
validator_signer: &ValidatorSigner,
) -> Self {
ledger_info_placeholder.set_consensus_data_hash(vote_data.hash());
let signature = validator_signer.sign(&ledger_info_placeholder);
let signature = validator_signer.sign(&ledger_info_placeholder).unwrap();
Self::new_with_signature(vote_data, author, ledger_info_placeholder, signature)
}

Expand Down
2 changes: 1 addition & 1 deletion consensus/safety-rules/src/fuzzing_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ prop_compose! {
let signature = if include_signature {
let mut rng = StdRng::from_seed(TEST_SEED);
let private_key = bls12381::PrivateKey::generate(&mut rng);
let signature = private_key.sign(&block_data);
let signature = private_key.sign(&block_data).unwrap();
Some(signature)
} else {
None
Expand Down
4 changes: 3 additions & 1 deletion consensus/safety-rules/src/safety_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ impl SafetyRules {
message: &T,
) -> Result<bls12381::Signature, Error> {
let signer = self.signer()?;
Ok(signer.sign(message))
signer
.sign(message)
.map_err(|err| Error::SerializationError(err.to_string()))
}

pub(crate) fn signer(&self) -> Result<&ValidatorSigner, Error> {
Expand Down
5 changes: 3 additions & 2 deletions consensus/safety-rules/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ pub fn make_proposal_with_qc_and_proof(
qc,
validator_signer,
Vec::new(),
),
)
.unwrap(),
None,
false,
)
Expand Down Expand Up @@ -200,7 +201,7 @@ pub fn make_timeout_cert(
) -> TwoChainTimeoutCertificate {
let timeout = TwoChainTimeout::new(1, round, hqc.clone());
let mut tc_partial = TwoChainTimeoutWithPartialSignatures::new(timeout.clone());
let signature = timeout.sign(signer);
let signature = timeout.sign(signer).unwrap();
tc_partial.add(signer.author(), timeout, signature);
tc_partial
.aggregate_signatures(&generate_validator_verifier(&[signer.clone()]))
Expand Down
3 changes: 2 additions & 1 deletion consensus/src/block_storage/block_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ async fn test_illegal_timestamp() {
certificate_for_genesis(),
&signer,
Vec::new(),
);
)
.unwrap();
let result = block_store
.execute_and_insert_block(block_with_illegal_timestamp)
.await;
Expand Down
6 changes: 4 additions & 2 deletions consensus/src/experimental/tests/execution_phase_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ fn add_execution_phase_test_cases(
) {
let genesis_qc = certificate_for_genesis();
let (signers, _validators) = random_validator_verifier(1, None, false);
let block = Block::new_proposal(Payload::empty(), 1, 1, genesis_qc, &signers[0], Vec::new());
let block =
Block::new_proposal(Payload::empty(), 1, 1, genesis_qc, &signers[0], Vec::new()).unwrap();

// happy path
phase_tester.add_test_case(
Expand Down Expand Up @@ -62,7 +63,8 @@ fn add_execution_phase_test_cases(
&LedgerInfo::mock_genesis(None),
random_hash_value,
);
let bad_block = Block::new_proposal(Payload::empty(), 1, 1, bad_qc, &signers[0], Vec::new());
let bad_block =
Block::new_proposal(Payload::empty(), 1, 1, bad_qc, &signers[0], Vec::new()).unwrap();
phase_tester.add_test_case(
ExecutionRequest {
ordered_blocks: vec![ExecutedBlock::new(
Expand Down
2 changes: 1 addition & 1 deletion consensus/src/liveness/proposal_generator_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async fn test_proposal_generation_empty_tree() {
.generate_proposal(1, &mut proposer_election, empty_callback())
.await
.unwrap();
let proposal = Block::new_proposal_from_block_data(proposal_data, &signer);
let proposal = Block::new_proposal_from_block_data(proposal_data, &signer).unwrap();
assert_eq!(proposal.parent_id(), genesis.id());
assert_eq!(proposal.round(), 1);
assert_eq!(proposal.quorum_cert().certified_block().id(), genesis.id());
Expand Down
Loading

0 comments on commit 18bbf8f

Please sign in to comment.