Skip to content

Commit 31fbc06

Browse files
committed
misc(node/bft): remove ALLOW_LEDGER_ACCESS parameter
1 parent 11bd713 commit 31fbc06

File tree

1 file changed

+31
-35
lines changed

1 file changed

+31
-35
lines changed

node/bft/src/bft.rs

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -507,10 +507,7 @@ impl<N: Network> BFT<N> {
507507
/// Stores the certificate in the DAG, and attempts to commit one or more anchors.
508508
///
509509
/// Callers of this function are required to hold the BFT write lock, for example, by calling [`Self::pause_for_block_sync`].
510-
async fn update_dag<const ALLOW_LEDGER_ACCESS: bool, const IS_SYNCING: bool>(
511-
&self,
512-
certificate: BatchCertificate<N>,
513-
) -> Result<()> {
510+
async fn update_dag<const IS_SYNCING: bool>(&self, certificate: BatchCertificate<N>) -> Result<()> {
514511
// ### First, insert the certificate into the DAG. ###
515512
// Retrieve the round of the new certificate to add to the DAG.
516513
let certificate_round = certificate.round();
@@ -595,11 +592,11 @@ impl<N: Network> BFT<N> {
595592
}
596593

597594
// Commit the leader certificate, and all previous leader certificates since the last committed round.
598-
self.commit_leader_certificate::<ALLOW_LEDGER_ACCESS, IS_SYNCING>(leader_certificate).await
595+
self.commit_leader_certificate::<IS_SYNCING>(leader_certificate).await
599596
}
600597

601598
/// Commits the leader certificate, and all previous leader certificates since the last committed round.
602-
async fn commit_leader_certificate<const ALLOW_LEDGER_ACCESS: bool, const IS_SYNCING: bool>(
599+
async fn commit_leader_certificate<const IS_SYNCING: bool>(
603600
&self,
604601
leader_certificate: BatchCertificate<N>,
605602
) -> Result<()> {
@@ -664,9 +661,8 @@ impl<N: Network> BFT<N> {
664661
// Retrieve the leader certificate round.
665662
let leader_round = leader_certificate.round();
666663
// Compute the commit subdag.
667-
let commit_subdag = self
668-
.order_dag_with_dfs::<ALLOW_LEDGER_ACCESS>(leader_certificate)
669-
.with_context(|| "BFT failed to order the DAG with DFS")?;
664+
let commit_subdag =
665+
self.order_dag_with_dfs(leader_certificate).with_context(|| "BFT failed to order the DAG with DFS")?;
670666
// If the node is not syncing, trigger consensus, as this will build a new block for the ledger.
671667
if !IS_SYNCING {
672668
// Initialize a map for the deduped transmissions.
@@ -814,7 +810,7 @@ impl<N: Network> BFT<N> {
814810
}
815811

816812
/// Returns the subdag of batch certificates to commit.
817-
fn order_dag_with_dfs<const ALLOW_LEDGER_ACCESS: bool>(
813+
fn order_dag_with_dfs(
818814
&self,
819815
leader_certificate: BatchCertificate<N>,
820816
) -> Result<BTreeMap<u64, IndexSet<BatchCertificate<N>>>> {
@@ -850,7 +846,7 @@ impl<N: Network> BFT<N> {
850846
continue;
851847
}
852848
// If the previous certificate already exists in the ledger, continue.
853-
if ALLOW_LEDGER_ACCESS && self.ledger().contains_certificate(previous_certificate_id).unwrap_or(false) {
849+
if self.ledger().contains_certificate(previous_certificate_id).unwrap_or(false) {
854850
continue;
855851
}
856852

@@ -936,7 +932,7 @@ impl<N: Network> BFT<N> {
936932
// Hold the lock while the primary is advancing blocks.
937933
let _lock = self_.lock.lock().await;
938934
// Update the DAG with the certificate.
939-
let result = self_.update_dag::<true, false>(certificate).await;
935+
let result = self_.update_dag::<false>(certificate).await;
940936
// Send the callback **after** updating the DAG.
941937
// Note: We must await the DAG update before proceeding.
942938
callback.send(result).ok();
@@ -957,7 +953,7 @@ impl<N: Network> BFT<N> {
957953
while let Some((certificate, callback)) = rx_sync_bft.recv().await {
958954
// Note: Do not lock here as `Sync` will already have called [`Self::pause_for_block_sync`]
959955
// Update the DAG with the certificate.
960-
let result = self_.update_dag::<true, true>(certificate).await;
956+
let result = self_.update_dag::<true>(certificate).await;
961957
// Send the callback **after** updating the DAG.
962958
// Note: We must await the DAG update before proceeding.
963959
callback.send(result).ok();
@@ -1382,11 +1378,11 @@ mod tests {
13821378

13831379
// Insert the previous certificates into the BFT.
13841380
for certificate in previous_certificates.clone() {
1385-
assert!(bft.update_dag::<false, false>(certificate).await.is_ok());
1381+
assert!(bft.update_dag::<false>(certificate).await.is_ok());
13861382
}
13871383

13881384
// Ensure this call succeeds and returns all given certificates.
1389-
let result = bft.order_dag_with_dfs::<false>(certificate.clone());
1385+
let result = bft.order_dag_with_dfs(certificate.clone());
13901386
assert!(result.is_ok());
13911387
let candidate_certificates = result.unwrap().into_values().flatten().collect::<Vec<_>>();
13921388
assert_eq!(candidate_certificates.len(), 1);
@@ -1412,11 +1408,11 @@ mod tests {
14121408

14131409
// Insert the previous certificates into the BFT.
14141410
for certificate in previous_certificates.clone() {
1415-
assert!(bft.update_dag::<false, false>(certificate).await.is_ok());
1411+
assert!(bft.update_dag::<false>(certificate).await.is_ok());
14161412
}
14171413

14181414
// Ensure this call succeeds and returns all given certificates.
1419-
let result = bft.order_dag_with_dfs::<false>(certificate.clone());
1415+
let result = bft.order_dag_with_dfs(certificate.clone());
14201416
assert!(result.is_ok());
14211417
let candidate_certificates = result.unwrap().into_values().flatten().collect::<Vec<_>>();
14221418
assert_eq!(candidate_certificates.len(), 5);
@@ -1472,7 +1468,7 @@ mod tests {
14721468
);
14731469

14741470
// Ensure this call fails on a missing previous certificate.
1475-
let result = bft.order_dag_with_dfs::<false>(certificate);
1471+
let result = bft.order_dag_with_dfs(certificate);
14761472
assert!(result.is_err());
14771473
assert_eq!(result.unwrap_err().to_string(), error_msg);
14781474
Ok(())
@@ -1533,11 +1529,11 @@ mod tests {
15331529

15341530
// Insert the certificates into the BFT.
15351531
for certificate in certificates {
1536-
assert!(bft.update_dag::<false, false>(certificate).await.is_ok());
1532+
assert!(bft.update_dag::<false>(certificate).await.is_ok());
15371533
}
15381534

15391535
// Commit the leader certificate.
1540-
bft.commit_leader_certificate::<false, false>(leader_certificate).await.unwrap();
1536+
bft.commit_leader_certificate::<false>(leader_certificate).await.unwrap();
15411537

15421538
// Ensure that the `gc_round` has been updated.
15431539
assert_eq!(bft.storage().gc_round(), commit_round - max_gc_rounds);
@@ -1597,11 +1593,11 @@ mod tests {
15971593

15981594
// Insert the previous certificates into the BFT.
15991595
for certificate in certificates.clone() {
1600-
assert!(bft.update_dag::<false, false>(certificate).await.is_ok());
1596+
assert!(bft.update_dag::<false>(certificate).await.is_ok());
16011597
}
16021598

16031599
// Commit the leader certificate.
1604-
bft.commit_leader_certificate::<false, false>(leader_certificate.clone()).await.unwrap();
1600+
bft.commit_leader_certificate::<false>(leader_certificate.clone()).await.unwrap();
16051601

16061602
// Simulate a bootup of the BFT.
16071603

@@ -1769,17 +1765,17 @@ mod tests {
17691765

17701766
// Insert the certificates into the BFT without bootup.
17711767
for certificate in pre_shutdown_certificates.clone() {
1772-
assert!(bft.update_dag::<false, false>(certificate).await.is_ok());
1768+
assert!(bft.update_dag::<false>(certificate).await.is_ok());
17731769
}
17741770

17751771
// Insert the post shutdown certificates into the BFT without bootup.
17761772
for certificate in post_shutdown_certificates.clone() {
1777-
assert!(bft.update_dag::<false, false>(certificate).await.is_ok());
1773+
assert!(bft.update_dag::<false>(certificate).await.is_ok());
17781774
}
17791775
// Commit the second leader certificate.
1780-
let commit_subdag = bft.order_dag_with_dfs::<false>(next_leader_certificate.clone()).unwrap();
1776+
let commit_subdag = bft.order_dag_with_dfs(next_leader_certificate.clone()).unwrap();
17811777
let commit_subdag_metadata = commit_subdag.iter().map(|(round, c)| (*round, c.len())).collect::<Vec<_>>();
1782-
bft.commit_leader_certificate::<false, false>(next_leader_certificate.clone()).await.unwrap();
1778+
bft.commit_leader_certificate::<false>(next_leader_certificate.clone()).await.unwrap();
17831779

17841780
// Simulate a bootup of the BFT.
17851781

@@ -1797,14 +1793,14 @@ mod tests {
17971793
bootup_bft.storage().testing_only_insert_certificate_testing_only(certificate.clone());
17981794
}
17991795
for certificate in post_shutdown_certificates.clone() {
1800-
assert!(bootup_bft.update_dag::<false, false>(certificate).await.is_ok());
1796+
assert!(bootup_bft.update_dag::<false>(certificate).await.is_ok());
18011797
}
18021798
// Commit the second leader certificate.
1803-
let commit_subdag_bootup = bootup_bft.order_dag_with_dfs::<false>(next_leader_certificate.clone()).unwrap();
1799+
let commit_subdag_bootup = bootup_bft.order_dag_with_dfs(next_leader_certificate.clone()).unwrap();
18041800
let commit_subdag_metadata_bootup =
18051801
commit_subdag_bootup.iter().map(|(round, c)| (*round, c.len())).collect::<Vec<_>>();
18061802
let committed_certificates_bootup = commit_subdag_bootup.values().flatten();
1807-
bootup_bft.commit_leader_certificate::<false, false>(next_leader_certificate.clone()).await.unwrap();
1803+
bootup_bft.commit_leader_certificate::<false>(next_leader_certificate.clone()).await.unwrap();
18081804

18091805
// Check that the final state of both BFTs is the same.
18101806

@@ -1985,12 +1981,12 @@ mod tests {
19851981

19861982
// Insert the post shutdown certificates into the DAG.
19871983
for certificate in post_shutdown_certificates.clone() {
1988-
assert!(bootup_bft.update_dag::<false, false>(certificate).await.is_ok());
1984+
assert!(bootup_bft.update_dag::<false>(certificate).await.is_ok());
19891985
}
19901986

19911987
// Get the next leader certificate to commit.
19921988
let next_leader_certificate = storage.get_certificate_for_round_with_author(next_round, next_leader).unwrap();
1993-
let commit_subdag = bootup_bft.order_dag_with_dfs::<false>(next_leader_certificate).unwrap();
1989+
let commit_subdag = bootup_bft.order_dag_with_dfs(next_leader_certificate).unwrap();
19941990
let committed_certificates = commit_subdag.values().flatten();
19951991

19961992
// Check that none of the certificates synced from the bootup appear in the subdag for the next commit round.
@@ -2125,7 +2121,7 @@ mod tests {
21252121
// Insert all certificates into the storage and DAG.
21262122
for certificate in certificates_by_round.into_iter().flat_map(|(_, certs)| certs) {
21272123
storage.testing_only_insert_certificate_testing_only(certificate.clone());
2128-
bft.update_dag::<false, false>(certificate).await.unwrap();
2124+
bft.update_dag::<false>(certificate).await.unwrap();
21292125
}
21302126

21312127
let leader_certificate_2 = storage.get_certificate_for_round_with_author(leader_round_2, leader2).unwrap();
@@ -2136,7 +2132,7 @@ mod tests {
21362132
);
21372133

21382134
// Explicitely commit leader certificate 2.
2139-
bft.commit_leader_certificate::<false, false>(leader_certificate_2.clone()).await.unwrap();
2135+
bft.commit_leader_certificate::<false>(leader_certificate_2.clone()).await.unwrap();
21402136

21412137
// Leader certificate 1 should be committed transitively when committing the leader certificate 2.
21422138
assert!(
@@ -2279,7 +2275,7 @@ mod tests {
22792275
// Insert all certificates into the storage and DAG.
22802276
for certificate in certificates_by_round.into_iter().flat_map(|(_, certs)| certs) {
22812277
storage.testing_only_insert_certificate_testing_only(certificate.clone());
2282-
bft.update_dag::<false, false>(certificate).await.unwrap();
2278+
bft.update_dag::<false>(certificate).await.unwrap();
22832279
}
22842280

22852281
let leader_certificate_2 = storage.get_certificate_for_round_with_author(leader_round_2, leader2).unwrap();
@@ -2291,7 +2287,7 @@ mod tests {
22912287
assert_eq!(bft.dag.read().last_committed_round(), 0);
22922288

22932289
// Explicitely commit leader certificate 2.
2294-
bft.commit_leader_certificate::<false, false>(leader_certificate_2.clone()).await.unwrap();
2290+
bft.commit_leader_certificate::<false>(leader_certificate_2.clone()).await.unwrap();
22952291

22962292
// Leader certificate 1 should be committed transitively when committing the leader certificate 2.
22972293
assert!(

0 commit comments

Comments
 (0)