chanmgr: Break out padding_parameters fn

Move some logic out of reconfigure_general into what was
update_padding_parameters_from_netdir, and rename that function.

We're going to want to call this twice, shortly...

* Move out the PaddingParametersBuilder
* Have it handle missing netdir, though we currently always pass Ok
* Have it handle the error cases

It still ignores the config for now.

No overall functional change.

"git show -b" may be a useful way to review the changes in what
becomes "padding_parameters".

crates/tor-chanmgr/src/mgr/map.rs

@@ -8,8 +8,10 @@ use crate::{ChannelConfig, Dormancy, Error, Result};
 use std::collections::{hash_map, HashMap};
 use std::result::Result as StdResult;
 use std::sync::Arc;
+use tor_config::PaddingLevel;
 use tor_error::{internal, into_internal};
 use tor_netdir::{params::CHANNEL_PADDING_TIMEOUT_UPPER_BOUND, NetDir};
+use tor_proto::channel::padding::Parameters as PaddingParameters;
 use tor_proto::channel::padding::ParametersBuilder as PaddingParametersBuilder;
 use tor_proto::ChannelsParams;
 use tor_units::{BoundedInt32, IntegerMilliseconds};
@@ -333,6 +335,7 @@ impl<C: AbstractChannel> ChannelMap<C> {
 
         // TODO support dormant mode
         // TODO when entering/leaving dormant mode, send CELL_PADDING_NEGOTIATE to peers
+        // TODO with reduced padding, send CELL_PADDING_NEGOTIATE
 
         // TODO when we support operation as a relay, inter-relay channels ought
         // not to get padding.
@@ -356,20 +359,7 @@ impl<C: AbstractChannel> ChannelMap<C> {
             inner.dormancy = new_dormancy;
         }
 
-        let padding_parameters = {
+        let padding_parameters = padding_parameters(inner.config.padding, Ok(&netdir))?;
-            let mut p = PaddingParametersBuilder::default();
-            update_padding_parameters_from_netdir(&mut p, &netdir).unwrap_or_else(|e| {
-                info!(
-                    "consensus channel padding parameters wrong, using defaults: {}",
-                    &e,
-                );
-            });
-            let p = p
-                .build()
-                .map_err(into_internal!("failed to build padding parameters"))?;
-
-            p
-        };
 
         let update = inner
             .channels_params
@@ -409,28 +399,53 @@ impl<C: AbstractChannel> ChannelMap<C> {
     }
 }
 
-/// Given a `NetDir`, update a `PaddingParametersBuilder` with channel padding parameters
+/// Given a `NetDirExtract` and whether we're reducing padding, return a `PaddingParameters`
-fn update_padding_parameters_from_netdir(
+///
-    p: &mut PaddingParametersBuilder,
+/// With `PaddingLevel::None`, will return `PaddingParameters::all_zeroes`; but
-    netdir: &NetDirExtract,
+/// does not account for padding being enabled/disabled other ways than via the config.
-) -> StdResult<(), &'static str> {
+fn padding_parameters(
-    // TODO support reduced padding via global client config,
+    _config: PaddingLevel,
-    // TODO and with reduced padding, send CELL_PADDING_NEGOTIATE
+    netdir: StdResult<&NetDirExtract, &()>,
+) -> StdResult<PaddingParameters, tor_error::Bug> {
     let reduced = false; // TODO
-    let nf_ito = netdir.nf_ito[usize::from(reduced)];
 
-    let get_timing_param =
+    Ok(match netdir {
-        |index: usize| nf_ito[index].try_map(|bounded| bounded.get().try_into());
+        Ok(netdir) => {
-    let low = get_timing_param(0).map_err(|_| "low value arithmetic overflow?!")?;
+            let mut p = PaddingParametersBuilder::default();
-    let high = get_timing_param(1).map_err(|_| "high value arithmetic overflow?!")?;
+            let () = (|| {
+                let nf_ito = netdir.nf_ito[usize::from(reduced)];
+                let get_timing_param =
+                    |index: usize| nf_ito[index].try_map(|bounded| bounded.get().try_into());
+                let low = get_timing_param(0).map_err(|_| "low value arithmetic overflow?!")?;
+                let high = get_timing_param(1).map_err(|_| "high value arithmetic overflow?!")?;
 
-    if high > low {
+                if high > low {
-        return Err("high > low");
+                    return Err("high > low");
-    }
+                }
+                p.low_ms(low);
+                p.high_ms(high);
+                Ok::<_, &'static str>(())
+            })()
+            .unwrap_or_else(|e| {
+                info!(
+                    "consensus channel padding parameters wrong, using defaults: {}",
+                    &e
+                );
+            });
 
-    p.low_ms(low);
+            p.build()
-    p.high_ms(high);
+                .map_err(into_internal!("failed to build padding parameters"))?
-    Ok(())
+        }
+        Err(()) => {
+            // TODO we should use a fallback here so that config overrides take effect,
+            // as discussed in https://gitlab.torproject.org/tpo/core/arti/-/issues/528
+            if reduced {
+                PaddingParameters::default_reduced()
+            } else {
+                PaddingParameters::default()
+            }
+        }
+    })
 }
 
 #[cfg(test)]