From 90f6b54be75439cadd98214f38db9c2c3d0495c0 Mon Sep 17 00:00:00 2001 From: Tino Breddin Date: Fri, 22 Sep 2023 15:31:18 +0200 Subject: [PATCH] core: Don't use destination in path finding (#5537) --- .../crates/core-packet/src/interaction.rs | 4 +- .../core/crates/core-packet/src/packet.rs | 2 +- .../core/crates/core-types/src/channels.rs | 5 +- packages/core/src/path/index.ts | 71 ++++++++++++++----- .../v3/paths/channels/{channelid}/index.ts | 2 +- scripts/api.sh | 27 +++++-- tests/integration-test.sh | 8 ++- 7 files changed, 88 insertions(+), 31 deletions(-) diff --git a/packages/core/crates/core-packet/src/interaction.rs b/packages/core/crates/core-packet/src/interaction.rs index 265b6296519..6f7b3b836ba 100644 --- a/packages/core/crates/core-packet/src/interaction.rs +++ b/packages/core/crates/core-packet/src/interaction.rs @@ -554,7 +554,7 @@ where // Decide whether to create 0-hop or multihop ticket let next_ticket = if path.length() == 1 { - Ticket::new_zero_hop(&next_peer, &self.cfg.chain_keypair, &domain_separator) + Ticket::new_zero_hop(&next_peer, &self.cfg.chain_keypair, &domain_separator)? } else { self.create_multihop_ticket(next_peer, path.length() as u8).await? }; @@ -737,7 +737,7 @@ where // Create next ticket for the packet next_ticket = if ticket_path_pos == 1 { - Ticket::new_zero_hop(&next_hop_addr, &self.cfg.chain_keypair, &domain_separator) + Ticket::new_zero_hop(&next_hop_addr, &self.cfg.chain_keypair, &domain_separator)? } else { self.create_multihop_ticket(next_hop_addr, ticket_path_pos).await? }; diff --git a/packages/core/crates/core-packet/src/packet.rs b/packages/core/crates/core-packet/src/packet.rs index 880667a22e8..1acaa480a5c 100644 --- a/packages/core/crates/core-packet/src/packet.rs +++ b/packages/core/crates/core-packet/src/packet.rs @@ -567,7 +567,7 @@ mod tests { ) .unwrap() } else { - Ticket::new_zero_hop(&next_peer_channel_key.to_address(), private_key, &Hash::default()) + Ticket::new_zero_hop(&next_peer_channel_key.to_address(), private_key, &Hash::default()).unwrap() } } diff --git a/packages/core/crates/core-types/src/channels.rs b/packages/core/crates/core-types/src/channels.rs index 32dee925234..216f3154332 100644 --- a/packages/core/crates/core-types/src/channels.rs +++ b/packages/core/crates/core-types/src/channels.rs @@ -568,7 +568,7 @@ impl Ticket { } /// Convenience method for creating a zero-hop ticket - pub fn new_zero_hop(destination: &Address, private_key: &ChainKeypair, domain_separator: &Hash) -> Self { + pub fn new_zero_hop(destination: &Address, private_key: &ChainKeypair, domain_separator: &Hash) -> Result { Self::new( destination, &Balance::new(0u32.into(), BalanceType::HOPR), @@ -580,7 +580,6 @@ impl Ticket { private_key, domain_separator, ) - .expect("Failed to create zero-hop ticket") } /// Based on the price of this ticket, determines the path position (hop number) this ticket @@ -965,7 +964,7 @@ pub mod tests { #[test] pub fn test_zero_hop() { - let zero_hop_ticket = Ticket::new_zero_hop(&BOB.public().to_address(), &ALICE, &Hash::default()); + let zero_hop_ticket = Ticket::new_zero_hop(&BOB.public().to_address(), &ALICE, &Hash::default()).unwrap(); assert!(zero_hop_ticket .verify(&ALICE.public().to_address(), &Hash::default()) .is_ok()); diff --git a/packages/core/src/path/index.ts b/packages/core/src/path/index.ts index a6ebc009b03..5293bf55458 100644 --- a/packages/core/src/path/index.ts +++ b/packages/core/src/path/index.ts @@ -1,9 +1,11 @@ import HeapPackage from 'heap-js' +import BN from 'bn.js' + +import { debug, random_float, ChannelEntry } from '@hoprnet/hopr-utils' + import { NETWORK_QUALITY_THRESHOLD, MAX_PATH_ITERATIONS, PATH_RANDOMNESS, MAX_HOPS } from '../constants.js' -import { type ChannelEntry, type Address } from '@hoprnet/hopr-utils' -import { debug, random_float } from '@hoprnet/hopr-utils' -import BN from 'bn.js' +import type { Address } from '@hoprnet/hopr-utils' const { Heap } = HeapPackage @@ -14,7 +16,12 @@ type ChannelPath = { weight: BN; path: ChannelEntry[] } const sum = (a: BN, b: BN) => a.add(b) const pathFrom = (c: ChannelPath): Path => c.path.map((ce) => ce.destination) // Doesn't include ourself [0] -const filterCycles = (c: ChannelEntry, p: ChannelPath): boolean => !pathFrom(p).find((x) => x.eq(c.destination)) +const filterCycles = (c: ChannelEntry, p: ChannelPath): boolean => { + if (p) { + return !pathFrom(p).find((x) => x.eq(c.destination)) + } + return true +} const debugPath = (p: ChannelPath) => pathFrom(p) .map((x) => x.toString()) @@ -28,6 +35,30 @@ const defaultWeight = async (edge: ChannelEntry): Promise => { return new BN(edge.balance.to_string(), 10).addn(1).muln(r) //log() } +// Filter given channels by a set of criteria to get good paths. +async function filterChannels( + channels: ChannelEntry[], + destination: Address, + currentPath: ChannelPath, + deadEnds: Set, + networkQualityOf: (p: Address) => Promise +): Promise { + return ( + await Promise.all( + channels.map(async (c): Promise<[boolean, ChannelEntry]> => { + const valid = + !destination.eq(c.destination) && + (await networkQualityOf(c.destination)) > NETWORK_QUALITY_THRESHOLD && + filterCycles(c, currentPath) && + !deadEnds.has(c.destination.to_hex()) + return [valid, c] + }) + ) + ) + .filter(([v, _c]) => v) + .map(([_v, c]) => c) +} + /** * Find a path through the payment channels. * @@ -55,12 +86,19 @@ export async function findPath( let queue = new Heap(comparePath) let deadEnds = new Set() + let currentPath: ChannelPath = undefined let iterations = 0 - let initialChannels = await getOpenChannelsFromPeer(start) + let initialChannels = await filterChannels( + await getOpenChannelsFromPeer(start), + destination, + currentPath, + deadEnds, + networkQualityOf + ) await Promise.all(initialChannels.map(async (x) => queue.add({ weight: await weight(x), path: [x] }))) while (queue.length > 0 && iterations++ < MAX_PATH_ITERATIONS) { - const currentPath: ChannelPath = queue.peek() + currentPath = queue.peek() const currPathLen = pathFrom(currentPath).length if (currPathLen >= hops && currPathLen <= MAX_HOPS) { log('Path of correct length found', debugPath(currentPath), ':', currentPath.weight.toString()) @@ -70,18 +108,15 @@ export async function findPath( const lastPeer = currentPath.path[currentPath.path.length - 1].destination const openChannels = await getOpenChannelsFromPeer(lastPeer) - const newChannels = [] - - for (const openChannel of openChannels) { - if ( - !destination.eq(openChannel.destination) && - (await networkQualityOf(openChannel.destination)) > NETWORK_QUALITY_THRESHOLD && - filterCycles(openChannel, currentPath) && - !deadEnds.has(openChannel.destination.to_hex()) - ) { - newChannels.push(openChannel) - } - } + const newChannels: ChannelEntry[] = [] + const usefuleOpenChannels: ChannelEntry[] = await filterChannels( + openChannels, + destination, + currentPath, + deadEnds, + networkQualityOf + ) + usefuleOpenChannels.forEach((c) => newChannels.push(c)) if (newChannels.length == 0) { queue.pop() diff --git a/packages/hoprd/src/api/v3/paths/channels/{channelid}/index.ts b/packages/hoprd/src/api/v3/paths/channels/{channelid}/index.ts index abf9c655000..6b52313f8d1 100644 --- a/packages/hoprd/src/api/v3/paths/channels/{channelid}/index.ts +++ b/packages/hoprd/src/api/v3/paths/channels/{channelid}/index.ts @@ -194,7 +194,7 @@ const GET: Operation = [ if (!channel) { return res.status(404).send() } - const response = formatChannelTopologyInfo(node, channel) + const response = await formatChannelTopologyInfo(node, channel) return res.status(200).send(response) } catch (err) { log(`${err}`) diff --git a/scripts/api.sh b/scripts/api.sh index 22cc5555108..85f5be978a2 100755 --- a/scripts/api.sh +++ b/scripts/api.sh @@ -67,7 +67,7 @@ api_call(){ else if [[ ${end_time_ns} -lt ${now} ]]; then log "${RED}attempt: ${attempt} - api_call (${cmd} \"${request_body}\") FAILED, received: ${result} but expected ${assertion}${NOFORMAT}" - exit 1 + return 1 else log "${YELLOW}attempt: ${attempt} - api_call (${cmd} \"${request_body}\") FAILED, received: ${result} but expected ${assertion}, retrying in ${step_time} seconds${NOFORMAT}" fi @@ -298,6 +298,25 @@ api_close_channel() { log "Node ${source_id} close channel ${channel_id} to Node ${destination_id} result -- ${result}" } +# $1 = source node id +# $2 = channel id +# $3 = destination address +# $4 = direction +api_get_channel_info() { + local source_api="${1}" + local channel_id="${2}" + local destination_address="${3}" + local direction="${4}" + + if [ -Z "${channel_id}" ] || [ "${channel_id}" = "null" ]; then + # fetch channel id from API + channels_info="$(api_get_all_channels "${source_api}" false)" + channel_id="$(echo "${channels_info}" | jq -r ".${direction}| map(select(.peerAddress | contains(\"${destination_address}\")))[0].id")" + fi + + api_call "${source_api}" "/channels/${channel_id}" "GET" "" 'channelId' 60 20 +} + # $1 = source node id # $2 = destination node id # $3 = channel source api endpoint @@ -311,7 +330,7 @@ api_open_channel() { local amount="${5:-1000000000000000000000}" local result balances hopr_balance - balances=$(api_get_balances ${api1}) + balances=$(api_get_balances ${source_api}) hopr_balance=$(echo ${balances} | jq -r .safeHopr) log "Safe balance of node ${source_api} before opening new channel: ${hopr_balance} weiHOPR, need ${amount} weiHOPR" @@ -334,11 +353,11 @@ api_validate_balances_gt0() { if [[ "$eth_balance" = "0" && "${safe_hopr_balance}" != "0" ]]; then log "Error: $1 Node has an invalid native balance: $eth_balance" - exit 1 + return 1 fi if [[ "$safe_hopr_balance" = "0" ]]; then log "Error: $1 Node Safe has an invalid HOPR balance: $safe_hopr_balance" - exit 1 + return 1 fi log "$1 is funded" } diff --git a/tests/integration-test.sh b/tests/integration-test.sh index 906c1e6d93e..1e482cf0d43 100755 --- a/tests/integration-test.sh +++ b/tests/integration-test.sh @@ -396,7 +396,9 @@ test_aggregate_redeem_in_specific_channel() { channel_info=$(api_open_channel "${node_id}" "${second_node_id}" "${node_api}" "${second_node_addr}") channel_id=$(echo "${channel_info}" | jq -r '.channelId') - log "Aggregate/Redeem in channel: Opened channel from node ${node_id} to ${second_node_id}: ${channel_id}" + channel_info_detail=$(api_get_channel_info "${node_api}" "${channel_id}" "${second_node_addr}" "outgoing") + channel_id=$(echo "${channel_info_detail}" | jq -r '.channelId') + log "Aggregate/Redeem in channel: Opened channel ${channel_id} from node ${node_id} to ${second_node_id}: ${channel_info_detail}" second_peer_id=$(get_hopr_address ${api_token}@${second_node_api}) @@ -444,7 +446,9 @@ test_redeem_in_specific_channel() { channel_info=$(api_open_channel "${node_id}" "${second_node_id}" "${node_api}" "${second_node_addr}") channel_id=$(echo "${channel_info}" | jq -r '.channelId') - log "Redeem in channel: Opened channel from node ${node_id} to ${second_node_id}: ${channel_id}" + channel_info_detail=$(api_get_channel_info "${node_api}" "${channel_id}" "${second_node_addr}" "outgoing") + channel_id=$(echo "${channel_info_detail}" | jq -r '.channelId') + log "Redeem in channel: Opened channel ${channel_id} from node ${node_id} to ${second_node_id}: ${channel_info_detail}" second_peer_id=$(get_hopr_address ${api_token}@${second_node_api})