Skip to content

GH-818 Are Chunked Responses Being Handled Wrong? #682

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.

Sign up for GitHub

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

GH-818 Are Chunked Responses Being Handled Wrong? #682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
fe2804d
Fixed the AddRouteResultMessage panic problem too
dnwiebe Aug 15, 2025
81333fe
Preemptive review issues
dnwiebe Aug 15, 2025
02d46c3
Formatting
dnwiebe Aug 15, 2025
442e63c
example.com -> www.example.com
dnwiebe Aug 18, 2025
ac4c096
Formatting
dnwiebe Aug 18, 2025
1234f5c
Added some debug logging to track RRI
dnwiebe Aug 26, 2025
70049a9
Formatting
dnwiebe Aug 26, 2025
30f832b
Clippy
dnwiebe Aug 26, 2025
b8c3894
Merge branch 'master' into GH-818
dnwiebe Aug 27, 2025
7c3fa52
Made RRI logs more consistent and increased straggler timeout to 30s
dnwiebe Aug 27, 2025
f9cab9a
Typo corrections
dnwiebe Aug 28, 2025
5a89e1a
Interim commit
dnwiebe Sep 1, 2025
8f774b6
Tests pass
dnwiebe Sep 6, 2025
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: 2 additions & 2 deletions node/src/hopper/live_cores_package.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,18 +302,18 @@ mod tests {
let encrypted_payload = encodex(cryptde, &first_stop_key, &payload).unwrap();
let paying_wallet = make_paying_wallet(b"wallet");
let contract_address = TEST_DEFAULT_CHAIN.rec().contract;
let mut route = Route::round_trip(
let route = Route::round_trip(
RouteSegment::new(vec![&relay_key, &first_stop_key], Component::Neighborhood),
RouteSegment::new(
vec![&first_stop_key, &relay_key, &second_stop_key],
Component::ProxyServer,
),
cryptde,
Some(paying_wallet.clone()),
1234,
Some(contract_address),
)
.unwrap();
let mut route = route.set_return_route_id(cryptde, 1234);
route.shift(&relay_cryptde).unwrap();
let subject = LiveCoresPackage::new(route.clone(), encrypted_payload.clone());

Expand Down
123 changes: 12 additions & 111 deletions node/src/neighborhood/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ pub struct Neighborhood {
mode: NeighborhoodModeLight,
min_hops: Hops,
db_patch_size: u8,
next_return_route_id: u32,
overall_connection_status: OverallConnectionStatus,
chain: Chain,
crashable: bool,
Expand Down Expand Up @@ -431,7 +430,6 @@ impl Neighborhood {
mode,
min_hops,
db_patch_size,
next_return_route_id: 0,
overall_connection_status,
chain: config.blockchain_bridge_config.chain,
crashable: config.crash_point == CrashPoint::Message,
Expand Down Expand Up @@ -980,7 +978,6 @@ impl Neighborhood {
}

fn zero_hop_route_response(&mut self) -> RouteQueryResponse {
let return_route_id = self.advance_return_route_id();
let route = Route::round_trip(
RouteSegment::new(
vec![self.cryptde.public_key(), self.cryptde.public_key()],
Expand All @@ -992,7 +989,6 @@ impl Neighborhood {
),
self.cryptde,
None,
return_route_id,
None,
)
.expect("Couldn't create route");
Expand All @@ -1001,7 +997,6 @@ impl Neighborhood {
expected_services: ExpectedServices::RoundTrip(
vec![ExpectedService::Nothing, ExpectedService::Nothing],
vec![ExpectedService::Nothing, ExpectedService::Nothing],
return_route_id,
),
}
}
Expand Down Expand Up @@ -1067,21 +1062,18 @@ impl Neighborhood {
Err(e) => return Err(e),
};

let return_route_id = self.advance_return_route_id();
Ok(RouteQueryResponse {
route: Route::round_trip(
over,
back,
self.cryptde,
self.consuming_wallet_opt.clone(),
return_route_id,
Some(self.chain.rec().contract),
)
.expect("Internal error: bad route"),
expected_services: ExpectedServices::RoundTrip(
expected_request_services,
expected_response_services,
return_route_id,
),
})
}
Expand Down Expand Up @@ -1323,12 +1315,6 @@ impl Neighborhood {
}
}

fn advance_return_route_id(&mut self) -> u32 {
let return_route_id = self.next_return_route_id;
self.next_return_route_id = return_route_id.wrapping_add(1);
return_route_id
}

pub fn find_exit_locations<'a>(
&'a self,
source: &'a PublicKey,
Expand Down Expand Up @@ -3382,7 +3368,6 @@ mod tests {
),
cryptde,
None,
0,
None,
)
.unwrap(),
Expand All @@ -3403,7 +3388,6 @@ mod tests {
),
ExpectedService::Nothing,
],
0,
),
};
assert_eq!(expected_response, result);
Expand Down Expand Up @@ -3455,39 +3439,17 @@ mod tests {
),
cryptde,
None,
0,
None,
)
.unwrap(),
expected_services: ExpectedServices::RoundTrip(
vec![ExpectedService::Nothing, ExpectedService::Nothing],
vec![ExpectedService::Nothing, ExpectedService::Nothing],
0,
),
};
assert_eq!(result, expected_response);
}

#[test]
fn zero_hop_routing_handles_return_route_id_properly() {
let mut subject = make_standard_subject();
let result0 = subject.zero_hop_route_response();
let result1 = subject.zero_hop_route_response();

let return_route_id_0 = match result0.expected_services {
ExpectedServices::RoundTrip(_, _, id) => id,
_ => panic!("expected RoundTrip got OneWay"),
};

let return_route_id_1 = match result1.expected_services {
ExpectedServices::RoundTrip(_, _, id) => id,
_ => panic!("expected RoundTrip got OneWay"),
};

assert_eq!(return_route_id_0, 0);
assert_eq!(return_route_id_1, 1);
}

/*
Database:

Expand Down Expand Up @@ -3546,7 +3508,6 @@ mod tests {
segment(&[r, q, p], &Component::ProxyServer),
cryptde,
consuming_wallet_opt,
0,
Some(contract_address),
)
.unwrap(),
Expand Down Expand Up @@ -3577,10 +3538,9 @@ mod tests {
),
ExpectedService::Nothing,
],
0,
),
};
assert_eq!(expected_response, result);
assert_eq!(result, expected_response);
}

#[test]
Expand All @@ -3599,18 +3559,6 @@ mod tests {
);
}

#[test]
fn next_return_route_id_wraps_around() {
let mut subject = make_standard_subject();
subject.next_return_route_id = 0xFFFFFFFF;

let end = subject.advance_return_route_id();
let beginning = subject.advance_return_route_id();

assert_eq!(end, 0xFFFFFFFF);
assert_eq!(beginning, 0x00000000);
}

/*
Database:

Expand All @@ -3619,37 +3567,6 @@ mod tests {
Tests will be written from the viewpoint of O.
*/

#[test]
fn return_route_ids_increase() {
let cryptde = main_cryptde();
let system = System::new("return_route_ids_increase");
let (_, _, _, mut subject) = make_o_r_e_subject();
subject.min_hops = Hops::TwoHops;
let addr: Addr<Neighborhood> = subject.start();
let sub: Recipient<RouteQueryMessage> = addr.recipient::<RouteQueryMessage>();

let data_route_0 = sub.send(RouteQueryMessage::data_indefinite_route_request(None, 2000));
let data_route_1 = sub.send(RouteQueryMessage::data_indefinite_route_request(None, 3000));

System::current().stop_with_code(0);
system.run();
let result_0 = data_route_0.wait().unwrap().unwrap();
let result_1 = data_route_1.wait().unwrap().unwrap();
let juicy_parts = |result: RouteQueryResponse| {
let last_element = result.route.hops.last().unwrap();
let last_element_dec = cryptde.decode(last_element).unwrap();
let network_return_route_id: u32 =
serde_cbor::de::from_slice(last_element_dec.as_slice()).unwrap();
let metadata_return_route_id = match result.expected_services {
ExpectedServices::RoundTrip(_, _, id) => id,
_ => panic!("expected RoundTrip got OneWay"),
};
(network_return_route_id, metadata_return_route_id)
};
assert_eq!(juicy_parts(result_0), (0, 0));
assert_eq!(juicy_parts(result_1), (1, 1));
}

#[test]
fn handle_neighborhood_graph_message_works() {
let test_name = "handle_neighborhood_graph_message_works";
Expand Down Expand Up @@ -4533,7 +4450,6 @@ mod tests {
error_expectation,
"Cannot make multi_hop with unknown neighbor"
);
assert_eq!(subject.next_return_route_id, 0);
}

#[test]
Expand Down Expand Up @@ -7017,7 +6933,7 @@ mod tests {

let hops = result.clone().unwrap().route.hops;
let actual_keys: Vec<PublicKey> = match hops.as_slice() {
[hop, exit, hop_back, origin, empty, _accounting] => vec![
[hop, exit, hop_back, origin, empty] => vec![
decodex::<LiveHop>(main_cryptde(), hop)
.expect("hop")
.public_key,
Expand All @@ -7034,7 +6950,11 @@ mod tests {
.expect("empty")
.public_key,
],
l => panic!("our match is wrong, real size is {}, {:?}", l.len(), l),
l => panic!(
"our match is wrong, real size is {} instead of 5, {:?}",
l.len(),
l
),
};
let expected_public_keys = vec![
next_door_neighbor.public_key().clone(),
Expand Down Expand Up @@ -7072,24 +6992,23 @@ mod tests {
}
};
/*
This is how the route_hops vector looks like: [C1, C2, ..., C(nodes_count), ..., C2, C1, accounting]
This is how the route_hops vector looks like: [C1, C2, ..., C(nodes_count), ..., C2, C1]

Let's consider for 3-hop route ==>
Nodes Count --> 4
Route Length --> 8
Route Hops --> [C1, C2, C3, C4, C3, C2, C1, accounting]
Route Hops --> [C1, C2, C3, C4, C3, C2, C1]
Over Route --> [C1, C2, C3]
Back Route --> [C4, C3, C2, C1]
*/
let mut route_hops = result.unwrap().route.hops;
let route_hops = result.unwrap().route.hops;
let route_length = route_hops.len();
let _accounting = route_hops.pop();
let over_route = &route_hops[..hops];
let back_route = &route_hops[hops..];
let over_cryptdes = cryptdes_from_node_records(&nodes[..hops]);
let mut back_cryptdes = cryptdes_from_node_records(&nodes);
back_cryptdes.reverse();
assert_eq!(route_length, 2 * nodes_count);
assert_eq!(route_length, 2 * nodes_count - 1);
assert_hops(over_cryptdes, over_route);
assert_hops(back_cryptdes, back_route);
}
Expand Down Expand Up @@ -7169,7 +7088,7 @@ mod tests {

let (over, back) = match response.expected_services {
ExpectedServices::OneWay(_) => panic!("Expecting RoundTrip"),
ExpectedServices::RoundTrip(o, b, _) => (o[1].clone(), b[1].clone()),
ExpectedServices::RoundTrip(o, b) => (o[1].clone(), b[1].clone()),
};
let extract_key = |es: ExpectedService| match es {
ExpectedService::Routing(pk, _, _) => pk,
Expand Down Expand Up @@ -7563,24 +7482,6 @@ mod tests {
subject
}

fn make_o_r_e_subject() -> (NodeRecord, NodeRecord, NodeRecord, Neighborhood) {
let mut subject = make_standard_subject();
let o = &subject.neighborhood_database.root().clone();
let r = &make_node_record(4567, false);
let e = &make_node_record(5678, false);
{
let db = &mut subject.neighborhood_database;
db.add_node(r.clone()).unwrap();
db.add_node(e.clone()).unwrap();
let mut dual_edge = |a: &NodeRecord, b: &NodeRecord| {
db.add_arbitrary_full_neighbor(a.public_key(), b.public_key())
};
dual_edge(o, r);
dual_edge(r, e);
}
(o.clone(), r.clone(), e.clone(), subject)
}

fn segment(nodes: &[&NodeRecord], component: &Component) -> RouteSegment {
RouteSegment::new(
nodes.into_iter().map(|n| n.public_key()).collect(),
Expand Down
Loading