Skip to content

Commit

Permalink
Fix peer enforcement when payment issues are ongoing
Browse files Browse the repository at this point in the history 
This is a pretty classic case of optimization causing problems. In order
to try and optimize tunnel handling on routers with many tunnels we
gated running the enforcmenet update code so that it was only run when
there was an actual change to execute.

At a later date we came along and added a check there to prevent running
enforcement code if we where having full node issues, but the other case
was never considered. What if someone needed to be unenforced while
payment issues where occuring?

This patch fixes the issue by removing some of the optimization and
checking over enforcement more consistently to avoid users getting stuck
in one wrong state or othe other.

I also cleaned up some old actix artifacts.
  • Loading branch information
@jkilpatr
jkilpatr committed Jan 2, 2024
1 parent b2c33f9 commit 4d5581d
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 67 deletions.
7 changes: 1 addition & 6 deletions rita_common/src/debt_keeper/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ use crate::simulated_txfee_manager::add_tx_to_total;
use crate::tunnel_manager::tm_tunnel_state_change;
use crate::tunnel_manager::TunnelAction;
use crate::tunnel_manager::TunnelChange;
use crate::tunnel_manager::TunnelStateChange;
use crate::RitaCommonError;
use crate::KI;

use althea_types::Denom;
use althea_types::Identity;
use althea_types::SystemChain;
Expand All @@ -32,7 +30,6 @@ use num_traits::Signed;
use settings::get_rita_common;
use settings::DEBT_KEEPER_DENOM;
use settings::DEBT_KEEPER_DENOM_DECIMAL;

use std::collections::HashMap;
use std::fs;
use std::fs::File;
Expand Down Expand Up @@ -357,9 +354,7 @@ pub fn send_debt_update() -> Result<(), RitaCommonError> {
}
}

if let Err(e) = tm_tunnel_state_change(TunnelStateChange {
tunnels: debts_message,
}) {
if let Err(e) = tm_tunnel_state_change(debts_message) {
warn!("Error during tunnel state change: {}", e);
}
Ok(())
Expand Down
97 changes: 36 additions & 61 deletions rita_common/src/tunnel_manager/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,16 @@ pub fn tm_common_slow_loop_helper(babel_interfaces: Vec<Interface>) {
tunnel_manager.tunnel_gc(TUNNEL_TIMEOUT, TUNNEL_HANDSHAKE_TIMEOUT, babel_interfaces);
}

/// Called by DebtKeeper with the updated billing status of every tunnel every round
pub fn tm_tunnel_state_change(msg: Vec<TunnelChange>) -> Result<(), RitaCommonError> {
let tm_pin = &mut *TUNNEL_MANAGER.write().unwrap();
let tunnel_manager = get_tunnel_manager_write_ref(tm_pin);
for tunnel in msg {
tunnel_manager.tunnel_payment_state_change(tunnel);
}
Ok(())
}

impl TunnelManager {
pub fn new() -> Self {
TunnelManager {
Expand Down Expand Up @@ -500,7 +510,6 @@ impl TunnelManager {
id,
action,
);
let mut tunnel_bw_limits_need_change = false;

// Find a tunnel
match self.tunnels.get_mut(&id) {
Expand All @@ -520,7 +529,6 @@ impl TunnelManager {
tunnel.neigh_id.global.wg_public_key
);
tunnel.payment_state = PaymentState::Paid;
tunnel_bw_limits_need_change = true;
// latency detector probably got confused while enforcement
// occurred
tunnel.speed_limit = None;
Expand All @@ -536,7 +544,6 @@ impl TunnelManager {
tunnel.neigh_id.global.wg_public_key
);
tunnel.payment_state = PaymentState::Overdue;
tunnel_bw_limits_need_change = true;
}
PaymentState::Overdue => {
continue;
Expand All @@ -545,6 +552,13 @@ impl TunnelManager {
}
}
}
// update the bw limits if required, don't gate calling this function
// if payment issues are occuring it may fail to enforce, it must be called
// again later even if there are no changes to ensure everything is in a proper state
let res = tunnel_bw_limit_update(tunnels);
if res.is_err() {
error!("Bandwidth limiting failed with {:?}", res);
}
}
None => {
// This is now pretty common since there's no more none action
Expand All @@ -553,75 +567,36 @@ impl TunnelManager {
trace!("Couldn't find tunnel for identity {:?}", id);
}
}

// this is done outside of the match to make the borrow checker happy
if tunnel_bw_limits_need_change {
if potential_payment_issues_detected() {
warn!("Potential payment issue detected");
return;
}
let res = tunnel_bw_limit_update(&self.tunnels);
// if this fails consistently it could be a wallet draining attack
// TODO check for that case
if res.is_err() {
error!("Bandwidth limiting failed with {:?}", res);
}
}
}
}

/// A single use internal struct used to flag what tunnels need to be updated
pub struct TunnelChange {
pub identity: Identity,
pub action: TunnelAction,
}

pub struct TunnelStateChange {
pub tunnels: Vec<TunnelChange>,
}

/// Called by DebtKeeper with the updated billing status of every tunnel every round
pub fn tm_tunnel_state_change(msg: TunnelStateChange) -> Result<(), RitaCommonError> {
let tm_pin = &mut *TUNNEL_MANAGER.write().unwrap();
let tunnel_manager = get_tunnel_manager_write_ref(tm_pin);
for tunnel in msg.tunnels {
tunnel_manager.tunnel_payment_state_change(tunnel);
}
Ok(())
}

/// Takes the tunnels list and iterates over it to update all of the traffic control settings
/// since we can't figure out how to combine interfaces bandwidth budgets we're subdividing it
/// here with manual terminal commands whenever there is a change
fn tunnel_bw_limit_update(tunnels: &HashMap<Identity, Vec<Tunnel>>) -> Result<(), RitaCommonError> {
/// Takes a vec of tunnels and then updates the bandwidth limits on them. The calling functions
/// ensure that this is only done when required. We further optimize by checking the qdisc before
/// performing the update here
fn tunnel_bw_limit_update(tunnels: &[Tunnel]) -> Result<(), RitaCommonError> {
info!("Running tunnel bw limit update!");
// number of interfaces over which we will have to divide free tier BW
let mut limited_interfaces = 0u16;
for sublist in tunnels.iter() {
for tunnel in sublist.1.iter() {
if tunnel.payment_state == PaymentState::Overdue {
limited_interfaces += 1;
}
}
}

let payment = settings::get_rita_common().payment;
let bw_per_iface = if limited_interfaces > 0 {
payment.free_tier_throughput / u32::from(limited_interfaces)
} else {
payment.free_tier_throughput
};

for sublist in tunnels.iter() {
for tunnel in sublist.1.iter() {
let payment_state = &tunnel.payment_state;
let iface_name = &tunnel.iface_name;
let has_limit = KI.has_limit(iface_name)?;

if *payment_state == PaymentState::Overdue {
KI.set_classless_limit(iface_name, bw_per_iface)?;
} else if *payment_state == PaymentState::Paid && has_limit {
KI.set_codel_shaping(iface_name, None)?;
}
let bw_per_iface = payment.free_tier_throughput;

for tunnel in tunnels {
let payment_state = &tunnel.payment_state;
let iface_name = &tunnel.iface_name;
let has_limit = KI.has_limit(iface_name)?;

if *payment_state == PaymentState::Overdue
&& !has_limit
&& !potential_payment_issues_detected()
{
KI.set_classless_limit(iface_name, bw_per_iface)?;
} else if *payment_state == PaymentState::Paid && has_limit {
KI.set_codel_shaping(iface_name, None)?;
}
}
Ok(())
Expand Down

0 comments on commit 4d5581d

Please sign in to comment.