Skip to content

Commit

Permalink
[refactoring] Rename check signature to verify signature
Browse files Browse the repository at this point in the history
  • Loading branch information
kchalkias committed May 4, 2022
1 parent 1ee582c commit 371d3b1
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 49 deletions.
7 changes: 3 additions & 4 deletions sui_core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,7 @@ impl AuthorityState {
transaction: Transaction,
) -> Result<TransactionInfoResponse, SuiError> {
// Check the sender's signature.
transaction.check_signature()?;

transaction.verify_signature()?;
let transaction_digest = *transaction.digest();

let response = self.handle_transaction_impl(transaction).await;
Expand Down Expand Up @@ -263,7 +262,7 @@ impl AuthorityState {

// Check the certificate and retrieve the transfer data.
tracing::trace_span!("cert_check_signature")
.in_scope(|| confirmation_transaction.certificate.check(&self.committee))?;
.in_scope(|| confirmation_transaction.certificate.verify(&self.committee))?;

self.process_certificate(confirmation_transaction).await
}
Expand Down Expand Up @@ -426,7 +425,7 @@ impl AuthorityState {
}

// Check the certificate.
certificate.check(&self.committee)?;
certificate.verify(&self.committee)?;

// Persist the certificate since we are about to lock one or more shared object.
// We thus need to make sure someone (if not the client) can continue the protocol.
Expand Down
8 changes: 4 additions & 4 deletions sui_core/src/authority_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ impl Validator for AuthorityServer {
obligation
.verify_all()
.map_err(|e| tonic::Status::invalid_argument(e.to_string()))?;
//TODO This is really really bad, we should have different types for checked transactions
transaction.is_checked = true;
//TODO This is really really bad, we should have different types for signature-verified transactions
transaction.is_verified = true;

let tx_digest = transaction.digest();

Expand Down Expand Up @@ -200,8 +200,8 @@ impl Validator for AuthorityServer {
obligation
.verify_all()
.map_err(|e| tonic::Status::invalid_argument(e.to_string()))?;
//TODO This is really really bad, we should have different types for checked transactions
transaction.is_checked = true;
//TODO This is really really bad, we should have different types for signature verified transactions
transaction.is_verified = true;

let tx_digest = transaction.digest();
let span = tracing::debug_span!(
Expand Down
2 changes: 1 addition & 1 deletion sui_core/src/consensus_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl ConsensusAdapter {
certificate: &ConsensusTransaction,
) -> SuiResult<TransactionInfoResponse> {
// Check the Sui certificate (submitted by the user).
certificate.check(&self.committee)?;
certificate.verify(&self.committee)?;

// Serialize the certificate in a way that is understandable to consensus (i.e., using
// bincode) and it certificate to consensus.
Expand Down
2 changes: 1 addition & 1 deletion sui_core/src/gateway_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ where
&self,
transaction: Transaction,
) -> Result<(CertifiedTransaction, TransactionEffects), anyhow::Error> {
transaction.check_signature()?;
transaction.verify_signature()?;
self.check_gas(
transaction.gas_payment_object_ref().0,
transaction.data.gas_budget,
Expand Down
12 changes: 6 additions & 6 deletions sui_core/src/safe_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl<C> SafeClient<C> {
) -> SuiResult {
if let Some(signed_transaction) = &response.signed_transaction {
// Check the transaction signature
signed_transaction.check(&self.committee)?;
signed_transaction.verify(&self.committee)?;
// Check it has the right signer
fp_ensure!(
signed_transaction.auth_sign_info.authority == self.address,
Expand All @@ -62,7 +62,7 @@ impl<C> SafeClient<C> {

if let Some(certificate) = &response.certified_transaction {
// Check signatures and quorum
certificate.check(&self.committee)?;
certificate.verify(&self.committee)?;
// Check it's the right transaction
fp_ensure!(
certificate.digest() == &digest,
Expand All @@ -77,7 +77,7 @@ impl<C> SafeClient<C> {
signed_effects
.auth_signature
.signature
.check(&signed_effects.effects, self.address)?;
.verify(&signed_effects.effects, self.address)?;
// Checks it concerns the right tx
fp_ensure!(
signed_effects.effects.transaction_digest == digest,
Expand All @@ -104,7 +104,7 @@ impl<C> SafeClient<C> {
) -> SuiResult {
// If we get a certificate make sure it is a valid certificate
if let Some(certificate) = &response.parent_certificate {
certificate.check(&self.committee)?;
certificate.verify(&self.committee)?;
}

// Check the right object ID and version is returned
Expand Down Expand Up @@ -160,7 +160,7 @@ impl<C> SafeClient<C> {
};

if let Some(signed_transaction) = &object_and_lock.lock {
signed_transaction.check(&self.committee)?;
signed_transaction.verify(&self.committee)?;
// Check it has the right signer
fp_ensure!(
signed_transaction.auth_sign_info.authority == self.address,
Expand All @@ -186,7 +186,7 @@ impl<C> SafeClient<C> {
// check the signature of the batch
signed_batch
.signature
.check(&signed_batch.batch, signed_batch.authority)?;
.verify(&signed_batch.batch, signed_batch.authority)?;

// ensure transactions enclosed match requested range

Expand Down
4 changes: 2 additions & 2 deletions sui_types/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ impl Signature {

/// This performs signature verification on the passed-in signature, additionally checking
/// that the signature was performed with a PublicKey belonging to an expected author, indicated by its Sui Address
pub fn check<T>(&self, value: &T, author: SuiAddress) -> Result<(), SuiError>
pub fn verify<T>(&self, value: &T, author: SuiAddress) -> Result<(), SuiError>
where
T: Signable<Vec<u8>>,
{
Expand Down Expand Up @@ -402,7 +402,7 @@ impl AuthoritySignature {
}

/// Signature verification for a single signature
pub fn check<T>(&self, value: &T, author: PublicKeyBytes) -> Result<(), SuiError>
pub fn verify<T>(&self, value: &T, author: PublicKeyBytes) -> Result<(), SuiError>
where
T: Signable<Vec<u8>>,
{
Expand Down
32 changes: 16 additions & 16 deletions sui_types/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ pub struct TransactionEnvelope<S> {
transaction_digest: OnceCell<TransactionDigest>,
// Deserialization sets this to "false"
#[serde(skip)]
pub is_checked: bool,
pub is_verified: bool,

pub data: TransactionData,
/// tx_signature is signed by the transaction sender, applied on `data`.
Expand All @@ -391,12 +391,12 @@ pub struct TransactionEnvelope<S> {
}

impl<S> TransactionEnvelope<S> {
pub fn check_signature(&self) -> Result<(), SuiError> {
pub fn verify_signature(&self) -> Result<(), SuiError> {
// We use this flag to see if someone has checked this before
// and therefore we can skip the check. Note that the flag has
// to be set to true manually, and is not set by calling this
// "check" function.
if self.is_checked {
if self.is_verified {
return Ok(());
}

Expand Down Expand Up @@ -520,7 +520,7 @@ impl Transaction {
pub fn new(data: TransactionData, signature: Signature) -> Self {
Self {
transaction_digest: OnceCell::new(),
is_checked: false,
is_verified: false,
data,
tx_signature: signature,
auth_sign_info: EmptySignInfo {},
Expand Down Expand Up @@ -555,7 +555,7 @@ impl SignedTransaction {
let signature = AuthoritySignature::new(&transaction.data, secret);
Self {
transaction_digest: OnceCell::new(),
is_checked: transaction.is_checked,
is_verified: transaction.is_verified,
data: transaction.data,
tx_signature: transaction.tx_signature,
auth_sign_info: AuthoritySignInfo {
Expand All @@ -567,13 +567,13 @@ impl SignedTransaction {
}

/// Verify the signature and return the non-zero voting right of the authority.
pub fn check(&self, committee: &Committee) -> Result<usize, SuiError> {
self.check_signature()?;
pub fn verify(&self, committee: &Committee) -> Result<usize, SuiError> {
self.verify_signature()?;
let weight = committee.weight(&self.auth_sign_info.authority);
fp_ensure!(weight > 0, SuiError::UnknownSigner);
self.auth_sign_info
.signature
.check(&self.data, self.auth_sign_info.authority)?;
.verify(&self.data, self.auth_sign_info.authority)?;
Ok(weight)
}

Expand Down Expand Up @@ -1029,7 +1029,7 @@ pub struct SignatureAggregator<'a> {
impl<'a> SignatureAggregator<'a> {
/// Start aggregating signatures for the given value into a certificate.
pub fn try_new(transaction: Transaction, committee: &'a Committee) -> Result<Self, SuiError> {
transaction.check_signature()?;
transaction.verify_signature()?;
Ok(Self::new_unsafe(transaction, committee))
}

Expand All @@ -1051,7 +1051,7 @@ impl<'a> SignatureAggregator<'a> {
authority: AuthorityName,
signature: AuthoritySignature,
) -> Result<Option<CertifiedTransaction>, SuiError> {
signature.check(&self.partial.data, authority)?;
signature.verify(&self.partial.data, authority)?;
// Check that each authority only appears once.
fp_ensure!(
!self.used_authorities.contains(&authority),
Expand Down Expand Up @@ -1080,7 +1080,7 @@ impl CertifiedTransaction {
pub fn new(transaction: Transaction) -> CertifiedTransaction {
CertifiedTransaction {
transaction_digest: transaction.transaction_digest,
is_checked: false,
is_verified: false,
data: transaction.data,
tx_signature: transaction.tx_signature,
auth_sign_info: AuthorityQuorumSignInfo {
Expand All @@ -1097,7 +1097,7 @@ impl CertifiedTransaction {
) -> CertifiedTransaction {
CertifiedTransaction {
transaction_digest: transaction.transaction_digest,
is_checked: false,
is_verified: false,
data: transaction.data,
tx_signature: transaction.tx_signature,
auth_sign_info: AuthorityQuorumSignInfo { epoch, signatures },
Expand All @@ -1109,12 +1109,12 @@ impl CertifiedTransaction {
}

/// Verify the certificate.
pub fn check(&self, committee: &Committee) -> Result<(), SuiError> {
pub fn verify(&self, committee: &Committee) -> Result<(), SuiError> {
// We use this flag to see if someone has checked this before
// and therefore we can skip the check. Note that the flag has
// to be set to true manually, and is not set by calling this
// "check" function.
if self.is_checked {
if self.is_verified {
return Ok(());
}

Expand Down Expand Up @@ -1235,9 +1235,9 @@ pub enum ConsensusTransaction {
}

impl ConsensusTransaction {
pub fn check(&self, committee: &Committee) -> SuiResult<()> {
pub fn verify(&self, committee: &Committee) -> SuiResult<()> {
match self {
Self::UserTransaction(certificate) => certificate.check(committee),
Self::UserTransaction(certificate) => certificate.verify(committee),
}
}
}
2 changes: 1 addition & 1 deletion sui_types/src/signature_seed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl SignatureSeed {
/// let sui_address = seed
/// .new_deterministic_address(&id, Some(&domain))
/// .unwrap();
/// let verification = signature.check(&msg, sui_address);
/// let verification = signature.verify(&msg, sui_address);
/// assert!(verification.is_ok());
/// # }
/// ```
Expand Down
8 changes: 4 additions & 4 deletions sui_types/src/unit_tests/base_types_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ fn test_signatures() {
let bar = Bar("hello".into());

let s = Signature::new(&foo, &sec1);
assert!(s.check(&foo, addr1).is_ok());
assert!(s.check(&foo, addr2).is_err());
assert!(s.check(&foox, addr1).is_err());
assert!(s.check(&bar, addr1).is_err());
assert!(s.verify(&foo, addr1).is_ok());
assert!(s.verify(&foo, addr2).is_err());
assert!(s.verify(&foox, addr1).is_err());
assert!(s.verify(&bar, addr1).is_err());
}

#[test]
Expand Down
12 changes: 6 additions & 6 deletions sui_types/src/unit_tests/messages_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,31 @@ fn test_signed_values() {
*sec1.public_key_bytes(),
&sec1,
);
assert!(v.check(&committee).is_ok());
assert!(v.verify(&committee).is_ok());

let v = SignedTransaction::new(
committee.epoch(),
transaction.clone(),
*sec2.public_key_bytes(),
&sec2,
);
assert!(v.check(&committee).is_err());
assert!(v.verify(&committee).is_err());

let v = SignedTransaction::new(
committee.epoch(),
transaction,
*sec3.public_key_bytes(),
&sec3,
);
assert!(v.check(&committee).is_err());
assert!(v.verify(&committee).is_err());

let v = SignedTransaction::new(
committee.epoch(),
bad_transaction,
*sec1.public_key_bytes(),
&sec1,
);
assert!(v.check(&committee).is_err());
assert!(v.verify(&committee).is_err());
}

#[test]
Expand Down Expand Up @@ -130,9 +130,9 @@ fn test_certificates() {
.append(v2.auth_sign_info.authority, v2.auth_sign_info.signature)
.unwrap()
.unwrap();
assert!(c.check(&committee).is_ok());
assert!(c.verify(&committee).is_ok());
c.auth_sign_info.signatures.pop();
assert!(c.check(&committee).is_err());
assert!(c.verify(&committee).is_err());

let mut builder = SignatureAggregator::try_new(transaction, &committee).unwrap();
assert!(builder
Expand Down
8 changes: 4 additions & 4 deletions sui_types/src/unit_tests/signature_seed_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,18 @@ fn test_deterministic_signing() {
assert!(sig_1.is_ok());

// Verify signatures.
let ver_0 = sig_0_ok.clone().check(&msg0, sui_address_0);
let ver_0 = sig_0_ok.clone().verify(&msg0, sui_address_0);
assert!(ver_0.is_ok());

let ver_1 = sig_1.unwrap().check(&msg0, sui_address_1);
let ver_1 = sig_1.unwrap().verify(&msg0, sui_address_1);
assert!(ver_1.is_ok());

// Ensure that signatures cannot be verified against another address.
let ver_0_with_address_1 = sig_0_ok.clone().check(&msg0, sui_address_1);
let ver_0_with_address_1 = sig_0_ok.clone().verify(&msg0, sui_address_1);
assert!(ver_0_with_address_1.is_err());

// Ensure that signatures cannot be verified against another message.
let ver_0_with_msg1 = sig_0_ok.clone().check(&msg1, sui_address_0);
let ver_0_with_msg1 = sig_0_ok.clone().verify(&msg1, sui_address_0);
assert!(ver_0_with_msg1.is_err());

// As we use ed25519, ensure that signatures on the same message are deterministic.
Expand Down

0 comments on commit 371d3b1

Please sign in to comment.