Fix embargo timeout in dandelion++

src/crypto/duration.h

@@ -68,4 +68,30 @@ namespace crypto
   //! Generate random duration with 1/4 second precision
   using random_poisson_subseconds =
     random_poisson_duration<std::chrono::duration<std::chrono::milliseconds::rep, std::ratio<1, 4>>>;
+
+  template<typename D>
+  struct random_exponential_duration
+  {
+    using result_type = D;
+    using rep = typename result_type::rep;
+
+    explicit random_exponential_duration(double rate)
+      : dist(rate)
+    {}
+
+    result_type operator()()
+    {
+      /* Note this always rounds down to nearest whole number. if `std::lround`
+        was used instead, then 0 seconds would be used less frequently. Not sure
+        which is better, since we cannot broadcast on sub-seconds intervals. */
+      crypto::random_device rand{};
+      return result_type{rep(dist(rand))};
+    }
+
+  private:
+    std::exponential_distribution<double> dist;
+  };
+
+  using random_exponential_seconds = random_exponential_duration<std::chrono::seconds>;
 }
+

src/cryptonote_config.h

@@ -104,11 +104,12 @@
 
 
 #define CRYPTONOTE_DANDELIONPP_STEMS              2 // number of outgoing stem connections per epoch
-#define CRYPTONOTE_DANDELIONPP_FLUFF_PROBABILITY 20 // out of 100
+#define CRYPTONOTE_DANDELIONPP_FLUFF_PROBABILITY 12 // out of 100
 #define CRYPTONOTE_DANDELIONPP_MIN_EPOCH         10 // minutes
 #define CRYPTONOTE_DANDELIONPP_EPOCH_RANGE       30 // seconds
 #define CRYPTONOTE_DANDELIONPP_FLUSH_AVERAGE      5 // seconds average for poisson distributed fluff flush
-#define CRYPTONOTE_DANDELIONPP_EMBARGO_AVERAGE   39 // seconds (see tx_pool.cpp for more info)
+#define CRYPTONOTE_DANDELIONPP_EMBARGO_RATE      double(1)/double(46.5) // seconds (see tx_pool.cpp for more info)
+#define CRYPTONOTE_DANDELIONPP_EMBARGO_MAX      180 // seconds
 
 // see src/cryptonote_protocol/levin_notify.cpp
 #define CRYPTONOTE_NOISE_MIN_EPOCH                      5      // minutes

src/cryptonote_core/tx_pool.cpp

@@ -58,28 +58,27 @@ namespace cryptonote
 {
   namespace
   {
-    /*! The Dandelion++ has formula for calculating the average embargo timeout:
-                          (-k*(k-1)*hop)/(2*log(1-ep))
-        where k is the number of hops before this node and ep is the probability
-        that one of the k hops hits their embargo timer, and hop is the average
-        time taken between hops. So decreasing ep will make it more probable
-        that "this" node is the first to expire the embargo timer. Increasing k
-        will increase the number of nodes that will be "hidden" as a prior
-        recipient of the tx.
-
-        As example, k=5 and ep=0.1 means "this" embargo timer has a 90%
-        probability of being the first to expire amongst 5 nodes that saw the
-        tx before "this" one. These values are independent to the fluff
-        probability, but setting a low k with a low p (fluff probability) is
-        not ideal since a blackhole is more likely to reveal earlier nodes in
-        the chain.
-
-        This value was calculated with k=5, ep=0.10, and hop = 175 ms. A
+    /*! The Dandelion++ has formula for calculating the embargo rate:
+                          (-k*(k-1)*hop)/(2*ln(1-ep))
+        where k is the number of hops before the fluff node and ep is the
+        probability that one of the k hops hits their embargo timer before
+        reaching the fluff node, and hop is the average time taken between hops.
+
+        NOTE: The paper says `2*log(1-ep)`, however if you read the explanation
+        in b.5 it is clear they meant `ln`.
+
+        As example, k=10 and ep=0.1 means "this" embargo timer has a 90%
+        probability of reaching 10 hops before the embargo timer fires. These
+        values are independent to the fluff probability.
+
+        The embargo rate was calculated with k=8, ep=0.1, and hop = 175 ms. A
         testrun from a recent Intel laptop took ~80ms to
         receive+parse+proces+send transaction. At least 50ms will be added to
         the latency if crossing an ocean. So 175ms is the fudge factor for
-        a single hop with 39s being the embargo timer. */
-    constexpr const std::chrono::seconds dandelionpp_embargo_average{CRYPTONOTE_DANDELIONPP_EMBARGO_AVERAGE};
+        a single hop with 1/46.5 being the embargo _rate_. The average time
+        to blackhole fluff will be 46.5/hops where hops is the number of hops
+        before being blackholed. */
+    // see cryptonote_config.h CRYPTONOTE_DANDELIONPP_EMBARGO_RATE
 
     //TODO: constants such as these should at least be in the header,
     //      but probably somewhere more accessible to the rest of the
@@ -889,7 +888,7 @@ namespace cryptonote
   {
     just_broadcasted.clear();
 
-    crypto::random_poisson_seconds embargo_duration{dandelionpp_embargo_average};
+    crypto::random_exponential_seconds embargo_duration{CRYPTONOTE_DANDELIONPP_EMBARGO_RATE};
     const auto now = std::chrono::system_clock::now();
     uint64_t next_relay = uint64_t{std::numeric_limits<time_t>::max()};
 
@@ -911,7 +910,12 @@ namespace cryptonote
 
           if (meta.dandelionpp_stem)
           {
-            meta.last_relayed_time = std::chrono::system_clock::to_time_t(now + embargo_duration());
+            // if `embargo_duration() == 0`, the next `on_idle()` will broadcast
+            // the tx.
+            meta.last_relayed_time =
+              std::chrono::system_clock::to_time_t(
+                now + std::min(std::chrono::seconds{CRYPTONOTE_DANDELIONPP_EMBARGO_MAX}, embargo_duration())
+              );
             next_relay = std::min(next_relay, meta.last_relayed_time);
           }
           else

src/cryptonote_protocol/levin_notify.cpp

@@ -74,7 +74,6 @@ namespace levin
        5000 milliseconds is given, 95% of the values fall between 4859ms-5141ms
        in 1ms increments (not enough time variance). Providing 20 quarter
        seconds yields 95% of the values between 3s-7.25s in 1/4s increments. */
-    using fluff_stepsize = std::chrono::duration<std::chrono::milliseconds::rep, std::ratio<1, 4>>;
     constexpr const std::chrono::seconds fluff_average_in{CRYPTONOTE_DANDELIONPP_FLUSH_AVERAGE};
 
     /*! Bitcoin Core is using 1/2 average seconds for outgoing connections