Delete outdated TODOs refering to closed issues (#6732)

* ZIPs were updated to remove ambiguity, this was tracked in #1267. * #2105 was fixed by #3039 and #2379 was closed by #3069 * #2230 was a duplicate of #2231 which was closed by #2511 * #3235 was obsoleted by #2156 which was fixed by #3505 * #1850 was fixed by #2944, #1851 was fixed by #2961 and #2902 was fixed by #2969 * We migrated to Rust 2021 edition in Jan 2022 with #3332 * #1631 was closed as not needed * #338 was fixed by #3040 and #1162 was fixed by #3067 * #2079 was fixed by #2445 * #4794 was fixed by #6122 * #1678 stopped being an issue * #3151 was fixed by #3934 * #3204 was closed as not needed * #1213 was fixed by #4586 * #1774 was closed as not needed * #4633 was closed as not needed * Clarify behaviour of difficulty spacing Co-authored-by: teor <teor@riseup.net> * Update comment to reflect implemented behaviour Co-authored-by: teor <teor@riseup.net> * Update comment to reflect implemented behaviour when retrying block downloads Co-authored-by: teor <teor@riseup.net> * Update `TODO` to remove closed issue and clarify when we might want to fix Co-authored-by: teor <teor@riseup.net> * Update `TODO` to remove closed issue and clarify what we might want to change in future Co-authored-by: teor <teor@riseup.net> * Clarify benefits of how we do block verification Co-authored-by: teor <teor@riseup.net> * Fix rustfmt errors --------- Co-authored-by: teor <teor@riseup.net>
ZcashFoundation · May 23, 2023 · ec2e9ca · ec2e9ca
1 parent f2be848
commit ec2e9ca
Show file tree

Hide file tree

Showing 15 changed files with 17 additions and 31 deletions.
diff --git a/zebra-chain/src/orchard/sinsemilla.rs b/zebra-chain/src/orchard/sinsemilla.rs
@@ -159,10 +159,6 @@ pub fn sinsemilla_hash(D: &[u8], M: &BitVec<u8, Lsb0>) -> Option<pallas::Base> {
     extract_p_bottom(sinsemilla_hash_to_point(D, M))
 }
 
-// TODO: test the above correctness and compatibility with the zcash-hackworks test vectors
-// https://github.com/ZcashFoundation/zebra/issues/2079
-// https://github.com/zcash-hackworks/zcash-test-vectors/pulls
-
 #[cfg(test)]
 mod tests {
 

diff --git a/zebra-chain/src/work/difficulty/tests/vectors.rs b/zebra-chain/src/work/difficulty/tests/vectors.rs
@@ -457,7 +457,9 @@ fn check_testnet_minimum_difficulty_block(height: block::Height) -> Result<(), R
             .signed_duration_since(previous_block.header.time);
 
         // zcashd requires a gap that's strictly greater than 6 times the target
-        // threshold, but ZIP-205 and ZIP-208 are ambiguous. See bug #1276.
+        // threshold, as documented in ZIP-205 and ZIP-208:
+        // https://zips.z.cash/zip-0205#change-to-difficulty-adjustment-on-testnet
+        // https://zips.z.cash/zip-0208#minimum-difficulty-blocks-on-testnet
         match NetworkUpgrade::minimum_difficulty_spacing_for_height(Network::Testnet, height) {
             None => Err(eyre!("the minimum difficulty rule is not active"))?,
             Some(spacing) if (time_gap <= spacing) => Err(eyre!(

diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs
@@ -740,10 +740,6 @@ where
             orchard_shielded_data,
             &shielded_sighash,
         )?))
-
-        // TODO:
-        // - verify orchard shielded pool (ZIP-224) (#2105)
-        // - shielded input and output limits? (#2379)
     }
 
     /// Verifies if a V5 `transaction` is supported by `network_upgrade`.

diff --git a/zebra-network/src/config.rs b/zebra-network/src/config.rs
@@ -347,7 +347,6 @@ impl<'de> Deserialize<'de> for Config {
 
         let config = DConfig::deserialize(deserializer)?;
 
-        // TODO: perform listener DNS lookups asynchronously with a timeout (#1631)
         let listen_addr = match config.listen_addr.parse::<SocketAddr>() {
             Ok(socket) => Ok(socket),
             Err(_) => match config.listen_addr.parse::<IpAddr>() {

diff --git a/zebra-network/src/peer_set/limit.rs b/zebra-network/src/peer_set/limit.rs
@@ -178,8 +178,9 @@ impl Drop for ConnectionTracker {
 
         // We ignore disconnected errors, because the receiver can be dropped
         // before some connections are dropped.
+        // # Security
         //
-        // TODO: This channel will be bounded by the connection limit (#1850, #1851, #2902).
+        // This channel is actually bounded by the inbound and outbound connection limit.
         let _ = self.close_notification_tx.send(ConnectionClosed);
     }
 }
diff --git a/zebra-network/src/peer_set/set.rs b/zebra-network/src/peer_set/set.rs
@@ -418,8 +418,6 @@ where
         for guard in self.guards.iter() {
             guard.abort();
         }
-
-        // TODO: implement graceful shutdown for InventoryRegistry (#1678)
     }
 
     /// Check busy peer services for request completion or errors.

diff --git a/zebra-network/src/peer_set/unready_service/tests/vectors.rs b/zebra-network/src/peer_set/unready_service/tests/vectors.rs
@@ -1,6 +1,4 @@
 //! Fixed test vectors for unready services.
-//!
-//! TODO: test that inner service errors are handled correctly (#3204)
 
 use std::marker::PhantomData;
 

diff --git a/zebra-network/src/policies.rs b/zebra-network/src/policies.rs
@@ -35,8 +35,11 @@ impl<Req: Clone + std::fmt::Debug, Res, E: std::fmt::Debug> Policy<Req, Res, E>
                     // Let other tasks run, so we're more likely to choose a different peer,
                     // and so that any notfound inv entries win the race to the PeerSet.
                     //
-                    // TODO: move syncer retries into the PeerSet,
-                    //       so we always choose different peers (#3235)
+                    // # Security
+                    //
+                    // We want to choose different peers for retries, so we have a better chance of getting each block.
+                    // This is implemented by the connection state machine sending synthetic `notfound`s to the
+                    // `InventoryRegistry`, as well as forwarding actual `notfound`s from peers.
                     Box::pin(tokio::task::yield_now().map(move |()| retry_outcome)),
                 )
             } else {

diff --git a/zebra-network/src/protocol/external/codec.rs b/zebra-network/src/protocol/external/codec.rs
@@ -184,10 +184,10 @@ impl Codec {
     /// Obtain the size of the body of a given message. This will match the
     /// number of bytes written to the writer provided to `write_body` for the
     /// same message.
-    ///
-    /// TODO: Replace with a size estimate, to avoid multiple serializations
-    /// for large data structures like lists, blocks, and transactions.
-    /// See #1774.
+    // # Performance TODO
+    //
+    // If this code shows up in profiles, replace with a size estimate or cached size,
+    // to avoid multiple serializations for large data structures like lists, blocks, and transactions.
     fn body_length(&self, msg: &Message) -> usize {
         let mut writer = FakeWriter(0);
 

diff --git a/zebra-network/src/protocol/external/message.rs b/zebra-network/src/protocol/external/message.rs
@@ -401,7 +401,7 @@ impl TryFrom<Message> for VersionMessage {
     }
 }
 
-// TODO: add tests for Error conversion and Reject message serialization (#4633)
+// TODO: add tests for Error conversion and Reject message serialization
 // (Zebra does not currently send reject messages, and it ignores received reject messages.)
 impl<E> From<E> for Message
 where

diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs
@@ -110,8 +110,7 @@ pub trait GetBlockTemplateRpc {
     /// - the parent block is a valid block that Zebra already has, or will receive soon.
     ///
     /// Zebra verifies blocks in parallel, and keeps recent chains in parallel,
-    /// so moving between chains is very cheap. (But forking a new chain may take some time,
-    /// until bug #4794 is fixed.)
+    /// so moving between chains and forking chains is very cheap.
     ///
     /// This rpc method is available only if zebra is built with `--features getblocktemplate-rpcs`.
     #[rpc(name = "getblocktemplate")]

diff --git a/zebra-state/src/config.rs b/zebra-state/src/config.rs
@@ -29,10 +29,7 @@ pub struct Config {
     /// When Zebra's state format changes, it creates a new state subdirectory for that version,
     /// and re-syncs from genesis.
     ///
-    /// Old state versions are [not automatically deleted](https://github.com/ZcashFoundation/zebra/issues/1213).
-    /// It is ok to manually delete old state versions.
-    ///
-    /// It is also ok to delete the entire cached state directory.
+    /// It is ok to delete the entire cached state directory.
     /// If you do, Zebra will re-sync from genesis next time it is launched.
     ///
     /// The default directory is platform dependent, based on

diff --git a/zebra-state/src/service/check/utxo.rs b/zebra-state/src/service/check/utxo.rs
@@ -229,7 +229,6 @@ pub fn remaining_transaction_value(
     utxos: &HashMap<transparent::OutPoint, transparent::OrderedUtxo>,
 ) -> Result<(), ValidateContextError> {
     for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() {
-        // TODO: check coinbase transaction remaining value (#338, #1162)
         if transaction.is_coinbase() {
             continue;
         }

diff --git a/zebra-state/src/service/finalized_state/tests/prop.rs b/zebra-state/src/service/finalized_state/tests/prop.rs
@@ -35,7 +35,6 @@ fn blocks_with_v5_transactions() -> Result<()> {
                 );
                 prop_assert_eq!(Some(height), state.finalized_tip_height());
                 prop_assert_eq!(hash.unwrap(), block.hash);
-                // TODO: check that the nullifiers were correctly inserted (#2230)
                 height = Height(height.0 + 1);
             }
     });

diff --git a/zebra-state/src/service/finalized_state/zebra_db/block/tests/snapshot.rs b/zebra-state/src/service/finalized_state/zebra_db/block/tests/snapshot.rs
@@ -253,7 +253,6 @@ fn snapshot_block_and_transaction_data(state: &FinalizedState) {
         //       test the rest of the chain data (value balance).
         let history_tree_at_tip = state.history_tree();
 
-        // TODO: split out block snapshots into their own function (#3151)
         for query_height in 0..=max_height.0 {
             let query_height = Height(query_height);