Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete outdated TODOs refering to closed issues #6732

Merged
merged 24 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c164cd5
ZIPs were updated to remove ambiguity, this was tracked in #1267.
mpguerra May 19, 2023
9f8161d
#2105 was fixed by #3039 and #2379 was closed by #3069
mpguerra May 19, 2023
39f86f7
#2230 was a duplicate of #2231 which was closed by #2511
mpguerra May 19, 2023
2e362d9
#3235 was obsoleted by #2156 which was fixed by #3505
mpguerra May 19, 2023
9b67e09
#1850 was fixed by #2944, #1851 was fixed by #2961 and #2902 was fixe…
mpguerra May 19, 2023
8ba4367
We migrated to Rust 2021 edition in Jan 2022 with #3332
mpguerra May 19, 2023
725283b
#1631 was closed as not needed
mpguerra May 19, 2023
c141645
#338 was fixed by #3040 and #1162 was fixed by #3067
mpguerra May 19, 2023
2be0331
#2079 was fixed by #2445
mpguerra May 19, 2023
778cce7
#4794 was fixed by #6122
mpguerra May 19, 2023
f1e04dc
#1678 stopped being an issue
mpguerra May 19, 2023
3bdf2cf
#3151 was fixed by #3934
mpguerra May 19, 2023
af3a487
#3204 was closed as not needed
mpguerra May 19, 2023
d3456b8
#1213 was fixed by #4586
mpguerra May 19, 2023
6ed7bca
#1774 was closed as not needed
mpguerra May 19, 2023
b56f858
#4633 was closed as not needed
mpguerra May 19, 2023
da6b1f5
Clarify behaviour of difficulty spacing
mpguerra May 22, 2023
ea15a48
Update comment to reflect implemented behaviour
mpguerra May 22, 2023
d98f55d
Update comment to reflect implemented behaviour when retrying block d…
mpguerra May 22, 2023
14899f3
Update `TODO` to remove closed issue and clarify when we might want t…
mpguerra May 22, 2023
23fdc81
Update `TODO` to remove closed issue and clarify what we might want t…
mpguerra May 22, 2023
7c5d6b6
Clarify benefits of how we do block verification
mpguerra May 22, 2023
f4dcdc0
Fix rustfmt errors
teor2345 May 23, 2023
32d37c7
Merge branch 'main' into pili-6281
teor2345 May 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions zebra-chain/src/orchard/sinsemilla.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
4 changes: 3 additions & 1 deletion zebra-chain/src/work/difficulty/tests/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
4 changes: 0 additions & 4 deletions zebra-consensus/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
1 change: 0 additions & 1 deletion zebra-network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
mpguerra marked this conversation as resolved.
Show resolved Hide resolved
let listen_addr = match config.listen_addr.parse::<SocketAddr>() {
Ok(socket) => Ok(socket),
Err(_) => match config.listen_addr.parse::<IpAddr>() {
Expand Down
3 changes: 2 additions & 1 deletion zebra-network/src/peer_set/limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
mpguerra marked this conversation as resolved.
Show resolved Hide resolved
}
}
2 changes: 0 additions & 2 deletions zebra-network/src/peer_set/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 0 additions & 2 deletions zebra-network/src/peer_set/unready_service/tests/vectors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
//! Fixed test vectors for unready services.
//!
//! TODO: test that inner service errors are handled correctly (#3204)

use std::marker::PhantomData;

Expand Down
7 changes: 5 additions & 2 deletions zebra-network/src/policies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
//
// 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)),
mpguerra marked this conversation as resolved.
Show resolved Hide resolved
)
} else {
Expand Down
8 changes: 4 additions & 4 deletions zebra-network/src/protocol/external/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
mpguerra marked this conversation as resolved.
Show resolved Hide resolved
let mut writer = FakeWriter(0);

Expand Down
2 changes: 1 addition & 1 deletion zebra-network/src/protocol/external/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
mpguerra marked this conversation as resolved.
Show resolved Hide resolved
where
Expand Down
3 changes: 1 addition & 2 deletions zebra-rpc/src/methods/get_block_template_rpcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
5 changes: 1 addition & 4 deletions zebra-state/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion zebra-state/src/service/check/utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 0 additions & 1 deletion zebra-state/src/service/finalized_state/tests/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down