From 31188fae7a0a3f553d2368ba4c67722e5c4f2be5 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 21 Aug 2023 09:28:53 -0700 Subject: [PATCH 01/13] send dc balance to charge for late packets --- src/apis/router_console_api.erl | 3 +- src/apis/router_console_dc_tracker.erl | 22 +++++++++++ src/device/router_device.erl | 7 +++- src/device/router_device_routing.erl | 52 +++++++++++++++++-------- src/device/router_device_worker.erl | 54 +++++++++++++++++++++++--- src/router_utils.erl | 36 ++++++++++++++--- 6 files changed, 145 insertions(+), 29 deletions(-) diff --git a/src/apis/router_console_api.erl b/src/apis/router_console_api.erl index 13ff4f881..3b0265ab1 100644 --- a/src/apis/router_console_api.erl +++ b/src/apis/router_console_api.erl @@ -324,7 +324,8 @@ event(Device, Map) -> #{ fcnt => maps:get(fcnt, Map), hotspot => maps:get(hotspot, Map), - hold_time => maps:get(hold_time, Map) + hold_time => maps:get(hold_time, Map), + dc => maps:get(dc, Map, #{}) }; {uplink_dropped, _SC} -> #{ diff --git a/src/apis/router_console_dc_tracker.erl b/src/apis/router_console_dc_tracker.erl index 2b1b2146d..311dec778 100644 --- a/src/apis/router_console_dc_tracker.erl +++ b/src/apis/router_console_dc_tracker.erl @@ -16,6 +16,7 @@ has_enough_dc/2, charge/2, current_balance/1, + maybe_charge_late/2, %% add_unfunded/1, remove_unfunded/1, @@ -177,6 +178,27 @@ current_balance(OrgID) -> {Balance, Nonce} end. +-spec maybe_charge_late( + Device :: router_device:device(), + Packet :: blockchain_helium_packet_v1:packet() +) -> {ok, non_neg_integer(), non_neg_integer()} | {error, any()}. +maybe_charge_late(Device, Packet) -> + OrgID = router_device:organization_id(Device), + case router_utils:get_env_bool(charge_late_packets, false) of + false -> + {Balance, Nonce} = current_balance(OrgID), + {ok, Balance, Nonce}; + true -> + Payload = blockchain_helium_packet_v1:payload(Packet), + PayloadSize = erlang:byte_size(Payload), + case charge(Device, PayloadSize) of + {error, _} = E -> + E; + Res -> + Res + end + end. + %% ------------------------------------------------------------------ %% gen_server Function Definitions %% ------------------------------------------------------------------ diff --git a/src/device/router_device.erl b/src/device/router_device.erl index 74b804076..ede2f5c67 100644 --- a/src/device/router_device.erl +++ b/src/device/router_device.erl @@ -50,7 +50,8 @@ credentials_to_evict/1, limit_credentials/1, devaddr_int_nwk_key/1, - make_skf_removes/1, make_skf_removes/2, make_skf_removes/3 + make_skf_removes/1, make_skf_removes/2, make_skf_removes/3, + organization_id/1 ]). %% ------------------------------------------------------------------ @@ -566,6 +567,10 @@ make_skf_removes(NwkKeys, DevAddrs, MultiBuy) -> lists:usort([{remove, DevAddrToInt(D), NSK, MultiBuy} || {NSK, _} <- NwkKeys, D <- DevAddrs]). +-spec organization_id(device()) -> undefined | binary(). +organization_id(#device_v7{metadata = Metadata}) -> + maps:get(organization_id, Metadata, undefined). + %% ------------------------------------------------------------------ %% RocksDB Device Functions %% ------------------------------------------------------------------ diff --git a/src/device/router_device_routing.erl b/src/device/router_device_routing.erl index 4f9c7396e..838b8b2a2 100644 --- a/src/device/router_device_routing.erl +++ b/src/device/router_device_routing.erl @@ -39,6 +39,12 @@ force_evict_packet_hash/1 ]). +-export([ + payload_b0/2, + payload_mic/1, + payload_fcnt_low/1 +]). + %% biggest unsigned number in 23 bits -define(BITS_23, 8388607). -define(MODULO_16_BITS, 16#10000). @@ -221,24 +227,15 @@ handle_free_packet(SCPacket, PacketTime, Pid) when is_pid(Pid) -> ok = lager:md([{device_id, router_device:id(Device)}]), case validate_payload_for_device(Device, Payload, PHash, PubKeyBin, PacketFCnt) of ok -> - Offer = blockchain_state_channel_offer_v1:from_packet( - Packet, PubKeyBin, Region - ), - case offer_check(Offer) of - ok -> - erlang:spawn(router_blockchain, track_offer, [Offer, Pid]), - {ok, Device}; - {error, _} -> - lager:debug("packet accepted even if on denylist"), - {ok, Device} - end; + {ok, Device}; {error, ?LATE_PACKET} = E2 -> ok = router_utils:event_uplink_dropped_late_packet( PacketTime, HoldTime, PacketFCnt, Device, - PubKeyBin + PubKeyBin, + Packet ), E2; {error, _} = E3 -> @@ -1187,7 +1184,7 @@ get_device_by_mic(_MIC, _Payload, []) -> undefined; get_device_by_mic(ExpectedMIC, Payload, [Device | Devices]) -> ExpectedFCnt = expected_fcnt(Device, Payload), - B0 = b0_from_payload(Payload, ExpectedFCnt), + B0 = payload_b0(Payload, ExpectedFCnt), DeviceID = router_device:id(Device), case router_device:nwk_s_key(Device) of undefined -> @@ -1235,10 +1232,31 @@ find_right_key(B0, MIC, Payload, Device, [{undefined, _} | Keys]) -> find_right_key(B0, MIC, Payload, Device, Keys); find_right_key(B0, MIC, Payload, Device, [{NwkSKey, _} | Keys]) -> case key_matches_mic(NwkSKey, B0, MIC) of - true -> {Device, NwkSKey}; - false -> find_right_key(B0, MIC, Payload, Device, Keys) + true -> + {Device, NwkSKey}; + false -> + case key_matches_any_fcnt(NwkSKey, MIC, Payload) of + false -> find_right_key(B0, MIC, Payload, Device, Keys); + true -> {Device, NwkSKey} + end end. +-spec key_matches_any_fcnt(binary(), binary(), binary()) -> boolean(). +key_matches_any_fcnt(NwkSKey, ExpectedMIC, Payload) -> + FCntLow = payload_fcnt_low(Payload), + lists:any( + fun(HighBits) -> + FCnt = binary:decode_unsigned( + <>, + little + ), + B0 = payload_b0(Payload, FCnt), + ComputedMIC = crypto:macN(cmac, aes_128_cbc, NwkSKey, B0, 4), + ComputedMIC =:= ExpectedMIC + end, + lists:seq(2#000, 2#111) + ). + -spec key_matches_mic(binary(), binary(), binary()) -> boolean(). key_matches_mic(Key, B0, ExpectedMIC) -> ComputedMIC = crypto:macN(cmac, aes_128_cbc, Key, B0, 4), @@ -1316,8 +1334,8 @@ expected_fcnt(Device, Payload) -> binary:decode_unsigned(<>) end. --spec b0_from_payload(binary(), non_neg_integer()) -> binary(). -b0_from_payload(Payload, ExpectedFCnt) -> +-spec payload_b0(binary(), non_neg_integer()) -> binary(). +payload_b0(Payload, ExpectedFCnt) -> <> = Payload, Msg = binary:part(Payload, {0, erlang:byte_size(Payload) - 4}), diff --git a/src/device/router_device_worker.erl b/src/device/router_device_worker.erl index 1021138b0..94b4f0eb6 100644 --- a/src/device/router_device_worker.erl +++ b/src/device/router_device_worker.erl @@ -245,7 +245,8 @@ handle_call( HoldTime, PacketFCnt, Device, - PubKeyBin + PubKeyBin, + Packet ), {reply, false, State}; true -> @@ -842,7 +843,8 @@ handle_cast( HoldTime, PacketFCnt, Device1, - PubKeyBin + PubKeyBin, + Packet0 ); _ -> ok = router_utils:event_uplink_dropped_invalid_packet( @@ -1545,9 +1547,19 @@ validate_frame( FrameCache, OfferCache ) -> - <> = blockchain_helium_packet_v1:payload(Packet), + Payload = + <> = blockchain_helium_packet_v1:payload(Packet), + + VerifiedFCnt = verified_fcnt_from_payload( + router_device_routing:payload_fcnt_low(Payload), + router_device:nwk_s_key(Device0), + router_device_routing:payload_mic(Payload), + Payload + ), + DeviceFCnt = router_device:fcnt(Device0), + case MType of MType when MType == ?CONFIRMED_UP orelse MType == ?UNCONFIRMED_UP -> FrameAck = router_utils:mtype_to_ack(MType), @@ -1606,6 +1618,12 @@ validate_frame( OfferCache, true ); + undefined when VerifiedFCnt < DeviceFCnt -> + lager:info( + "we got a replay packet [verified: ~p] [device: ~p]", + [VerifiedFCnt, DeviceFCnt] + ), + {error, late_packet}; undefined -> lager:debug("we got a fresh packet [fcnt: ~p]", [PacketFCnt]), validate_frame_( @@ -2415,6 +2433,32 @@ maybe_will_downlink(Device, #frame{mtype = MType, adrackreq = ADRAckReqBit}) -> ADR = ADRAllowed andalso ADRAckReqBit == 1, DeviceQueue =/= [] orelse ACK == 1 orelse ADR orelse ChannelCorrection == false. +-spec verified_fcnt_from_payload(non_neg_integer(), binary(), binary(), binary()) -> + non_neg_integer(). +verified_fcnt_from_payload(FCntLow, NwkSKey, ExpectedMIC, Payload) -> + find_first( + fun(HighBits) -> + FCnt = binary:decode_unsigned( + <>, + little + ), + B0 = router_device_routing:payload_b0(Payload, FCnt), + ComputedMIC = crypto:macN(cmac, aes_128_cbc, NwkSKey, B0, 4), + {ComputedMIC =:= ExpectedMIC, FCnt} + end, + lists:seq(2#000, 2#111) + ). + +-spec find_first( + FN :: fun((HighBit :: non_neg_integer()) -> {Verified :: boolean(), FCnt :: non_neg_integer()}), + HighBits :: list(non_neg_integer()) +) -> non_neg_integer(). +find_first(Fn, [FCntHigh | Rest]) -> + case Fn(FCntHigh) of + {true, Found} -> Found; + _ -> find_first(Fn, Rest) + end. + -ifdef(TEST). -include_lib("eunit/include/eunit.hrl"). diff --git a/src/router_utils.erl b/src/router_utils.erl index b93c293f6..ac860ea4f 100644 --- a/src/router_utils.erl +++ b/src/router_utils.erl @@ -9,7 +9,7 @@ event_uplink/9, event_uplink_dropped_device_inactive/4, event_uplink_dropped_not_enough_dc/4, - event_uplink_dropped_late_packet/5, + event_uplink_dropped_late_packet/6, event_uplink_dropped_invalid_packet/7, event_downlink/9, event_downlink_dropped_payload_size_exceeded/5, @@ -39,7 +39,8 @@ enumerate_last/1, metadata_fun/0, random_non_miner_predicate/1, - get_swarm_key_location/0 + get_swarm_key_location/0, + calculate_dc_amount_for_packet/1 ]). -type uuid_v4() :: binary(). @@ -75,6 +76,13 @@ metadata_fun() -> #{} end. +-spec calculate_dc_amount_for_packet(blockchain_helium_packet_v1:packet()) -> non_neg_integer(). +calculate_dc_amount_for_packet(Packet) -> + Payload = blockchain_helium_packet_v1:payload(Packet), + PayloadSize = erlang:byte_size(Payload), + Used = router_blockchain:calculate_dc_amount(PayloadSize), + Used. + -spec event_join_request( ID :: uuid_v4(), Timestamp :: non_neg_integer(), @@ -264,9 +272,22 @@ event_uplink_dropped_not_enough_dc(Timestamp, FCnt, Device, PubKeyBin) -> HoldTime :: non_neg_integer(), FCnt :: non_neg_integer(), Device :: router_device:device(), - PubKeyBin :: libp2p_crypto:pubkey_bin() + PubKeyBin :: libp2p_crypto:pubkey_bin(), + Packet :: blockchain_helium_packet_v1:packet() ) -> ok. -event_uplink_dropped_late_packet(Timestamp, HoldTime, FCnt, Device, PubKeyBin) -> +event_uplink_dropped_late_packet( + Timestamp, + HoldTime, + FCnt, + Device, + PubKeyBin, + Packet +) -> + {Balance, Nonce, Used} = + case router_console_dc_tracker:maybe_charge_late(Device, Packet) of + {ok, B, N} -> {B, N, ?MODULE:calculate_dc_amount_for_packet(Packet)}; + {error, _E} -> {0, 0, 0} + end, Map = #{ id => router_utils:uuid_v4(), category => uplink_dropped, @@ -279,7 +300,12 @@ event_uplink_dropped_late_packet(Timestamp, HoldTime, FCnt, Device, PubKeyBin) - payload => <<>>, port => 0, devaddr => lorawan_utils:binary_to_hex(router_device:devaddr(Device)), - hotspot => format_uncharged_hotspot(PubKeyBin) + hotspot => format_uncharged_hotspot(PubKeyBin), + dc => #{ + balance => Balance, + nonce => Nonce, + used => Used + } }, ok = router_console_api:event(Device, Map). From 15d8cd33da819612ef5e915d4bce147a2c84b3f9 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 21 Aug 2023 09:29:09 -0700 Subject: [PATCH 02/13] better failure formatting --- test/test_utils.erl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/test_utils.erl b/test/test_utils.erl index 9a7b1ecb0..0e87bd451 100644 --- a/test/test_utils.erl +++ b/test/test_utils.erl @@ -639,10 +639,7 @@ wait_for_console_event_sub(SubCategory, Expected) -> {ok, Got}; {false, Reason} -> ct:pal("FAILED got: ~n~p~n expected: ~n~p", [Got, Expected]), - ct:fail("wait_for_console_event_sub ~p data failed ~p", [ - SubCategory, - Reason - ]) + ct:fail(Reason) end after 4250 -> ct:fail("wait_for_console_event_sub ~p timeout", [SubCategory]) end @@ -652,7 +649,12 @@ wait_for_console_event_sub(SubCategory, Expected) -> SubCategory, {_Reason, _Stacktrace} ]), - ct:fail("wait_for_console_event_sub ~p failed", [SubCategory]) + ct:fail( + {wait_for_console_event_sub, [ + {sub_category, SubCategory}, + {reason, _Reason} + ]} + ) end. wait_for_join_resp(PubKeyBin, AppKey, DevNonce) -> From 9dc5b078be77c60afebc95663cd53706103d096d Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 21 Aug 2023 09:29:19 -0700 Subject: [PATCH 03/13] fix test for bigger fcnt search space --- test/router_device_routing_SUITE.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/router_device_routing_SUITE.erl b/test/router_device_routing_SUITE.erl index 6154bf83b..672c8e48f 100644 --- a/test/router_device_routing_SUITE.erl +++ b/test/router_device_routing_SUITE.erl @@ -846,8 +846,9 @@ handle_packet_wrong_fcnt_test(Config) -> devaddr => router_device:devaddr(Device0) }), + %% NOTE: packets with previous fcnts are now correctly identified to the device that can decode them. ?assertEqual( - {error, unknown_device}, + ok, router_device_routing:handle_packet( SCPacket1, erlang:system_time(millisecond), self() ) From 7d77e2b235006dc91ce7b1adf50dec0277d8ce06 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 21 Aug 2023 09:38:13 -0700 Subject: [PATCH 04/13] send the verified fcnt --- src/device/router_device_worker.erl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/device/router_device_worker.erl b/src/device/router_device_worker.erl index 94b4f0eb6..97298e616 100644 --- a/src/device/router_device_worker.erl +++ b/src/device/router_device_worker.erl @@ -837,11 +837,11 @@ handle_cast( {error, Reason} -> lager:debug("packet not validated: ~p", [Reason]), case Reason of - late_packet -> + {late_packet, VerifiedFCnt} -> ok = router_utils:event_uplink_dropped_late_packet( PacketTime, HoldTime, - PacketFCnt, + VerifiedFCnt, Device1, PubKeyBin, Packet0 @@ -1591,7 +1591,7 @@ validate_frame( PacketFCnt, LastSeenFCnt ]), - {error, late_packet}; + {error, {late_packet, VerifiedFCnt}}; undefined when FrameAck == 1 andalso PacketFCnt == DownlinkHandledAtFCnt andalso Window < ?RX_MAX_WINDOW @@ -1600,7 +1600,7 @@ validate_frame( "we got a late confirmed up packet for ~p: DownlinkHandledAt: ~p within window ~p", [PacketFCnt, DownlinkHandledAtFCnt, Window] ), - {error, late_packet}; + {error, {late_packet, VerifiedFCnt}}; undefined when FrameAck == 1 andalso PacketFCnt == DownlinkHandledAtFCnt andalso Window >= ?RX_MAX_WINDOW @@ -1623,7 +1623,7 @@ validate_frame( "we got a replay packet [verified: ~p] [device: ~p]", [VerifiedFCnt, DeviceFCnt] ), - {error, late_packet}; + {error, {late_packet, VerifiedFCnt}}; undefined -> lager:debug("we got a fresh packet [fcnt: ~p]", [PacketFCnt]), validate_frame_( From db00edc9801f46b672697d5f5805de85d79271c6 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 21 Aug 2023 09:38:31 -0700 Subject: [PATCH 05/13] test for very late packets --- test/router_device_worker_SUITE.erl | 107 ++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/test/router_device_worker_SUITE.erl b/test/router_device_worker_SUITE.erl index 94ff03b22..55b01237d 100644 --- a/test/router_device_worker_SUITE.erl +++ b/test/router_device_worker_SUITE.erl @@ -19,6 +19,7 @@ replay_joins_test/1, ddos_joins_test/1, replay_uplink_test/1, + replay_uplink_far_in_the_past_test/1, device_worker_stop_children_test/1, device_worker_late_packet_double_charge_test/1, offer_cache_test/1, @@ -73,6 +74,7 @@ all_tests() -> replay_joins_test, ddos_joins_test, replay_uplink_test, + replay_uplink_far_in_the_past_test, device_worker_late_packet_double_charge_test, offer_cache_test, load_offer_cache_test, @@ -1325,6 +1327,111 @@ replay_uplink_test(Config) -> ok. +replay_uplink_far_in_the_past_test(Config) -> + #{ + pubkey_bin := PubKeyBin, + stream := Stream, + hotspot_name := HotspotName + } = test_utils:join_device(Config), + + %% Check that device is in cache now + {ok, DB, CF} = router_db:get_devices(), + WorkerID = router_devices_sup:id(?CONSOLE_DEVICE_ID), + {ok, Device0} = router_device:get_by_id(DB, CF, WorkerID), + + Stream ! + {send, + test_utils:frame_packet( + ?UNCONFIRMED_UP, + PubKeyBin, + router_device:nwk_s_key(Device0), + router_device:app_s_key(Device0), + 10_000 + )}, + + test_utils:wait_channel_data(#{ + <<"type">> => <<"uplink">>, + <<"replay">> => false, + <<"uuid">> => fun erlang:is_binary/1, + <<"id">> => ?CONSOLE_DEVICE_ID, + <<"downlink_url">> => + <>, + <<"name">> => ?CONSOLE_DEVICE_NAME, + <<"dev_eui">> => lorawan_utils:binary_to_hex(?DEVEUI), + <<"app_eui">> => lorawan_utils:binary_to_hex(?APPEUI), + <<"metadata">> => #{ + <<"labels">> => ?CONSOLE_LABELS, + <<"organization_id">> => ?CONSOLE_ORG_ID, + <<"multi_buy">> => fun erlang:is_integer/1, + <<"adr_allowed">> => false, + <<"cf_list_enabled">> => false, + <<"rx_delay_state">> => fun erlang:is_binary/1, + <<"rx_delay">> => 0, + <<"preferred_hotspots">> => fun erlang:is_list/1 + }, + <<"fcnt">> => 10_000, + <<"reported_at">> => fun erlang:is_integer/1, + <<"payload">> => <<>>, + <<"payload_size">> => 0, + <<"raw_packet">> => fun erlang:is_binary/1, + <<"port">> => 1, + <<"devaddr">> => '_', + <<"hotspots">> => [ + #{ + <<"id">> => erlang:list_to_binary(libp2p_crypto:bin_to_b58(PubKeyBin)), + <<"name">> => erlang:list_to_binary(HotspotName), + <<"reported_at">> => fun erlang:is_integer/1, + <<"hold_time">> => fun erlang:is_integer/1, + <<"status">> => <<"success">>, + <<"rssi">> => 0.0, + <<"snr">> => 0.0, + <<"spreading">> => <<"SF8BW125">>, + <<"frequency">> => fun erlang:is_float/1, + <<"channel">> => fun erlang:is_number/1, + <<"lat">> => fun erlang:is_float/1, + <<"long">> => fun erlang:is_float/1 + } + ], + <<"dc">> => #{ + <<"balance">> => fun erlang:is_integer/1, + <<"nonce">> => fun erlang:is_integer/1 + } + }), + + timer:sleep(2000 + 100), + + Stream ! + {send, + test_utils:frame_packet( + ?UNCONFIRMED_UP, + PubKeyBin, + router_device:nwk_s_key(Device0), + router_device:app_s_key(Device0), + 5000 + )}, + + test_utils:wait_for_console_event_sub(<<"uplink_dropped_late">>, #{ + <<"category">> => <<"uplink_dropped">>, + <<"data">> => #{ + <<"fcnt">> => 5000, + <<"hold_time">> => 100, + <<"hotspot">> => fun erlang:is_map/1, + <<"dc">> => #{ + <<"balance">> => 97, + <<"nonce">> => 1, + <<"used">> => 1 + } + }, + <<"description">> => <<"Late packet">>, + <<"device_id">> => <<"yolo_id">>, + <<"id">> => fun erlang:is_binary/1, + <<"reported_at">> => fun erlang:is_integer/1, + <<"sub_category">> => <<"uplink_dropped_late">> + }), + + ok. + offer_cache_test(Config) -> #{ pubkey_bin := PubKeyBin1 From 0c0fbb5ef71534da92ef0d9c44b3edb8df4133e7 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 21 Aug 2023 11:25:54 -0700 Subject: [PATCH 06/13] update tests and add env vars for charging late packets --- .env-template | 3 +++ config/sys.config.src | 3 ++- config/test.config | 1 + test/router_device_worker_SUITE.erl | 10 ++++++++-- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.env-template b/.env-template index 2dbc9b30b..33a7bb984 100644 --- a/.env-template +++ b/.env-template @@ -68,3 +68,6 @@ ROUTER_DEVADDR_PREFIX=72 # Charge an Organization for join requests (default: true) ROUTER_CHARGE_JOINS=true + +# Charge an org for a packet received late or replayed (default: false). +ROUTER_CHARGE_LATE_PACKETS=false diff --git a/config/sys.config.src b/config/sys.config.src index 68ef969a8..f929ed1fe 100644 --- a/config/sys.config.src +++ b/config/sys.config.src @@ -69,7 +69,8 @@ {metrics_port, "${ROUTER_METRICS_PORT}"}, {denylist_keys, ["1SbEYKju337P6aYsRd9DT2k4qgK5ZK62kXbSvnJgqeaxK3hqQrYURZjL"]}, {denylist_url, "https://api.github.com/repos/helium/denylist/releases/latest"}, - {charge_joins, "${ROUTER_CHARGE_JOINS}"} + {charge_joins, "${ROUTER_CHARGE_JOINS}"}, + {charge_late_packets, "${ROUTER_CHARGE_LATE_PACKETS}"} ]}, {grpcbox, [ {client, #{ diff --git a/config/test.config b/config/test.config index 2a34463d1..b12634468 100644 --- a/config/test.config +++ b/config/test.config @@ -8,6 +8,7 @@ {router_http_channel_url_check, false}, {router_xor_filter_worker, false}, {charge_when_no_offer, true}, + {charge_late_packets, true}, {ics, #{ transport => http, host => "localhost", diff --git a/test/router_device_worker_SUITE.erl b/test/router_device_worker_SUITE.erl index 55b01237d..6458d32a1 100644 --- a/test/router_device_worker_SUITE.erl +++ b/test/router_device_worker_SUITE.erl @@ -344,8 +344,13 @@ device_worker_late_packet_double_charge_test(Config) -> %% Make sure DC is not being charged for the late packet. {EndingBalance, EndingNonce} = router_console_dc_tracker:current_balance(?CONSOLE_ORG_ID), + ChargedPackets = + case router_utils:get_env_bool(charge_late_packets, false) of + true -> 2; + false -> 1 + end, - ?assertEqual(EndingBalance, StartingBalance - 1), + ?assertEqual(EndingBalance, StartingBalance - ChargedPackets), ?assertEqual(EndingNonce, StartingNonce), ok. @@ -1601,7 +1606,8 @@ offer_cache_test(Config) -> <<"id">> => erlang:list_to_binary(libp2p_crypto:bin_to_b58(PubKeyBin1)), <<"name">> => erlang:list_to_binary(blockchain_utils:addr2name(PubKeyBin1)) }, - <<"hold_time">> => fun erlang:is_integer/1 + <<"hold_time">> => fun erlang:is_integer/1, + <<"dc">> => fun erlang:is_map/1 } }), From 6c880a6879c4926a83e05905eac9d04dbb224b52 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 21 Aug 2023 11:40:39 -0700 Subject: [PATCH 07/13] remove offer tracking test, dup of multi buy test --- test/router_device_routing_SUITE.erl | 93 +--------------------------- 1 file changed, 1 insertion(+), 92 deletions(-) diff --git a/test/router_device_routing_SUITE.erl b/test/router_device_routing_SUITE.erl index 672c8e48f..232c2f48b 100644 --- a/test/router_device_routing_SUITE.erl +++ b/test/router_device_routing_SUITE.erl @@ -25,8 +25,7 @@ handle_join_packet_from_preferred_hotspot_test/1, handle_join_packet_with_no_preferred_hotspot_test/1, handle_packet_fcnt_32_bit_rollover_test/1, - handle_packet_wrong_fcnt_test/1, - handle_packet_using_post_offer/1 + handle_packet_wrong_fcnt_test/1 ]). -include_lib("helium_proto/include/blockchain_state_channel_v1_pb.hrl"). @@ -67,7 +66,6 @@ groups() -> all_tests() -> [ - handle_packet_using_post_offer, multi_buy_test, packet_hash_cache_test, join_offer_from_strange_hotspot_test, @@ -643,95 +641,6 @@ multi_buy_test(Config) -> ok. -handle_packet_using_post_offer(Config) -> - %% We can toggle accepting packets by running it through the handle_offer function - ok = remove_devaddr_allocate_meck(), - test_utils:add_oui(Config), - #{pubkey_bin := PubKeyBin1} = test_utils:join_device(Config), - - %% Waiting for reply from router to hotspot - test_utils:wait_state_channel_message(1250), - - %% Check that device is in cache now - {ok, DB, CF} = router_db:get_devices(), - WorkerID = router_devices_sup:id(?CONSOLE_DEVICE_ID), - {ok, Device} = router_device:get_by_id(DB, CF, WorkerID), - - %% Hotspot 2-5 - [PubKeyBin2, PubKeyBin3, PubKeyBin4, PubKeyBin5] = lists:map( - fun(_) -> - #{public := PubKey} = libp2p_crypto:generate_keys(ecc_compact), - libp2p_crypto:pubkey_to_bin(PubKey) - end, - lists:seq(1, 4) - ), - - HandlePacketForHotspotFun = fun(PubKeyBin, Device0, FCnt) -> - SCPacket = test_utils:frame_packet( - ?UNCONFIRMED_UP, - PubKeyBin, - router_device:nwk_s_key(Device0), - router_device:app_s_key(Device0), - FCnt, - #{dont_encode => true, devaddr => router_device:devaddr(Device0)} - ), - router_device_routing:handle_free_packet(SCPacket, erlang:system_time(millisecond), self()) - end, - - DeviceID = router_device:id(Device), - ok = meck:new(blockchain_state_channels_server, [passthrough]), - ok = meck:expect( - blockchain_state_channels_server, - track_offer, - %% All validation has been passed, but there are no state channels in - %% the tests, let's pretend we tracked it well. - fun(_Offer, _Ledger, _HandlerPid) -> - ok - end - ), - - %% =================================================================== - %% Already default - - %% Multi buy for device is set to 2 - ok = router_device_multibuy:max(DeviceID, 2), - - %% Send the same packet from all hotspots - ok = HandlePacketForHotspotFun(PubKeyBin1, Device, 0), - ok = HandlePacketForHotspotFun(PubKeyBin2, Device, 0), - {error, multi_buy_max_packet} = HandlePacketForHotspotFun(PubKeyBin3, Device, 0), - {error, multi_buy_max_packet} = HandlePacketForHotspotFun(PubKeyBin4, Device, 0), - - case router_blockchain:is_chain_dead() of - false -> - %% 2 packets should have made it to state channels - ?assertEqual(2, meck:num_calls(blockchain_state_channels_server, track_offer, '_')); - true -> - ok - end, - - %% Change multi-buy to 4 - ok = router_device_multibuy:max(DeviceID, 4), - - %% Send the same packet from all hotspots - ok = HandlePacketForHotspotFun(PubKeyBin1, Device, 1), - ok = HandlePacketForHotspotFun(PubKeyBin2, Device, 1), - ok = HandlePacketForHotspotFun(PubKeyBin3, Device, 1), - ok = HandlePacketForHotspotFun(PubKeyBin4, Device, 1), - {error, multi_buy_max_packet} = HandlePacketForHotspotFun(PubKeyBin5, Device, 1), - - case router_blockchain:is_chain_dead() of - false -> - %% 4 packets should have made it to state channels (2 previous + 4 new) - ?assertEqual(6, meck:num_calls(blockchain_state_channels_server, track_offer, '_')); - true -> - ok - end, - - ok = meck:unload(blockchain_state_channels_server), - - ok. - handle_packet_fcnt_32_bit_rollover_test(Config) -> WaitForDeviceWorker = fun() -> timer:sleep(500) From 7ebec37cc1e29a5ef142bd8b502181e84a7a5b89 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Mon, 21 Aug 2023 11:47:17 -0700 Subject: [PATCH 08/13] update eunit for expecting records --- src/grpc/router_ics_skf_worker.erl | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/grpc/router_ics_skf_worker.erl b/src/grpc/router_ics_skf_worker.erl index 3da9f1cc2..51360f353 100644 --- a/src/grpc/router_ics_skf_worker.erl +++ b/src/grpc/router_ics_skf_worker.erl @@ -536,21 +536,29 @@ dedup_updates(Updates) -> -include_lib("eunit/include/eunit.hrl"). dedup_updates_test() -> + MakeUpdate = fun(Action, Devaddr, SessionKey, MaxCopies) -> + #iot_config_route_skf_update_v1_pb{ + action = Action, + devaddr = Devaddr, + session_key = SessionKey, + max_copies = MaxCopies + } + end, ?assertEqual( - [{add, 1, <<>>, 5}], - dedup_updates([{add, 1, <<>>, 5}]), + [MakeUpdate(add, 1, <<>>, 5)], + dedup_updates([MakeUpdate(add, 1, <<>>, 5)]), "adds are untouched" ), ?assertEqual( - [{add, 1, <<>>, 5}], - dedup_updates([{add, 1, <<>>, 5}, {remove, 1, <<>>, 9999}]), + [MakeUpdate(add, 1, <<>>, 5)], + dedup_updates([MakeUpdate(add, 1, <<>>, 5), MakeUpdate(remove, 1, <<>>, 9999)]), "removes that match an add are removed" ), ?assertEqual( - [{add, 1, <<>>, 5}], - dedup_updates([{remove, 1, <<>>, 9999}, {add, 1, <<>>, 5}]), + [MakeUpdate(add, 1, <<>>, 5)], + dedup_updates([MakeUpdate(remove, 1, <<>>, 9999), MakeUpdate(add, 1, <<>>, 5)]), "removes that match an add are removed regardless of order" ), From a42ea6c33fd7cf1c054b13eeaa9ec6d8c51a6ddb Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Tue, 22 Aug 2023 09:51:19 -0700 Subject: [PATCH 09/13] require dc map --- src/apis/router_console_api.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apis/router_console_api.erl b/src/apis/router_console_api.erl index 3b0265ab1..ad3a33e86 100644 --- a/src/apis/router_console_api.erl +++ b/src/apis/router_console_api.erl @@ -325,7 +325,7 @@ event(Device, Map) -> fcnt => maps:get(fcnt, Map), hotspot => maps:get(hotspot, Map), hold_time => maps:get(hold_time, Map), - dc => maps:get(dc, Map, #{}) + dc => maps:get(dc, Map) }; {uplink_dropped, _SC} -> #{ From fe4a744edf3c17717e1ef3b08b870d535e31a613 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Tue, 22 Aug 2023 10:52:14 -0700 Subject: [PATCH 10/13] ensure dc used is 0 if not charging for late packets --- src/apis/router_console_dc_tracker.erl | 11 +++++++---- src/router_utils.erl | 12 ++---------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/apis/router_console_dc_tracker.erl b/src/apis/router_console_dc_tracker.erl index 311dec778..28031324a 100644 --- a/src/apis/router_console_dc_tracker.erl +++ b/src/apis/router_console_dc_tracker.erl @@ -181,21 +181,24 @@ current_balance(OrgID) -> -spec maybe_charge_late( Device :: router_device:device(), Packet :: blockchain_helium_packet_v1:packet() -) -> {ok, non_neg_integer(), non_neg_integer()} | {error, any()}. +) -> + {ok, Balance :: non_neg_integer(), Nonce :: non_neg_integer(), Used :: non_neg_integer()} + | {error, any()}. maybe_charge_late(Device, Packet) -> OrgID = router_device:organization_id(Device), case router_utils:get_env_bool(charge_late_packets, false) of false -> {Balance, Nonce} = current_balance(OrgID), - {ok, Balance, Nonce}; + {ok, Balance, Nonce, 0}; true -> Payload = blockchain_helium_packet_v1:payload(Packet), PayloadSize = erlang:byte_size(Payload), case charge(Device, PayloadSize) of {error, _} = E -> E; - Res -> - Res + {ok, Balance, Nonce} -> + Used = router_blockchain:calculate_dc_amount(PayloadSize), + {ok, Balance, Nonce, Used} end end. diff --git a/src/router_utils.erl b/src/router_utils.erl index ac860ea4f..cc82a9847 100644 --- a/src/router_utils.erl +++ b/src/router_utils.erl @@ -39,8 +39,7 @@ enumerate_last/1, metadata_fun/0, random_non_miner_predicate/1, - get_swarm_key_location/0, - calculate_dc_amount_for_packet/1 + get_swarm_key_location/0 ]). -type uuid_v4() :: binary(). @@ -76,13 +75,6 @@ metadata_fun() -> #{} end. --spec calculate_dc_amount_for_packet(blockchain_helium_packet_v1:packet()) -> non_neg_integer(). -calculate_dc_amount_for_packet(Packet) -> - Payload = blockchain_helium_packet_v1:payload(Packet), - PayloadSize = erlang:byte_size(Payload), - Used = router_blockchain:calculate_dc_amount(PayloadSize), - Used. - -spec event_join_request( ID :: uuid_v4(), Timestamp :: non_neg_integer(), @@ -285,7 +277,7 @@ event_uplink_dropped_late_packet( ) -> {Balance, Nonce, Used} = case router_console_dc_tracker:maybe_charge_late(Device, Packet) of - {ok, B, N} -> {B, N, ?MODULE:calculate_dc_amount_for_packet(Packet)}; + {ok, B, N, U} -> {B, N, U}; {error, _E} -> {0, 0, 0} end, Map = #{ From 29a8192dd4fe1bed3a0fd2c851790c6faf84a73d Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Tue, 22 Aug 2023 15:13:43 -0700 Subject: [PATCH 11/13] add callout of settings at beginning of test - to ensure that settings is present despite how test.config may change. - change fcnt of second replay to make debugging easier --- test/router_device_worker_SUITE.erl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/router_device_worker_SUITE.erl b/test/router_device_worker_SUITE.erl index 6458d32a1..61d8a01a6 100644 --- a/test/router_device_worker_SUITE.erl +++ b/test/router_device_worker_SUITE.erl @@ -1339,6 +1339,9 @@ replay_uplink_far_in_the_past_test(Config) -> hotspot_name := HotspotName } = test_utils:join_device(Config), + %% Callout this setting + ok = application:set_env(router, charge_late_packets, true), + %% Check that device is in cache now {ok, DB, CF} = router_db:get_devices(), WorkerID = router_devices_sup:id(?CONSOLE_DEVICE_ID), From 4032b16d130b2775ea4cff4615f13c3d4d34d36b Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Wed, 23 Aug 2023 13:08:28 -0700 Subject: [PATCH 12/13] show payload and size for late packet when charged --- src/router_utils.erl | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/router_utils.erl b/src/router_utils.erl index cc82a9847..cf7213822 100644 --- a/src/router_utils.erl +++ b/src/router_utils.erl @@ -280,6 +280,11 @@ event_uplink_dropped_late_packet( {ok, B, N, U} -> {B, N, U}; {error, _E} -> {0, 0, 0} end, + Payload = + case ?MODULE:get_env_bool(charge_late_packets, false) of + true -> blockchain_helium_packet_v1:payload(Packet); + false -> <<>> + end, Map = #{ id => router_utils:uuid_v4(), category => uplink_dropped, @@ -288,8 +293,8 @@ event_uplink_dropped_late_packet( reported_at => Timestamp, hold_time => HoldTime, fcnt => FCnt, - payload_size => 0, - payload => <<>>, + payload_size => erlang:byte_size(Payload), + payload => base64:encode(Payload), port => 0, devaddr => lorawan_utils:binary_to_hex(router_device:devaddr(Device)), hotspot => format_uncharged_hotspot(PubKeyBin), From 252a5527d927203f58e5bf7dbf5e79275297a855 Mon Sep 17 00:00:00 2001 From: Michael Jeffrey Date: Wed, 23 Aug 2023 13:12:43 -0700 Subject: [PATCH 13/13] better handle default env bool case --- src/router_utils.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/router_utils.erl b/src/router_utils.erl index cf7213822..256971539 100644 --- a/src/router_utils.erl +++ b/src/router_utils.erl @@ -688,6 +688,7 @@ get_env_int(Key, Default) -> -spec get_env_bool(atom(), boolean()) -> boolean(). get_env_bool(Key, Default) -> case application:get_env(router, Key, Default) of + [] -> Default; "true" -> true; true -> true; _ -> false