Skip to content

Doc and build warning fixes #2068

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

Merged
merged 3 commits into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion fuzz/src/full_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use lightning::ln::msgs::{self, DecodeError};
use lightning::ln::script::ShutdownScript;
use lightning::routing::gossip::{P2PGossipSync, NetworkGraph};
use lightning::routing::utxo::UtxoLookup;
use lightning::routing::router::{find_route, InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, Router};
use lightning::routing::router::{find_route, InFlightHtlcs, PaymentParameters, Route, RouteParameters, Router};
use lightning::routing::scoring::FixedPenaltyScorer;
use lightning::util::config::UserConfig;
use lightning::util::errors::APIError;
Expand Down
4 changes: 3 additions & 1 deletion lightning-background-processor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ use lightning::routing::gossip::{NetworkGraph, P2PGossipSync};
use lightning::routing::utxo::UtxoLookup;
use lightning::routing::router::Router;
use lightning::routing::scoring::{Score, WriteableScore};
use lightning::util::events::{Event, EventHandler, EventsProvider, PathFailure};
use lightning::util::events::{Event, PathFailure};
#[cfg(feature = "std")]
use lightning::util::events::{EventHandler, EventsProvider};
use lightning::util::logger::Logger;
use lightning::util::persist::Persister;
use lightning_rapid_gossip_sync::RapidGossipSync;
Expand Down
3 changes: 3 additions & 0 deletions lightning-block-sync/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ impl BlockSourceError {
}

/// Converts the error into the underlying error.
///
/// May contain an [`std::io::Error`] from the [`BlockSource`]. See implementations for further
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesnt seem super helpful, it doesn't mention that the block source may well be returning an RpcError, which I think is the intent behind it. More generally why "may", isn't it does? Maybe we just leave this out and fix the error handling in a followup to just have enums and return proper errors rather than making users downcast several times.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RpcError is specific to RpcClient. Other BlockSources don't use it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but that isn't documented anywhere in RpcClient, which seemed like part of the reason for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's on RpcClient::call_method. Let me put it up on the struct docs, too, and explicitly tie it to BlockSource.

/// details, if any.
pub fn into_inner(self) -> Box<dyn std::error::Error + Send + Sync> {
self.error
}
Expand Down
6 changes: 6 additions & 0 deletions lightning-block-sync/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ impl fmt::Display for RpcError {
impl Error for RpcError {}

/// A simple RPC client for calling methods using HTTP `POST`.
///
/// Implements [`BlockSource`] and may return an `Err` containing [`RpcError`]. See
/// [`RpcClient::call_method`] for details.
pub struct RpcClient {
basic_auth: String,
endpoint: HttpEndpoint,
Expand All @@ -57,6 +60,9 @@ impl RpcClient {
}

/// Calls a method with the response encoded in JSON format and interpreted as type `T`.
///
/// When an `Err` is returned, [`std::io::Error::into_inner`] may contain an [`RpcError`] if
/// [`std::io::Error::kind`] is [`std::io::ErrorKind::Other`].
pub async fn call_method<T>(&self, method: &str, params: &[serde_json::Value]) -> std::io::Result<T>
where JsonResponse: TryFrom<Vec<u8>, Error = std::io::Error> + TryInto<T, Error = std::io::Error> {
let host = format!("{}:{}", self.endpoint.host(), self.endpoint.port());
Expand Down
2 changes: 1 addition & 1 deletion lightning-rapid-gossip-sync/src/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
&self,
read_cursor: &mut R,
) -> Result<u32, GraphSyncError> {
#[allow(unused_mut)]
#[allow(unused_mut, unused_assignments)]
let mut current_time_unix = None;
#[cfg(all(feature = "std", not(test)))]
{
Expand Down
12 changes: 4 additions & 8 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@
//! responsible for tracking which channels are open, HTLCs are in flight and reestablishing those
//! upon reconnect to the relevant peer(s).
//!
//! It does not manage routing logic (see [`find_route`] for that) nor does it manage constructing
//! It does not manage routing logic (see [`Router`] for that) nor does it manage constructing
//! on-chain transactions (it only monitors the chain to watch for any force-closes that might
//! imply it needs to fail HTLCs/payments/channels it manages).
//!
//! [`find_route`]: crate::routing::router::find_route

use bitcoin::blockdata::block::BlockHeader;
use bitcoin::blockdata::transaction::Transaction;
Expand Down Expand Up @@ -1745,14 +1743,12 @@ where
self.list_channels_with_filter(|_| true)
}

/// Gets the list of usable channels, in random order. Useful as an argument to [`find_route`]
/// to ensure non-announced channels are used.
/// Gets the list of usable channels, in random order. Useful as an argument to
/// [`Router::find_route`] to ensure non-announced channels are used.
///
/// These are guaranteed to have their [`ChannelDetails::is_usable`] value set to true, see the
/// documentation for [`ChannelDetails::is_usable`] for more info on exactly what the criteria
/// are.
///
/// [`find_route`]: crate::routing::router::find_route
pub fn list_usable_channels(&self) -> Vec<ChannelDetails> {
// Note we use is_live here instead of usable which leads to somewhat confused
// internal/external nomenclature, but that's ok cause that's probably what the user
Expand Down Expand Up @@ -3986,7 +3982,7 @@ where
None => None
};

let mut peer_state_opt = counterparty_node_id_opt.as_ref().map(
let peer_state_opt = counterparty_node_id_opt.as_ref().map(
|counterparty_node_id| per_peer_state.get(counterparty_node_id).map(
|peer_mutex| peer_mutex.lock().unwrap()
)
Expand Down
11 changes: 9 additions & 2 deletions lightning/src/ln/outbound_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,11 @@ pub(crate) struct PaymentAttemptsUsingTime<T: Time> {
/// it means the result of the first attempt is not known yet.
pub(crate) count: usize,
/// This field is only used when retry is `Retry::Timeout` which is only build with feature std
first_attempted_at: T
#[cfg(not(feature = "no-std"))]
first_attempted_at: T,
#[cfg(feature = "no-std")]
phantom: core::marker::PhantomData<T>,

}

#[cfg(not(any(feature = "no-std", test)))]
Expand All @@ -290,7 +294,10 @@ impl<T: Time> PaymentAttemptsUsingTime<T> {
pub(crate) fn new() -> Self {
PaymentAttemptsUsingTime {
count: 0,
first_attempted_at: T::now()
#[cfg(not(feature = "no-std"))]
first_attempted_at: T::now(),
#[cfg(feature = "no-std")]
phantom: core::marker::PhantomData,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/sync/debug_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl LockMetadata {
// For each lock which is currently locked, check that no lock's locked-before
// set includes the lock we're about to lock, which would imply a lockorder
// inversion.
for (locked_idx, locked) in held.borrow().iter() {
for (locked_idx, _locked) in held.borrow().iter() {
if *locked_idx == this.lock_idx {
// Note that with `feature = "backtrace"` set, we may be looking at different
// instances of the same lock. Still, doing so is quite risky, a total order
Expand All @@ -143,7 +143,7 @@ impl LockMetadata {
panic!("Tried to acquire a lock while it was held!");
}
}
for (locked_idx, locked) in held.borrow().iter() {
for (_locked_idx, locked) in held.borrow().iter() {
for (locked_dep_idx, _locked_dep) in locked.locked_before.lock().unwrap().iter() {
if *locked_dep_idx == this.lock_idx && *locked_dep_idx != locked.lock_idx {
#[cfg(feature = "backtrace")]
Expand Down
1 change: 1 addition & 0 deletions lightning/src/sync/fairrwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ impl<T> FairRwLock<T> {
self.lock.read()
}

#[allow(dead_code)]
pub fn try_write(&self) -> TryLockResult<RwLockWriteGuard<'_, T>> {
self.lock.try_write()
}
Expand Down
1 change: 0 additions & 1 deletion lightning/src/sync/test_lockorder_checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::sync::debug_sync::{RwLock, Mutex};
use super::{LockHeldState, LockTestExt};

use std::sync::Arc;
use std::thread;

#[test]
#[should_panic]
Expand Down