Skip to content

Commit 27a04fa

Browse files
vicsnljedrz
authored andcommitted
perf: introduce compact Subdags
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com> Co-authored-by: ljedrz <ljedrz@users.noreply.github.com>
1 parent 52b46c9 commit 27a04fa

22 files changed

Lines changed: 346 additions & 299 deletions

File tree

Cargo.lock

Lines changed: 164 additions & 125 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ default-features = false
4747
[workspace.dependencies.snarkvm]
4848
#path = "../snarkVM"
4949
git = "https://github.com/ProvableHQ/snarkVM.git"
50-
rev = "bcded2d"
50+
branch = "perf/compact_subdag_rebased"
51+
#rev = "bcded2d"
5152
#version = "=4.3.0"
5253
default-features = false
5354

node/bft/examples/simple_node.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ fn consensus_handler(receiver: ConsensusReceiver<CurrentNetwork>) {
326326
tokio::task::spawn(async move {
327327
while let Some((subdag, transmissions, callback)) = rx_consensus_subdag.recv().await {
328328
// Determine the amount of time to sleep for the subdag.
329-
let subdag_ms = subdag.values().flatten().count();
329+
let subdag_ms = subdag.num_certificates();
330330
// Determine the amount of time to sleep for the transmissions.
331331
let transmissions_ms = transmissions.len() * 25;
332332
// Add a constant delay.

node/bft/ledger-service/Cargo.toml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@ workspace = true
3535
[dependencies.async-trait]
3636
version = "0.1"
3737

38-
[dependencies.indexmap]
39-
workspace = true
40-
features = [ "serde", "rayon" ]
41-
4238
[dependencies.locktick]
4339
workspace = true
4440
features = [ "parking_lot" ]

node/bft/ledger-service/src/ledger.rs

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ use crate::{LedgerService, fmt_id, spawn_blocking};
1717
use snarkvm::{
1818
ledger::{
1919
Ledger,
20+
SubdagTransmissions,
2021
block::{Block, Transaction},
2122
committee::Committee,
22-
narwhal::{BatchCertificate, Data, Subdag, Transmission, TransmissionID},
23+
narwhal::{Data, Subdag, Transmission, TransmissionID},
2324
puzzle::{Solution, SolutionID},
2425
store::ConsensusStorage,
2526
},
@@ -41,7 +42,6 @@ use snarkvm::{
4142
};
4243

4344
use anyhow::ensure;
44-
use indexmap::IndexMap;
4545
#[cfg(feature = "locktick")]
4646
use locktick::parking_lot::RwLock;
4747
#[cfg(not(feature = "locktick"))]
@@ -168,15 +168,6 @@ impl<N: Network, C: ConsensusStorage<N>> LedgerService<N> for CoreLedgerService<
168168
self.ledger.get_unconfirmed_transaction(&transaction_id)
169169
}
170170

171-
/// Returns the batch certificate for the given batch certificate ID.
172-
fn get_batch_certificate(&self, certificate_id: &Field<N>) -> Result<BatchCertificate<N>> {
173-
match self.ledger.get_batch_certificate(certificate_id) {
174-
Ok(Some(certificate)) => Ok(certificate),
175-
Ok(None) => bail!("No batch certificate found for certificate ID {certificate_id} in the ledger"),
176-
Err(error) => Err(error),
177-
}
178-
}
179-
180171
/// Returns the current committee.
181172
fn current_committee(&self) -> Result<Committee<N>> {
182173
self.ledger.latest_committee()
@@ -365,9 +356,9 @@ impl<N: Network, C: ConsensusStorage<N>> LedgerService<N> for CoreLedgerService<
365356
fn prepare_advance_to_next_quorum_block(
366357
&self,
367358
subdag: Subdag<N>,
368-
transmissions: IndexMap<TransmissionID<N>, Transmission<N>>,
359+
subdag_transmissions: SubdagTransmissions<N>,
369360
) -> Result<Block<N>> {
370-
self.ledger.prepare_advance_to_next_quorum_block(subdag, transmissions, &mut rand::thread_rng())
361+
self.ledger.prepare_advance_to_next_quorum_block(subdag, subdag_transmissions, &mut rand::thread_rng())
371362
}
372363

373364
/// Adds the given block as the next block in the ledger.

node/bft/ledger-service/src/mock.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@
1616
use crate::{LedgerService, fmt_id};
1717
use snarkvm::{
1818
ledger::{
19+
SubdagTransmissions,
1920
block::{Block, Transaction},
2021
committee::Committee,
21-
narwhal::{BatchCertificate, Data, Subdag, Transmission, TransmissionID},
22+
narwhal::{Data, Subdag, Transmission, TransmissionID},
2223
puzzle::{Solution, SolutionID},
2324
},
2425
prelude::{Address, ConsensusVersion, Field, Network, Result, Zero, bail, consensus_config_value, ensure},
2526
};
2627

27-
use indexmap::IndexMap;
2828
#[cfg(feature = "locktick")]
2929
use locktick::parking_lot::Mutex;
3030
#[cfg(not(feature = "locktick"))]
@@ -145,11 +145,6 @@ impl<N: Network> LedgerService<N> for MockLedgerService<N> {
145145
unreachable!("MockLedgerService does not support get_unconfirmed_transaction")
146146
}
147147

148-
/// Returns the batch certificate for the given batch certificate ID.
149-
fn get_batch_certificate(&self, _certificate_id: &Field<N>) -> Result<BatchCertificate<N>> {
150-
unreachable!("MockLedgerService does not support get_batch_certificate")
151-
}
152-
153148
/// Returns the current committee.
154149
fn current_committee(&self) -> Result<Committee<N>> {
155150
Ok(self.committee.clone())
@@ -221,7 +216,7 @@ impl<N: Network> LedgerService<N> for MockLedgerService<N> {
221216
fn prepare_advance_to_next_quorum_block(
222217
&self,
223218
_subdag: Subdag<N>,
224-
_transmissions: IndexMap<TransmissionID<N>, Transmission<N>>,
219+
_subdag_transmissions: SubdagTransmissions<N>,
225220
) -> Result<Block<N>> {
226221
unreachable!("MockLedgerService does not support prepare_advance_to_next_quorum_block")
227222
}

node/bft/ledger-service/src/prover.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@
1616
use crate::LedgerService;
1717
use snarkvm::{
1818
ledger::{
19+
SubdagTransmissions,
1920
block::{Block, Transaction},
2021
committee::Committee,
21-
narwhal::{BatchCertificate, Data, Subdag, Transmission, TransmissionID},
22+
narwhal::{Data, Subdag, Transmission, TransmissionID},
2223
puzzle::{Solution, SolutionID},
2324
},
2425
prelude::{Address, ConsensusVersion, Field, Network, Result, Zero, bail},
2526
};
2627

27-
use indexmap::IndexMap;
2828
use std::ops::Range;
2929

3030
/// A ledger service for a prover.
@@ -113,11 +113,6 @@ impl<N: Network> LedgerService<N> for ProverLedgerService<N> {
113113
bail!("Transaction '{transaction_id}' does not exist in prover")
114114
}
115115

116-
/// Returns the batch certificate for the given batch certificate ID.
117-
fn get_batch_certificate(&self, certificate_id: &Field<N>) -> Result<BatchCertificate<N>> {
118-
bail!("Batch certificate '{certificate_id}' does not exist in prover")
119-
}
120-
121116
/// Returns the current committee.
122117
fn current_committee(&self) -> Result<Committee<N>> {
123118
bail!("Committee does not exist in prover")
@@ -176,7 +171,7 @@ impl<N: Network> LedgerService<N> for ProverLedgerService<N> {
176171
fn prepare_advance_to_next_quorum_block(
177172
&self,
178173
_subdag: Subdag<N>,
179-
_transmissions: IndexMap<TransmissionID<N>, Transmission<N>>,
174+
_subdag_transmissions: SubdagTransmissions<N>,
180175
) -> Result<Block<N>> {
181176
bail!("Cannot prepare advance to next quorum block in prover")
182177
}

node/bft/ledger-service/src/traits.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@
1515

1616
use snarkvm::{
1717
ledger::{
18+
SubdagTransmissions,
1819
block::{Block, Transaction},
1920
committee::Committee,
20-
narwhal::{BatchCertificate, Data, Subdag, Transmission, TransmissionID},
21+
narwhal::{Data, Subdag, Transmission, TransmissionID},
2122
puzzle::{Solution, SolutionID},
2223
},
2324
prelude::{Address, ConsensusVersion, Field, Network, Result},
2425
};
2526

26-
use indexmap::IndexMap;
2727
use std::{fmt::Debug, ops::Range};
2828

2929
#[async_trait]
@@ -71,9 +71,6 @@ pub trait LedgerService<N: Network>: Debug + Send + Sync {
7171
/// Returns the unconfirmed transaction for the given transaction ID.
7272
fn get_unconfirmed_transaction(&self, transaction_id: N::TransactionID) -> Result<Transaction<N>>;
7373

74-
/// Returns the batch certificate for the given batch certificate ID.
75-
fn get_batch_certificate(&self, certificate_id: &Field<N>) -> Result<BatchCertificate<N>>;
76-
7774
/// Returns the current committee.
7875
fn current_committee(&self) -> Result<Committee<N>>;
7976

@@ -114,7 +111,7 @@ pub trait LedgerService<N: Network>: Debug + Send + Sync {
114111
fn prepare_advance_to_next_quorum_block(
115112
&self,
116113
subdag: Subdag<N>,
117-
transmissions: IndexMap<TransmissionID<N>, Transmission<N>>,
114+
subdag_transmissions: SubdagTransmissions<N>,
118115
) -> Result<Block<N>>;
119116

120117
/// Adds the given block as the next block in the ledger.

node/bft/ledger-service/src/translucent.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,18 @@
1515

1616
use crate::{CoreLedgerService, LedgerService};
1717
use async_trait::async_trait;
18-
use indexmap::IndexMap;
1918
use snarkvm::{
19+
console::network::{ConsensusVersion, Network},
2020
ledger::{
2121
Ledger,
22+
SubdagTransmissions,
2223
block::{Block, Transaction},
2324
committee::Committee,
2425
narwhal::{Data, Subdag, Transmission, TransmissionID},
2526
puzzle::{Solution, SolutionID},
2627
store::ConsensusStorage,
2728
},
28-
prelude::{Address, ConsensusVersion, Field, Network, Result, narwhal::BatchCertificate},
29+
prelude::{Address, Field, Result},
2930
};
3031
use std::{
3132
fmt,
@@ -124,11 +125,6 @@ impl<N: Network, C: ConsensusStorage<N>> LedgerService<N> for TranslucentLedgerS
124125
self.inner.get_unconfirmed_transaction(transaction_id)
125126
}
126127

127-
/// Returns the batch certificate for the given batch certificate ID.
128-
fn get_batch_certificate(&self, certificate_id: &Field<N>) -> Result<BatchCertificate<N>> {
129-
self.inner.get_batch_certificate(certificate_id)
130-
}
131-
132128
/// Returns the current committee.
133129
fn current_committee(&self) -> Result<Committee<N>> {
134130
self.inner.current_committee()
@@ -186,9 +182,9 @@ impl<N: Network, C: ConsensusStorage<N>> LedgerService<N> for TranslucentLedgerS
186182
fn prepare_advance_to_next_quorum_block(
187183
&self,
188184
subdag: Subdag<N>,
189-
transmissions: IndexMap<TransmissionID<N>, Transmission<N>>,
185+
subdag_transmissions: SubdagTransmissions<N>,
190186
) -> Result<Block<N>> {
191-
self.inner.prepare_advance_to_next_quorum_block(subdag, transmissions)
187+
self.inner.prepare_advance_to_next_quorum_block(subdag, subdag_transmissions)
192188
}
193189

194190
/// Adds the given block as the next block in the ledger.

node/bft/src/bft.rs

Lines changed: 53 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@ use snarkos_node_sync::{BlockSync, Ping};
3434
use snarkvm::{
3535
console::account::Address,
3636
ledger::{
37+
SubdagTransmissions,
3738
block::Transaction,
3839
committee::Committee,
39-
narwhal::{BatchCertificate, Data, Subdag, Transmission, TransmissionID},
40+
narwhal::{BatchCertificate, Data, NarwhalCertificate, Subdag, Transmission, TransmissionID},
4041
puzzle::{Solution, SolutionID},
4142
},
4243
prelude::{Field, Network, Result, bail, ensure},
@@ -636,10 +637,14 @@ impl<N: Network> BFT<N> {
636637
if !IS_SYNCING {
637638
// Initialize a map for the deduped transmissions.
638639
let mut transmissions = IndexMap::new();
640+
// Initialize a map for the deduped prior transmission ids.
641+
let mut prior_included_transmissions = IndexSet::new();
642+
// Initialize a map for transmission ids which could not be read from the ledger.
643+
let mut aborted_transmissions = IndexSet::new();
639644
// Initialize a map for the deduped transaction ids.
640-
let mut seen_transaction_ids = IndexSet::new();
645+
let mut seen_transaction_ids: IndexSet<N::TransactionID> = IndexSet::new();
641646
// Initialize a map for the deduped solution ids.
642-
let mut seen_solution_ids = IndexSet::new();
647+
let mut seen_solution_ids: IndexSet<SolutionID<N>> = IndexSet::new();
643648
// Start from the oldest leader certificate.
644649
for certificate in commit_subdag.values().flatten() {
645650
// Retrieve the transmissions.
@@ -650,7 +655,7 @@ impl<N: Network> BFT<N> {
650655
match transmission_id {
651656
TransmissionID::Solution(solution_id, _) => {
652657
// If the solution already exists, skip it.
653-
if seen_solution_ids.contains(&solution_id) {
658+
if seen_solution_ids.contains(solution_id) {
654659
continue;
655660
}
656661
}
@@ -670,41 +675,52 @@ impl<N: Network> BFT<N> {
670675
}
671676
// If the transmission already exists in the ledger, skip it.
672677
// Note: On failure to read from the ledger, we skip including this transmission, out of safety.
673-
if self.ledger().contains_transmission(transmission_id).unwrap_or(true) {
674-
continue;
675-
}
676-
// Retrieve the transmission.
677-
let Some(transmission) = self.storage().get_transmission(*transmission_id) else {
678-
bail!(
679-
"BFT failed to retrieve transmission '{}.{}' from round {}",
680-
fmt_id(transmission_id),
681-
fmt_id(transmission_id.checksum().unwrap_or_default()).dimmed(),
682-
certificate.round()
683-
);
684-
};
685-
// Insert the transaction ID or solution ID into the map.
686-
match transmission_id {
687-
TransmissionID::Solution(id, _) => {
688-
seen_solution_ids.insert(id);
678+
// Check if the ledger contains the transmission already.
679+
match self.ledger().contains_transmission(transmission_id) {
680+
// On failure to read from the ledger, we skip including this transmission, out of safety.
681+
Err(err) => {
682+
warn!("{}", err);
683+
aborted_transmissions.insert(*transmission_id);
684+
}
685+
// If the transmission already exists in the ledger, save just the transmission ID.
686+
Ok(true) => {
687+
prior_included_transmissions.insert(*transmission_id);
689688
}
690-
TransmissionID::Transaction(id, _) => {
691-
seen_transaction_ids.insert(id);
689+
// If the transmission does not exist in the ledger, retrieve it from the storage.
690+
Ok(false) => {
691+
// Retrieve the transmission.
692+
let Some(transmission) = self.storage().get_transmission(*transmission_id) else {
693+
bail!(
694+
"BFT failed to retrieve transmission '{}' from round {}",
695+
fmt_id(transmission_id),
696+
certificate.round()
697+
);
698+
};
699+
// Insert the transaction ID or solution ID into the map.
700+
match transmission_id {
701+
TransmissionID::Solution(id, _) => {
702+
seen_solution_ids.insert(*id);
703+
}
704+
TransmissionID::Transaction(id, _) => {
705+
seen_transaction_ids.insert(*id);
706+
}
707+
TransmissionID::Ratification => {}
708+
}
709+
// Add the transmission to the set.
710+
transmissions.insert(*transmission_id, transmission);
692711
}
693-
TransmissionID::Ratification => {}
694712
}
695-
// Add the transmission to the set.
696-
transmissions.insert(*transmission_id, transmission);
697713
}
698714
}
699715
// Trigger consensus, as this will build a new block for the ledger.
700716
// Construct the subdag.
701-
let subdag = Subdag::from(commit_subdag.clone())?;
717+
let subdag = Subdag::from_full(commit_subdag.clone())?;
702718
// Retrieve the anchor round.
703719
let anchor_round = subdag.anchor_round();
704720
// Retrieve the number of transmissions.
705721
let num_transmissions = transmissions.len();
706722
// Retrieve metadata about the subdag.
707-
let subdag_metadata = subdag.iter().map(|(round, c)| (*round, c.len())).collect::<Vec<_>>();
723+
let subdag_metadata = subdag.num_certificates_rounds();
708724

709725
// Ensure the subdag anchor round matches the leader round.
710726
ensure!(
@@ -714,10 +730,13 @@ impl<N: Network> BFT<N> {
714730

715731
// Trigger consensus.
716732
if let Some(consensus_sender) = self.consensus_sender.get() {
733+
// Construct subdag transmissions.
734+
let subdag_transmissions =
735+
SubdagTransmissions { transmissions, prior_included_transmissions, aborted_transmissions };
717736
// Initialize a callback sender and receiver.
718737
let (callback_sender, callback_receiver) = oneshot::channel();
719738
// Send the subdag and transmissions to consensus.
720-
consensus_sender.tx_consensus_subdag.send((subdag, transmissions, callback_sender)).await?;
739+
consensus_sender.tx_consensus_subdag.send((subdag, subdag_transmissions, callback_sender)).await?;
721740
// Await the callback to continue.
722741
match callback_receiver.await {
723742
Ok(Ok(())) => (), // continue
@@ -733,7 +752,7 @@ impl<N: Network> BFT<N> {
733752
}
734753

735754
info!(
736-
"\n\nCommitting a subdag from round {anchor_round} with {num_transmissions} transmissions: {subdag_metadata:?}\n"
755+
"\n\nCommitting a subdag from round {anchor_round} with {num_transmissions} new transmissions: {subdag_metadata:?}\n"
737756
);
738757
}
739758

@@ -745,7 +764,7 @@ impl<N: Network> BFT<N> {
745764

746765
// Update the validator telemetry.
747766
#[cfg(feature = "telemetry")]
748-
self.primary().gateway().validator_telemetry().insert_subdag(&Subdag::from(commit_subdag)?);
767+
self.primary().gateway().validator_telemetry().insert_subdag(&Subdag::from_full(commit_subdag)?);
749768
}
750769

751770
// Perform garbage collection based on the latest committed leader round.
@@ -960,7 +979,10 @@ mod tests {
960979
console::account::{Address, PrivateKey},
961980
ledger::{
962981
committee::Committee,
963-
narwhal::batch_certificate::test_helpers::{sample_batch_certificate, sample_batch_certificate_for_round},
982+
narwhal::{
983+
NarwhalCertificate,
984+
batch_certificate::test_helpers::{sample_batch_certificate, sample_batch_certificate_for_round},
985+
},
964986
},
965987
utilities::TestRng,
966988
};

0 commit comments

Comments
 (0)