Skip to content

Commit

Permalink
feat: verify (always) Amount matches Commitment
Browse files Browse the repository at this point in the history
High level changes:

1. The check that secret amount matches commitment is now moved inside
Dbc::verify() so there is no chance of a wallet forgetting to do it.

2. We standardize on verify/verification instead of validate/validation.
This matches usage in ringct and blsttc, so it makes the api more
regular.

Details:

* change validate command to verify in mint-repl
* impl From<AmountSecrets> instead of Into<RevealedCommitment>
* impl From<AmountSecrets> instead of Into<Amount>
* replace validate/validation with verify/verification in code/comments
* rename validation.rs to verification.rs
* rename TransactionValidator to TransactionVerifier
* rename Dbc::confirm_valid() to verify()
* verify that secret Amount matches public Commitment inside Dbc::verify()
* update tests appropriately
  • Loading branch information
dan-da committed Mar 3, 2022
1 parent 4d81514 commit d38c5de
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 112 deletions.
16 changes: 8 additions & 8 deletions examples/mint-repl/mint-repl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,14 @@ fn main() -> Result<()> {
// "reissue_prepared" => reissue_prepared_cli(&mut mintinfo),
"reissue" => reissue_cli(&mut mintinfo),
"reissue_auto" => reissue_auto_cli(&mut mintinfo),
"validate" => validate(&mintinfo),
"verify" => verify(&mintinfo),
"newkey" => newkey(),
"newkeys" => newkeys(),
"decode" => decode_input(),
"quit" | "exit" => break,
"help" => {
println!(
"\nCommands:\n Mint: [mintinfo, newmint]\n Client: [newkey, newkeys, reissue, reissue_auto, decode, validate]\n General: [exit, help]\n"
"\nCommands:\n Mint: [mintinfo, newmint]\n Client: [newkey, newkeys, reissue, reissue_auto, decode, verify]\n General: [exit, help]\n"
);
Ok(())
}
Expand Down Expand Up @@ -541,12 +541,12 @@ __)(_| |(/_ | \|(/_|_\/\/(_)| |<
);
}

/// Implements validate command. Validates signatures and that a
/// Implements verify command. Validates signatures and that a
/// DBC has not been double-spent. Also checks if spent/unspent.
fn validate(mintinfo: &MintInfo) -> Result<()> {
fn verify(mintinfo: &MintInfo) -> Result<()> {
let dbc_input = readline_prompt_nl("\nInput DBC, or '[c]ancel': ")?;
let dbc: Dbc = if dbc_input == "c" {
println!("\nvalidate cancelled\n");
println!("\nVerify cancelled\n");
return Ok(());
} else {
from_be_hex(&dbc_input)?
Expand All @@ -557,7 +557,7 @@ fn validate(mintinfo: &MintInfo) -> Result<()> {
Owner::PublicKey(_pk) => {
let sk_input = readline_prompt_nl("\nSecret Key, or '[c]ancel': ")?;
let sk: SecretKey = if dbc_input == "c" {
println!("\nvalidate cancelled\n");
println!("\nVerify cancelled\n");
return Ok(());
} else {
from_be_hex(&sk_input)?
Expand All @@ -566,7 +566,7 @@ fn validate(mintinfo: &MintInfo) -> Result<()> {
}
};

match dbc.confirm_valid(&secret_key, mintinfo.mintnode()?.key_manager()) {
match dbc.verify(&secret_key, mintinfo.mintnode()?.key_manager()) {
Ok(_) => match mintinfo.spentbook()?.is_spent(&dbc.key_image(&secret_key)?) {
true => println!("\nThis DBC is unspendable. (valid but has already been spent)\n"),
false => println!("\nThis DBC is spendable. (valid and has not been spent)\n"),
Expand Down Expand Up @@ -618,7 +618,7 @@ fn prepare_tx(mintinfo: &MintInfo) -> Result<RingCtTransactionRevealed> {

// Get outputs from user
// note, we upcast to i128 to allow negative value.
// This permits unbalanced inputs/outputs to reach sn_dbc layer for validation.
// This permits unbalanced inputs/outputs to reach sn_dbc layer for verification.
let inputs_amount_sum = tx_builder.inputs_amount_sum();
while inputs_amount_sum as i128 - tx_builder.outputs_amount_sum() as i128 > 0 {
println!();
Expand Down
17 changes: 7 additions & 10 deletions src/amount_secrets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use blsttc::{
};
use rand_core::RngCore;
use std::collections::BTreeMap;
use std::convert::Into;
use std::convert::TryFrom;

use crate::{Amount, BlindingFactor, Error, SecretKeyBlst};
Expand Down Expand Up @@ -123,19 +122,17 @@ impl From<(Amount, BlindingFactor)> for AmountSecrets {
}
}

#[allow(clippy::from_over_into)]
impl Into<RevealedCommitment> for AmountSecrets {
/// convert AmountSecrets into the inner RevealedCommitment
fn into(self) -> RevealedCommitment {
self.0
impl From<AmountSecrets> for RevealedCommitment {
/// convert AmountSecrets into RevealedCommitment
fn from(a: AmountSecrets) -> RevealedCommitment {
a.0
}
}

#[allow(clippy::from_over_into)]
impl Into<Amount> for AmountSecrets {
impl From<AmountSecrets> for Amount {
/// convert AmountSecrets into an Amount
fn into(self) -> Amount {
self.0.value()
fn from(a: AmountSecrets) -> Amount {
a.0.value()
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ pub mod mock {
let reissue_request = rr_builder.build()?;

for mint_node in self.mint_nodes.into_iter() {
// note: for our (mock) purposes, all spentbook nodes are validated to
// note: for our (mock) purposes, all spentbook nodes are verified to
// have the same public key. (in the same section)
let spentbook_node_arbitrary = &self.spentbook_nodes[0];
let mint_node = mint_node.trust_spentbook_public_key(
Expand Down
67 changes: 22 additions & 45 deletions src/dbc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// KIND, either express or implied. Please review the Licences for the specific language governing
// permissions and limitations relating to use of the SAFE Network Software.

use crate::{AmountSecrets, KeyImage, Owner, SpentProof, TransactionValidator};
use crate::{AmountSecrets, KeyImage, Owner, SpentProof, TransactionVerifier};
use crate::{DbcContent, DerivationIndex, Error, KeyManager, Result};
use blst_ringct::ringct::{OutputProof, RingCtTransaction};
use blst_ringct::{RevealedCommitment, TrueInput};
Expand Down Expand Up @@ -185,15 +185,15 @@ impl Dbc {
}

// Check there exists a Transaction with the output containing this Dbc
// note: common validation logic is shared by MintNode::reissue() and Dbc::confirm_valid()
// note: common verification logic is shared by MintNode::reissue() and Dbc::verify()
//
// note: for spent_proofs to validate, the mint_verifier must have/know the spentbook section's public key.
pub fn confirm_valid<K: KeyManager>(
// note: for spent_proofs to verify, the mint_verifier must have/know the spentbook section's public key.
pub fn verify<K: KeyManager>(
&self,
base_sk: &SecretKey,
mint_verifier: &K,
) -> Result<(), Error> {
TransactionValidator::validate(
TransactionVerifier::verify(
mint_verifier,
&self.transaction,
&self.mint_sigs,
Expand All @@ -208,16 +208,16 @@ impl Dbc {
.iter()
.any(|o| *o.public_key() == owner)
{
Err(Error::DbcContentNotPresentInTransactionOutput)
} else {
Ok(())
return Err(Error::DbcContentNotPresentInTransactionOutput);
}

self.verify_amount_matches_commitment(base_sk)
}

/// bearer version of confirm_valid()
/// bearer version of verify()
/// will return an error if the SecretKey is not available. (not bearer)
pub fn confirm_valid_bearer<K: KeyManager>(&self, mint_verifier: &K) -> Result<(), Error> {
self.confirm_valid(&self.owner_base().secret_key()?, mint_verifier)
pub fn verify_bearer<K: KeyManager>(&self, mint_verifier: &K) -> Result<(), Error> {
self.verify(&self.owner_base().secret_key()?, mint_verifier)
}

/// Checks if the provided AmountSecrets matches the amount commitment.
Expand All @@ -241,34 +241,17 @@ impl Dbc {
/// If the merchant were to send the goods without first performing
/// this check, then they could be stuck with an unspendable Dbc
/// and no recourse.
pub fn confirm_provided_amount_matches_commitment(
&self,
base_sk: &SecretKey,
amount: &AmountSecrets,
) -> Result<()> {
pub(crate) fn verify_amount_matches_commitment(&self, base_sk: &SecretKey) -> Result<()> {
let rc: RevealedCommitment = self.amount_secrets(base_sk)?.into();
let secrets_commitment = rc.commit(&Default::default()).to_affine();
let tx_commitment = self.my_output_proof(base_sk)?.commitment();
let secrets_commitment = RevealedCommitment {
value: amount.amount(),
blinding: amount.blinding_factor(),
}
.commit(&Default::default())
.to_affine();

match secrets_commitment == tx_commitment {
true => Ok(()),
false => Err(Error::AmountCommitmentsDoNotMatch),
}
}

/// bearer version of confirm_provided_amount_matches_commitment()
/// will return an error if the SecretKey is not available. (not bearer)
pub fn confirm_provided_amount_matches_commitment_bearer(
&self,
amount: &AmountSecrets,
) -> Result<()> {
self.confirm_provided_amount_matches_commitment(&self.owner_base().secret_key()?, amount)
}

fn my_output_proof(&self, base_sk: &SecretKey) -> Result<&OutputProof> {
let owner = self.owner_once(base_sk)?.public_key_blst();
self.transaction
Expand Down Expand Up @@ -347,7 +330,7 @@ pub(crate) mod tests {
}

#[test]
fn test_dbc_without_inputs_is_invalid() -> Result<(), Error> {
fn test_dbc_without_inputs_fails_verification() -> Result<(), Error> {
let mut rng8 = rand8::rngs::StdRng::from_seed([0u8; 32]);
let mut rng = rand::rngs::StdRng::from_seed([0u8; 32]);
let amount = 100;
Expand Down Expand Up @@ -386,7 +369,7 @@ pub(crate) mod tests {
let mint_key_manager = SimpleKeyManager::from(SimpleSigner::from(id));

assert!(matches!(
dbc.confirm_valid(&owner_once.owner_base().secret_key()?, &mint_key_manager),
dbc.verify(&owner_once.owner_base().secret_key()?, &mint_key_manager),
Err(Error::RingCt(
blst_ringct::Error::TransactionMustHaveAnInput
))
Expand All @@ -397,7 +380,7 @@ pub(crate) mod tests {

#[allow(clippy::too_many_arguments)]
#[quickcheck]
fn prop_dbc_validation(
fn prop_dbc_verification(
n_inputs: NonZeroTinyInt, // # of input DBC's
n_valid_sigs: TinyInt, // # of valid sigs
n_wrong_signer_sigs: TinyInt, // # of valid sigs from unrecognized authority
Expand Down Expand Up @@ -493,7 +476,7 @@ pub(crate) mod tests {
let fuzzed_content = DbcContent::from((
owner_once.owner_base.clone(),
owner_once.derivation_index,
fuzzed_amt_secrets.clone(),
fuzzed_amt_secrets,
));

let mut fuzzed_mint_sigs: BTreeMap<KeyImage, (PublicKey, Signature)> = BTreeMap::new();
Expand Down Expand Up @@ -572,21 +555,15 @@ pub(crate) mod tests {
};

let key_manager = mint_node.key_manager();
let validation_res = dbc.confirm_valid(&owner_once.owner_base().secret_key()?, key_manager);
let verification_res = dbc.verify(&owner_once.owner_base().secret_key()?, key_manager);

let commitments_match = matches!(verification_res, Err(Error::AmountCommitmentsDoNotMatch));

let dbc_owner = dbc
.owner_once(&owner_once.owner_base().secret_key()?)?
.public_key_blst();

// Check if commitment in AmountSecrets matches the commitment in tx OutputProof
let commitments_match = dbc
.confirm_provided_amount_matches_commitment(
&owner_once.owner_base().secret_key()?,
&fuzzed_amt_secrets,
)
.is_ok();

match validation_res {
match verification_res {
Ok(()) => {
assert!(dbc
.transaction
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ mod mint;
mod owner;
mod spent_proof;
mod spentbook;
mod validation;
mod verification;

pub use crate::{
amount_secrets::AmountSecrets,
Expand All @@ -42,7 +42,7 @@ pub use crate::{
owner::{DerivationIndex, Owner, OwnerOnce},
spent_proof::{SpentProof, SpentProofContent, SpentProofShare},
spentbook::SpentBookNodeMock,
validation::TransactionValidator,
verification::TransactionVerifier,
};

#[cfg(feature = "serde")]
Expand Down

0 comments on commit d38c5de

Please sign in to comment.