Skip to content

Commit

Permalink
fix(dan_layer/core): include state root in checkpoint signature (#4285)
Browse files Browse the repository at this point in the history
Description
---
- includes the "real" state root in the checkpoint signature, replacing the zero state root.
- makes fields in `SignerSignature` private
- adds `sign_checkpoint` method to `SigningService`

Motivation and Context
---
Generates the state root for the pre-committed state and includes that in the checkpoint signatures on the commit step
instead of a dummy value for the state root.

BUG: the state root MMR only includes state updates for existing keys and does not include _new_ keys. This was a pre-existing bug
TODO: Share a blinding factor seed between validators and derive checkpoint blinding factors. Currently, a zero commitment is included in the checkpoint signature and a random blinding factor generated by the VN wallet is used in the checkpoint output.

How Has This Been Tested?
---
Manually, signatures are included in the checkpoint
  • Loading branch information
sdbondi authored Jul 7, 2022
1 parent 414be33 commit bcaabf0
Show file tree
Hide file tree
Showing 22 changed files with 127 additions and 94 deletions.
12 changes: 6 additions & 6 deletions applications/tari_app_grpc/src/conversions/sidechain_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,8 @@ impl TryFrom<grpc::CommitteeSignatures> for CommitteeSignatures {
impl<B: Borrow<SignerSignature>> From<B> for grpc::SignerSignature {
fn from(value: B) -> Self {
Self {
signer: value.borrow().signer.to_vec(),
signature: Some(grpc::Signature::from(&value.borrow().signature)),
signer: value.borrow().signer().to_vec(),
signature: Some(grpc::Signature::from(value.borrow().signature())),
}
}
}
Expand All @@ -539,13 +539,13 @@ impl TryFrom<grpc::SignerSignature> for SignerSignature {
type Error = String;

fn try_from(value: grpc::SignerSignature) -> Result<Self, Self::Error> {
Ok(Self {
signer: PublicKey::from_bytes(&value.signer).map_err(|err| err.to_string())?,
signature: value
Ok(Self::new(
PublicKey::from_bytes(&value.signer).map_err(|err| err.to_string())?,
value
.signature
.map(TryInto::try_into)
.ok_or("signature not provided")??,
})
))
}
}
//---------------------------------- ContractAcceptance --------------------------------------------//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ impl WalletClient for GrpcWalletClient {
contract_id: &FixedHash,
state_root: &StateRoot,
checkpoint_number: u64,
checkpoint_signatures: Vec<SignerSignature>,
checkpoint_signatures: &[SignerSignature],
) -> Result<(), DigitalAssetError> {
let inner = self.connection().await?;
let committee_signatures = grpc::CommitteeSignatures {
signatures: checkpoint_signatures.into_iter().map(Into::into).collect(),
signatures: checkpoint_signatures.iter().map(Into::into).collect(),
};

if checkpoint_number == 0 {
Expand Down
8 changes: 4 additions & 4 deletions applications/tari_validator_node/src/p2p/proto/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,13 +387,13 @@ impl TryFrom<proto::common::SignerSignature> for SignerSignature {
type Error = String;

fn try_from(value: proto::common::SignerSignature) -> Result<Self, Self::Error> {
Ok(Self {
signer: PublicKey::from_bytes(&value.signer).map_err(|err| err.to_string())?,
signature: value
Ok(Self::new(
PublicKey::from_bytes(&value.signer).map_err(|err| err.to_string())?,
value
.signature
.map(TryInto::try_into)
.ok_or("signature not provided")??,
})
))
}
}

Expand Down
12 changes: 6 additions & 6 deletions base_layer/core/src/proto/types_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ impl<T: Borrow<Signature>> From<T> for proto::Signature {
impl<B: Borrow<SignerSignature>> From<B> for proto::SignerSignature {
fn from(value: B) -> Self {
Self {
signer: value.borrow().signer.to_vec(),
signature: Some(proto::Signature::from(&value.borrow().signature)),
signer: value.borrow().signer().to_vec(),
signature: Some(proto::Signature::from(value.borrow().signature())),
}
}
}
Expand All @@ -92,13 +92,13 @@ impl TryFrom<proto::SignerSignature> for SignerSignature {
type Error = String;

fn try_from(value: proto::SignerSignature) -> Result<Self, Self::Error> {
Ok(Self {
signer: PublicKey::from_bytes(&value.signer).map_err(|err| err.to_string())?,
signature: value
Ok(Self::new(
PublicKey::from_bytes(&value.signer).map_err(|err| err.to_string())?,
value
.signature
.map(TryInto::try_into)
.ok_or("signature not provided")??,
})
))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

use digest::Digest;
use tari_common_types::types::{Commitment, FixedHash, HashDigest};
use tari_utilities::ByteArray;
use tari_common_types::types::{Commitment, FixedHash};

use crate::consensus::ConsensusHashWriter;

#[derive(Debug, Clone, Copy)]
pub struct CheckpointChallenge(FixedHash);
Expand All @@ -31,15 +31,15 @@ impl CheckpointChallenge {
pub fn new(
contract_id: &FixedHash,
checkpoint_commitment: &Commitment,
merkle_root: FixedHash,
merkle_root: &FixedHash,
checkpoint_number: u64,
) -> Self {
// TODO: Use new tari_crypto domain-separated hashing
let hash = HashDigest::new()
.chain(contract_id.as_slice())
.chain(checkpoint_commitment.as_bytes())
.chain(merkle_root.as_slice())
.chain(&checkpoint_number.to_le_bytes())
let hash = ConsensusHashWriter::default()
.chain(contract_id)
.chain(checkpoint_commitment)
.chain(merkle_root)
.chain(&checkpoint_number)
.finalize()
.into();
Self(hash)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ pub use sidechain_features::{SideChainFeatures, SideChainFeaturesBuilder};
mod contract_checkpoint;
pub use contract_checkpoint::ContractCheckpoint;

mod checkpoint_challenge;
pub use checkpoint_challenge::CheckpointChallenge;

// Length of FixedString
pub const FIXED_STR_LEN: usize = 32;
pub type FixedString = [u8; FIXED_STR_LEN];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ use crate::consensus::{ConsensusDecoding, ConsensusEncoding, ConsensusEncodingSi

#[derive(Debug, Clone, Hash, PartialEq, Deserialize, Serialize, Eq, Default)]
pub struct SignerSignature {
pub signer: PublicKey,
pub signature: Signature,
signer: PublicKey,
signature: Signature,
}

impl SignerSignature {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,7 @@ pub fn create_acceptance_signature(
private_key: RistrettoSecretKey,
) -> Signature {
let challenge = ContractAcceptanceChallenge::new(&commitment, &contract_id);

SignerSignature::sign(&private_key, &challenge).signature
SignerSignature::sign(&private_key, &challenge).signature().clone()
}

pub fn create_random_key_pair() -> (RistrettoSecretKey, RistrettoPublicKey) {
Expand Down
2 changes: 0 additions & 2 deletions dan_layer/core/src/models/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use std::{convert::TryFrom, fmt::Debug, hash::Hash};
mod asset_definition;
mod base_layer_metadata;
mod base_layer_output;
mod checkpoint_challenge;
mod committee;
pub mod domain_events;
mod error;
Expand All @@ -45,7 +44,6 @@ mod view_id;
pub use asset_definition::{AssetDefinition, InitialState};
pub use base_layer_metadata::BaseLayerMetadata;
pub use base_layer_output::{BaseLayerOutput, CheckpointOutput, CommitteeOutput};
pub use checkpoint_challenge::CheckpointChallenge;
pub use committee::Committee;
pub use error::ModelError;
pub use hot_stuff_message::HotStuffMessage;
Expand Down
4 changes: 2 additions & 2 deletions dan_layer/core/src/services/acceptance_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ impl<TWallet: WalletClient + Sync + Send, TBaseNode: BaseNodeClient + Sync + Sen
let secret_key = node_identity.secret_key();
let constitution_commitment = self.fetch_constitution_commitment(contract_id).await?;
let challenge = ContractAcceptanceChallenge::new(&constitution_commitment, contract_id);
let signature = SignerSignature::sign(secret_key, challenge).signature;
let signature = SignerSignature::sign(secret_key, challenge);

// publish the acceptance
self.wallet
.submit_contract_acceptance(contract_id, public_key, &signature)
.submit_contract_acceptance(contract_id, public_key, signature.signature())
.await
}
}
Expand Down
4 changes: 2 additions & 2 deletions dan_layer/core/src/services/checkpoint_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub trait CheckpointManager {
&mut self,
checkpoint_number: u64,
state_root: StateRoot,
signature: Vec<SignerSignature>,
signature: &[SignerSignature],
) -> Result<(), DigitalAssetError>;
}

Expand All @@ -60,7 +60,7 @@ impl<TWallet: WalletClient + Sync + Send> CheckpointManager for ConcreteCheckpoi
&mut self,
checkpoint_number: u64,
state_root: StateRoot,
signatures: Vec<SignerSignature>,
signatures: &[SignerSignature],
) -> Result<(), DigitalAssetError> {
info!(
target: LOG_TARGET,
Expand Down
23 changes: 12 additions & 11 deletions dan_layer/core/src/services/mocks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,14 @@

use std::{
collections::VecDeque,
marker::PhantomData,
sync::{Arc, Mutex},
};

use async_trait::async_trait;
use tari_common_types::types::{FixedHash, PublicKey};
use tari_core::{
chain_storage::UtxoMinedInfo,
transactions::transaction_components::{OutputType, SignerSignature},
transactions::transaction_components::{CheckpointChallenge, OutputType, SignerSignature},
};
use tari_crypto::ristretto::RistrettoPublicKey;
use tari_dan_common_types::TemplateId;
Expand Down Expand Up @@ -201,18 +200,20 @@ impl<TEvent: Event> EventsPublisher<TEvent> for MockEventsPublisher<TEvent> {
}
}

pub fn mock_signing_service() -> MockSigningService<RistrettoPublicKey> {
MockSigningService::<RistrettoPublicKey> { p: PhantomData }
pub fn mock_signing_service() -> MockSigningService {
MockSigningService
}

pub struct MockSigningService<TAddr: NodeAddressable> {
p: PhantomData<TAddr>,
}
pub struct MockSigningService;

impl<TAddr: NodeAddressable> SigningService<TAddr> for MockSigningService<TAddr> {
fn sign(&self, _identity: &TAddr, _challenge: &[u8]) -> Result<ValidatorSignature, DigitalAssetError> {
impl SigningService for MockSigningService {
fn sign(&self, _challenge: &[u8]) -> Result<ValidatorSignature, DigitalAssetError> {
Ok(ValidatorSignature {})
}

fn sign_checkpoint(&self, _challenge: &CheckpointChallenge) -> Result<SignerSignature, DigitalAssetError> {
todo!()
}
}

#[derive(Clone)]
Expand Down Expand Up @@ -345,7 +346,7 @@ impl WalletClient for MockWalletClient {
_contract_id: &FixedHash,
_state_root: &StateRoot,
_checkpoint_number: u64,
_checkpoint_signatures: Vec<SignerSignature>,
_checkpoint_signatures: &[SignerSignature],
) -> Result<(), DigitalAssetError> {
Ok(())
}
Expand Down Expand Up @@ -493,7 +494,7 @@ impl ServiceSpecification for MockServiceSpecification {
type Payload = TariDanPayload;
type PayloadProcessor = MockPayloadProcessor;
type PayloadProvider = MockStaticPayloadProvider<Self::Payload>;
type SigningService = MockSigningService<Self::Addr>;
type SigningService = MockSigningService;
type StateDbBackendAdapter = MockStateDbBackupAdapter;
type ValidatorNodeClientFactory = MockValidatorNodeClientFactory;
type WalletClient = MockWalletClient;
Expand Down
2 changes: 1 addition & 1 deletion dan_layer/core/src/services/service_specification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub trait ServiceSpecification: Default + Clone {
type Payload: Payload;
type PayloadProcessor: PayloadProcessor<Self::Payload>;
type PayloadProvider: PayloadProvider<Self::Payload>;
type SigningService: SigningService<Self::Addr>;
type SigningService: SigningService;
type StateDbBackendAdapter: StateDbBackendAdapter;
type ValidatorNodeClientFactory: ValidatorNodeClientFactory<Addr = Self::Addr> + Clone;
type WalletClient: WalletClient + Clone;
Expand Down
27 changes: 13 additions & 14 deletions dan_layer/core/src/services/signing_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,15 @@

use std::sync::Arc;

use tari_comms::{types::CommsPublicKey, NodeIdentity};
use tari_comms::NodeIdentity;
use tari_core::transactions::transaction_components::{CheckpointChallenge, SignerSignature};

use crate::{
digital_assets_error::DigitalAssetError,
models::ValidatorSignature,
services::infrastructure_services::NodeAddressable,
};
use crate::{digital_assets_error::DigitalAssetError, models::ValidatorSignature};

pub trait SigningService<TAddr: NodeAddressable> {
fn sign(&self, identity: &TAddr, challenge: &[u8]) -> Result<ValidatorSignature, DigitalAssetError>;
pub trait SigningService {
fn sign(&self, challenge: &[u8]) -> Result<ValidatorSignature, DigitalAssetError>;

fn sign_checkpoint(&self, challenge: &CheckpointChallenge) -> Result<SignerSignature, DigitalAssetError>;
}

pub struct NodeIdentitySigningService {
Expand All @@ -44,13 +43,13 @@ impl NodeIdentitySigningService {
}
}

impl SigningService<CommsPublicKey> for NodeIdentitySigningService {
fn sign(&self, identity: &CommsPublicKey, _challenge: &[u8]) -> Result<ValidatorSignature, DigitalAssetError> {
if identity != self.node_identity.public_key() {
return Err(DigitalAssetError::InvalidSignature);
}

impl SigningService for NodeIdentitySigningService {
fn sign(&self, _challenge: &[u8]) -> Result<ValidatorSignature, DigitalAssetError> {
// TODO better sig
Ok(ValidatorSignature {})
}

fn sign_checkpoint(&self, challenge: &CheckpointChallenge) -> Result<SignerSignature, DigitalAssetError> {
Ok(SignerSignature::sign(self.node_identity.secret_key(), challenge))
}
}
2 changes: 1 addition & 1 deletion dan_layer/core/src/services/wallet_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub trait WalletClient: Send + Sync {
contract_id: &FixedHash,
state_root: &StateRoot,
checkpoint_number: u64,
checkpoint_signatures: Vec<SignerSignature>,
checkpoint_signatures: &[SignerSignature],
) -> Result<(), DigitalAssetError>;

async fn submit_contract_acceptance(
Expand Down
1 change: 0 additions & 1 deletion dan_layer/core/src/storage/metadata_backend_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

// TODO: probably want to use something like bors or consensus encoding
use tari_utilities::message_format::MessageFormat;

use crate::storage::AtomicDb;
Expand Down
Loading

0 comments on commit bcaabf0

Please sign in to comment.