chore(consensus): clean up TODOs (#8385)

# Description of change

- Adds issue tracking to TODOs in `starfish` consensus crate
- removes obsolite TODOs.
## Links to any relevant issues

closes #7529 

## How the change has been tested

- [ ] Basic tests (linting, compilation, formatting, unit/integration
tests)
- [ ] Patch-specific tests (correctness, functionality coverage)
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have checked that new and existing unit tests pass locally with
my changes

### Release Notes

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
- [ ] REST API:

Author: filipdulic

crates/starfish/core/src/commit.rs

@@ -146,7 +146,8 @@ impl CommitAPI for CommitV1 {
         &self.blocks
     }
 
-    // TODO: does this need to be a vector? block refs are a slice == less cloning?
+    // TODO: https://github.com/iotaledger/iota/issues/8375
+    // Does this need to be a vector? block refs are a slice == less cloning?
     fn committed_transactions(&self) -> Vec<BlockRef> {
         self.committed_transactions.clone()
     }

crates/starfish/core/src/commit_syncer.rs

@@ -368,7 +368,8 @@ impl<C: NetworkClient> CommitSyncer<C> {
                             "Fetched blocks have missing committed transactions: {:?} for commit range {:?}",
                             missing_committed_txns, fetched_commit_range
                         );
-                        // TODO: decide whether to rely on periodic transactions
+                        // TODO: https://github.com/iotaledger/iota/issues/8376
+                        // Decide whether to rely on periodic transactions
                         // synchronizer or make use of live one
                         if let Err(err) = self
                             .inner

crates/starfish/core/src/core.rs

@@ -26,11 +26,9 @@ use tokio::{
 use tracing::{debug, info, trace, warn};
 
 #[cfg(test)]
-use crate::{CommitConsumer, TransactionClient, storage::mem_store::MemStore};
-// TODO: remove this once CommittedSubDag is properly used again
-#[allow(unused_imports)]
+use crate::{CommitConsumer, CommittedSubDag, TransactionClient, storage::mem_store::MemStore};
 use crate::{
-    CommittedSubDag, Transaction,
+    Transaction,
     block_header::{
         BlockHeader, BlockHeaderAPI, BlockHeaderV1, BlockRef, BlockTimestampMs, GENESIS_ROUND,
         Round, SignedBlockHeader, Slot, TransactionsCommitment, VerifiedBlock, VerifiedBlockHeader,
@@ -58,7 +56,8 @@ const MAX_COMMIT_VOTES_PER_BLOCK: usize = 100;
 // reasonably larger than the number of validators because not all validators
 // create their blocks at the same pace. For now we set it as a
 // constant to not make the block header size too large.
-// TODO: after testing decide how to compress acknowledgments and move to a
+// TODO: https://github.com/iotaledger/iota/issues/8378
+// After testing decide how to compress acknowledgments and move to a
 // protocol config
 const MAX_ACKNOWLEDGMENTS_PER_BLOCK: usize = 400;
 
@@ -627,9 +626,6 @@ impl Core {
         // this would acknowledge the inclusion of transactions. Just let this
         // be done in the end of the method.
         let (transactions, ack_transactions, _limit_reached) = self.transaction_consumer.next();
-        // TODO: remove this info debug when transaction consumption is ensured to be
-        // aligned with expectation
-        debug!("{} transaction are consumed by a block", transactions.len());
         // Serialize the transaction
         let serialized_transactions = Transaction::serialize(&transactions)
             .expect("We should expect correct serialization for transactions");
@@ -702,7 +698,6 @@ impl Core {
         }
 
         // Construct verified transactions to be used for storing and broadcasting
-        // TODO: consume this transactions in the data manager and for broadcasting
         let verified_transactions = VerifiedTransactions::new(
             transactions,
             verified_block_header.reference(),
@@ -788,8 +783,6 @@ impl Core {
             // Always try to process the synced commits first. If there are certified
             // commits to process then the decided leaders and the commits will be returned.
 
-            // TODO: limit commits by commits_until_update, which may be needed when leader
-            // schedule length is reduced.
             let mut decided_leaders = self.committer.try_decide(self.last_decided_leader);
 
             // Truncate the decided leaders to fit the commit schedule limit.

crates/starfish/core/src/encoder.rs

@@ -71,12 +71,8 @@ impl ShardEncoder for ReedSolomonEncoder {
         let mut shard_bytes = (bytes_length + 4).div_ceil(info_length);
 
         // Ensure shard_bytes meets alignment requirements.
-        // TODO:
-        // - New version of the crate only requires shard_bytes to be even.
-        // - Create tests to check alignment.
-        // - Change 64 to 2 when the crate is updated.
-        if shard_bytes % 64 != 0 {
-            shard_bytes += 64 - shard_bytes % 64;
+        if shard_bytes % 2 != 0 {
+            shard_bytes += 1;
         }
 
         let length_with_padding = shard_bytes * info_length;

crates/starfish/core/src/error.rs

@@ -142,7 +142,7 @@ pub(crate) enum ConsensusError {
     InsufficientParentStakes { parent_stakes: Stake, quorum: Stake },
 
     #[error("Invalid transaction: {0}")]
-    InvalidTransaction(String), // TODO: To be used for transaction validation errors in tests
+    InvalidTransaction(String),
 
     #[error("Ancestors max timestamp {max_timestamp_ms} > block timestamp {block_timestamp_ms}")]
     InvalidBlockTimestamp {

crates/starfish/core/src/linearizer.rs

@@ -22,7 +22,8 @@ use crate::{
 
 /// The maximum depth of the linearizer, i.e. how many rounds back it will
 /// traverse the DAG from a committed leader block
-// TODO: make it derivable from the protocol parameters
+// TODO: https://github.com/iotaledger/iota/issues/8379
+// make it derivable from the protocol parameters
 pub(crate) const MAX_LINEARIZER_DEPTH: Round = 60;
 
 /// The `StorageAPI` trait provides an interface for the block store and has

crates/starfish/core/src/subscriber.rs

@@ -143,7 +143,8 @@ impl<C: NetworkClient, S: NetworkService> Subscriber<C, S> {
             }
             retries += 1;
 
-            // TODO:: Port PR 7292 from consensus crate to starfish
+            // TODO: https://github.com/iotaledger/iota/issues/8380
+            // Port PR 7292 from consensus crate to starfish
             let mut block_bundles = match network_client
                 .subscribe_block_bundles(peer, last_received, MAX_RETRY_INTERVAL)
                 .await
@@ -187,7 +188,6 @@ impl<C: NetworkClient, S: NetworkService> Subscriber<C, S> {
             'stream: loop {
                 match block_bundles.next().await {
                     Some(block) => {
-                        // TODO:: make new metric for bundle, rename, or leave it as it is?
                         context
                             .metrics
                             .node_metrics

crates/starfish/core/src/synchronizer.rs

@@ -656,8 +656,6 @@ impl<C: NetworkClient, V: BlockVerifier, D: CoreThreadDispatcher> Synchronizer<C
                 "Missing committed transactions after fetching blocks: {:?}",
                 missing_committed_txns
             );
-            // TODO: decide whether remove this live transactions synchronizer
-            // here or not.
             if let Err(err) = transactions_synchronizer
                 .fetch_transactions(missing_committed_txns)
                 .await