helium/blockchain-core

HIP17, incorrect number of neighbor hexes

ci-work opened this issue · 2 comments

according to HIP17:

Note there are 7 “neighbor” hexs (6 + reference hex in center).

However the implementation https://github.com/helium/blockchain-core/blob/master/src/blockchain_hex.erl#L273 uses kring and does not consider the hex in the center,

occupied_count(DensityTarget, ThisResHex, ClipETS) ->
    H3Neighbors = h3:k_ring(ThisResHex, 1),

    lists:foldl(
        fun(Neighbor, Acc) ->
            case lookup(ClipETS, Neighbor) >= DensityTarget of
                false -> Acc;
                true -> Acc + 1
            end
        end,
        0,
        H3Neighbors
    ).

to consider the middle hex, this would need to be changed to something like:

occupied_count(DensityTarget, ThisResHex, ClipETS) ->
    H3Neighbors = h3:k_ring(ThisResHex, 1),
    %% HIP17 states Note there are 7 "neighbor" hexs (6 + reference hex in center).
    Base = case lookup(ClipETS, ThisResHex) >= DensityTarget of
               false -> 0;
               true -> 1
           end,
    lists:foldl(
        fun(Neighbor, Acc) ->
            case lookup(ClipETS, Neighbor) >= DensityTarget of
                false -> Acc;
                true -> Acc + 1
            end
        end,
        Base,
        H3Neighbors
    ).
vihu commented

Thank you for the issue but I believe that h3:k_ring already returns the hex around which the k_ring is being requested. Example:

(miner@127.0.0.1)33> H3 = 631262236224198143.
631262236224198143
(miner@127.0.0.1)34> h3:k_ring(H3, 1).                
[631262236224198143,631262236224193535,631262236224207359,
 631262236224207871,631262236224198655,631262236224197119,
 631262236224200191]
(miner@127.0.0.1)35> lists:member(H3, h3:k_ring(H3, 1)).
true

thanks for checking so quickly @vihu