Skip to content

Commit

Permalink
feat: sign proposal (#858)
Browse files Browse the repository at this point in the history
Description
---
Sing the proposal (the proposer signs the block).

Motivation and Context
---

How Has This Been Tested?
---
I just did sample run on dan-testing.

What process can a PR reviewer use to test or verify this change?
---
You can run the dan-testing and maybe do a debug logs on the signature
check to make sure that the checks are enforced.


Breaking Changes
---

- [x] None
- [ ] Requires data directory to be deleted
- [ ] Other - Please specify
  • Loading branch information
Cifko authored Jan 3, 2024
1 parent 3b769ec commit 5139cbb
Show file tree
Hide file tree
Showing 21 changed files with 91 additions and 23 deletions.
2 changes: 1 addition & 1 deletion applications/tari_validator_node/src/consensus/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ impl ConsensusSpec for TariConsensusSpec {
type Addr = PeerAddress;
type EpochManager = EpochManagerHandle<Self::Addr>;
type LeaderStrategy = RoundRobinLeaderStrategy;
type SignatureService = TariSignatureService;
type StateManager = TariStateManager;
type StateStore = SqliteStateStore<Self::Addr>;
type SyncManager = CommsRpcStateSyncManager<Self::EpochManager, Self::StateStore>;
type VoteSignatureService = TariSignatureService;
}
24 changes: 24 additions & 0 deletions dan_layer/consensus/src/block_validations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,30 @@ pub fn check_proposed_by_leader<TAddr: DerivableFromPublicKey, TLeaderStrategy:
Ok(())
}

pub fn check_signature(candidate_block: &Block) -> Result<(), ProposalValidationError> {
if candidate_block.is_dummy() {
// Dummy blocks don't have signatures
return Ok(());
}
if candidate_block.is_genesis() {
// Genesis block doesn't have signatures
return Ok(());
}
let validator_signature = candidate_block
.get_signature()
.ok_or(ProposalValidationError::MissingSignature {
block_id: *candidate_block.id(),
height: candidate_block.height(),
})?;
if !validator_signature.verify(candidate_block.proposed_by(), candidate_block.id()) {
return Err(ProposalValidationError::InvalidSignature {
block_id: *candidate_block.id(),
height: candidate_block.height(),
});
}
Ok(())
}

pub fn check_quorum_certificate<TAddr: NodeAddressable>(
_local_committee: &Committee<TAddr>,
candidate_block: &Block,
Expand Down
4 changes: 4 additions & 0 deletions dan_layer/consensus/src/hotstuff/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,8 @@ pub enum ProposalValidationError {
},
#[error("Proposed block {block_id} {height} already has been processed")]
BlockAlreadyProcessed { block_id: BlockId, height: NodeHeight },
#[error("Proposed block {block_id} {height} doesn't have signature")]
MissingSignature { block_id: BlockId, height: NodeHeight },
#[error("Proposed block {block_id} {height} has invalid signature")]
InvalidSignature { block_id: BlockId, height: NodeHeight },
}
3 changes: 2 additions & 1 deletion dan_layer/consensus/src/hotstuff/on_inbound_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use tari_transaction::TransactionId;
use tokio::{sync::mpsc, time};

use crate::{
block_validations::{check_hash_and_height, check_proposed_by_leader, check_quorum_certificate},
block_validations::{check_hash_and_height, check_proposed_by_leader, check_quorum_certificate, check_signature},
hotstuff::{error::HotStuffError, pacemaker_handle::PaceMakerHandle},
messages::{HotstuffMessage, ProposalMessage, RequestMissingTransactionsMessage},
traits::ConsensusSpec,
Expand Down Expand Up @@ -163,6 +163,7 @@ where TConsensusSpec: ConsensusSpec
.get_committee_by_validator_public_key(block.epoch(), block.proposed_by())
.await?;
check_proposed_by_leader(&self.leader_strategy, &committee_for_block, &block)?;
check_signature(&block)?;
check_quorum_certificate(&committee_for_block, &block)?;

let Some(ready_block) = self.handle_missing_transactions(block).await? else {
Expand Down
13 changes: 9 additions & 4 deletions dan_layer/consensus/src/hotstuff/on_propose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use super::common::CommitteeAndMessage;
use crate::{
hotstuff::{common::EXHAUST_DIVISOR, error::HotStuffError, proposer},
messages::{HotstuffMessage, ProposalMessage},
traits::ConsensusSpec,
traits::{ConsensusSpec, ValidatorSignatureService},
};

const LOG_TARGET: &str = "tari::dan::consensus::hotstuff::on_propose_locally";
Expand All @@ -44,6 +44,7 @@ pub struct OnPropose<TConsensusSpec: ConsensusSpec> {
store: TConsensusSpec::StateStore,
epoch_manager: TConsensusSpec::EpochManager,
transaction_pool: TransactionPool<TConsensusSpec::StateStore>,
signing_service: TConsensusSpec::SignatureService,
tx_broadcast: mpsc::Sender<CommitteeAndMessage<TConsensusSpec::Addr>>,
}

Expand All @@ -54,12 +55,14 @@ where TConsensusSpec: ConsensusSpec
store: TConsensusSpec::StateStore,
epoch_manager: TConsensusSpec::EpochManager,
transaction_pool: TransactionPool<TConsensusSpec::StateStore>,
signing_service: TConsensusSpec::SignatureService,
tx_broadcast: mpsc::Sender<CommitteeAndMessage<TConsensusSpec::Addr>>,
) -> Self {
Self {
store,
epoch_manager,
transaction_pool,
signing_service,
tx_broadcast,
}
}
Expand Down Expand Up @@ -262,14 +265,12 @@ where TConsensusSpec: ConsensusSpec
local_committee_shard.bucket(),
)?;

// let mut foreign_indexes = HashMap::new();

let foreign_indexes = non_local_buckets
.iter()
.map(|bucket| (*bucket, foreign_counters.increment_counter(*bucket)))
.collect();

let next_block = Block::new(
let mut next_block = Block::new(
*parent_block.block_id(),
high_qc,
parent_block.height() + NodeHeight(1),
Expand All @@ -278,8 +279,12 @@ where TConsensusSpec: ConsensusSpec
commands,
total_leader_fee,
foreign_indexes,
None,
);

let signature = self.signing_service.sign(next_block.id());
next_block.set_signature(signature);

Ok(next_block)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub struct OnReadyToVoteOnLocalBlock<TConsensusSpec: ConsensusSpec> {
validator_addr: TConsensusSpec::Addr,
store: TConsensusSpec::StateStore,
epoch_manager: TConsensusSpec::EpochManager,
vote_signing_service: TConsensusSpec::VoteSignatureService,
vote_signing_service: TConsensusSpec::SignatureService,
leader_strategy: TConsensusSpec::LeaderStrategy,
state_manager: TConsensusSpec::StateManager,
transaction_pool: TransactionPool<TConsensusSpec::StateStore>,
Expand All @@ -68,7 +68,7 @@ where TConsensusSpec: ConsensusSpec
validator_addr: TConsensusSpec::Addr,
store: TConsensusSpec::StateStore,
epoch_manager: TConsensusSpec::EpochManager,
vote_signing_service: TConsensusSpec::VoteSignatureService,
vote_signing_service: TConsensusSpec::SignatureService,
leader_strategy: TConsensusSpec::LeaderStrategy,
state_manager: TConsensusSpec::StateManager,
transaction_pool: TransactionPool<TConsensusSpec::StateStore>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<TConsensusSpec: ConsensusSpec> OnReceiveProposalHandler<TConsensusSpec> {
leader_strategy: TConsensusSpec::LeaderStrategy,
pacemaker: PaceMakerHandle,
tx_leader: mpsc::Sender<(TConsensusSpec::Addr, HotstuffMessage)>,
vote_signing_service: TConsensusSpec::VoteSignatureService,
vote_signing_service: TConsensusSpec::SignatureService,
state_manager: TConsensusSpec::StateManager,
transaction_pool: TransactionPool<TConsensusSpec::StateStore>,
tx_events: broadcast::Sender<HotstuffEvent>,
Expand Down
4 changes: 2 additions & 2 deletions dan_layer/consensus/src/hotstuff/vote_receiver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct VoteReceiver<TConsensusSpec: ConsensusSpec> {
store: TConsensusSpec::StateStore,
leader_strategy: TConsensusSpec::LeaderStrategy,
epoch_manager: TConsensusSpec::EpochManager,
vote_signature_service: TConsensusSpec::VoteSignatureService,
vote_signature_service: TConsensusSpec::SignatureService,
pacemaker: PaceMakerHandle,
}

Expand All @@ -36,7 +36,7 @@ where TConsensusSpec: ConsensusSpec
store: TConsensusSpec::StateStore,
leader_strategy: TConsensusSpec::LeaderStrategy,
epoch_manager: TConsensusSpec::EpochManager,
vote_signature_service: TConsensusSpec::VoteSignatureService,
vote_signature_service: TConsensusSpec::SignatureService,
pacemaker: PaceMakerHandle,
) -> Self {
Self {
Expand Down
5 changes: 3 additions & 2 deletions dan_layer/consensus/src/hotstuff/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl<TConsensusSpec: ConsensusSpec> HotstuffWorker<TConsensusSpec> {
state_store: TConsensusSpec::StateStore,
epoch_manager: TConsensusSpec::EpochManager,
leader_strategy: TConsensusSpec::LeaderStrategy,
signing_service: TConsensusSpec::VoteSignatureService,
signing_service: TConsensusSpec::SignatureService,
state_manager: TConsensusSpec::StateManager,
transaction_pool: TransactionPool<TConsensusSpec::StateStore>,
tx_broadcast: mpsc::Sender<CommitteeAndMessage<TConsensusSpec::Addr>>,
Expand Down Expand Up @@ -127,7 +127,7 @@ impl<TConsensusSpec: ConsensusSpec> HotstuffWorker<TConsensusSpec> {
leader_strategy.clone(),
pacemaker.clone_handle(),
tx_leader.clone(),
signing_service,
signing_service.clone(),
state_manager,
transaction_pool.clone(),
tx_events,
Expand Down Expand Up @@ -157,6 +157,7 @@ impl<TConsensusSpec: ConsensusSpec> HotstuffWorker<TConsensusSpec> {
state_store.clone(),
epoch_manager.clone(),
transaction_pool.clone(),
signing_service,
tx_broadcast,
),

Expand Down
2 changes: 1 addition & 1 deletion dan_layer/consensus/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub trait ConsensusSpec: Send + Sync + Clone + 'static {
type StateStore: StateStore<Addr = Self::Addr> + Send + Sync + Clone + 'static;
type EpochManager: EpochManagerReader<Addr = Self::Addr> + Send + Sync + Clone + 'static;
type LeaderStrategy: LeaderStrategy<Self::Addr> + Send + Sync + Clone + 'static;
type VoteSignatureService: VoteSignatureService + Send + Sync + Clone + 'static;
type SignatureService: VoteSignatureService + ValidatorSignatureService + Send + Sync + Clone + 'static;
type StateManager: StateManager<Self::StateStore> + Send + Sync + 'static;
type SyncManager: SyncManager + Send + Sync + 'static;
}
10 changes: 7 additions & 3 deletions dan_layer/consensus_tests/src/support/signing_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
use rand::rngs::OsRng;
use tari_common_types::types::{FixedHash, PrivateKey, PublicKey};
use tari_consensus::traits::{ValidatorSignatureService, VoteSignatureService};
use tari_crypto::keys::PublicKey as _;
use tari_crypto::keys::SecretKey;
use tari_dan_storage::consensus_models::{BlockId, QuorumDecision, ValidatorSchnorrSignature, ValidatorSignature};

use super::TestAddress;

#[derive(Debug, Clone)]
pub struct TestVoteSignatureService {
pub public_key: PublicKey,
Expand All @@ -15,8 +17,10 @@ pub struct TestVoteSignatureService {
}

impl TestVoteSignatureService {
pub fn new(public_key: PublicKey) -> Self {
let (secret_key, _public_key) = PublicKey::random_keypair(&mut OsRng);
pub fn new(public_key: PublicKey, addr: TestAddress) -> Self {
let mut bytes = [0u8; 64];
bytes[0..addr.0.as_bytes().len()].copy_from_slice(addr.0.as_bytes());
let secret_key = PrivateKey::from_uniform_bytes(&bytes).unwrap();
Self {
public_key,
secret_key,
Expand Down
2 changes: 1 addition & 1 deletion dan_layer/consensus_tests/src/support/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ impl ConsensusSpec for TestConsensusSpec {
type Addr = TestAddress;
type EpochManager = TestEpochManager;
type LeaderStrategy = RoundRobinLeaderStrategy;
type SignatureService = TestVoteSignatureService;
type StateManager = NoopStateManager;
type StateStore = SqliteStateStore<Self::Addr>;
type SyncManager = AlwaysSyncedSyncManager;
type VoteSignatureService = TestVoteSignatureService;
}
2 changes: 1 addition & 1 deletion dan_layer/consensus_tests/src/support/validator/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl ValidatorBuilder {
let (tx_mempool, rx_mempool) = mpsc::unbounded_channel();

let store = SqliteStateStore::connect(&self.sql_url).unwrap();
let signing_service = TestVoteSignatureService::new(self.public_key.clone());
let signing_service = TestVoteSignatureService::new(self.public_key.clone(), self.address.clone());
let transaction_pool = TransactionPool::new();
let noop_state_manager = NoopStateManager::new();
let (tx_events, _) = broadcast::channel(100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ create table blocks
is_processed boolean not NULL,
is_dummy boolean not NULL,
foreign_indexes text not NULL,
signature text NULL,
created_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
FOREIGN KEY (qc_id) REFERENCES quorum_certificates (qc_id)
);
Expand All @@ -46,6 +47,7 @@ create table parked_blocks
commands text not NULL,
total_leader_fee bigint not NULL,
foreign_indexes text not NULL,
signature text NULL,
created_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP
);

Expand Down
2 changes: 2 additions & 0 deletions dan_layer/state_store_sqlite/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ diesel::table! {
is_processed -> Bool,
is_dummy -> Bool,
foreign_indexes -> Text,
signature -> Nullable<Text>,
created_at -> Timestamp,
}
}
Expand Down Expand Up @@ -149,6 +150,7 @@ diesel::table! {
commands -> Text,
total_leader_fee -> BigInt,
foreign_indexes -> Text,
signature -> Nullable<Text>,
created_at -> Timestamp,
}
}
Expand Down
4 changes: 4 additions & 0 deletions dan_layer/state_store_sqlite/src/sql_models/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub struct Block {
pub is_processed: bool,
pub is_dummy: bool,
pub foreign_indexes: String,
pub signature: Option<String>,
pub created_at: PrimitiveDateTime,
}

Expand All @@ -54,6 +55,7 @@ impl Block {
self.is_processed,
self.is_committed,
deserialize_json(&self.foreign_indexes)?,
self.signature.map(|val| deserialize_json(&val)).transpose()?,
self.created_at,
))
}
Expand All @@ -72,6 +74,7 @@ pub struct ParkedBlock {
pub commands: String,
pub total_leader_fee: i64,
pub foreign_indexes: String,
pub signature: Option<String>,
pub created_at: PrimitiveDateTime,
}

Expand All @@ -98,6 +101,7 @@ impl TryFrom<ParkedBlock> for consensus_models::Block {
false,
false,
deserialize_json(&value.foreign_indexes)?,
value.signature.map(|val| deserialize_json(&val)).transpose()?,
value.created_at,
))
}
Expand Down
1 change: 1 addition & 0 deletions dan_layer/state_store_sqlite/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ mod confirm_all_transitions {
[Command::Prepare(atom1.clone())].into_iter().collect(),
Default::default(),
HashMap::new(),
None,
);
block1.insert(&mut tx).unwrap();

Expand Down
Loading

0 comments on commit 5139cbb

Please sign in to comment.