Tests: Contract testing for the procedure AddTimeData and related fixes #15052

  66 | +}
  67 | +
  68 | +BOOST_AUTO_TEST_CASE(util_UtilBuildAddress)
  69 | +{
  70 | +    CNetAddr cn1 = UtilBuildAddress(0x001, 0x001, 0x001, 0x0D2); // 1.1.1.210
  71 | +

practicalswift commented at 4:59 PM on December 29, 2018:

Drop all these empty lines surrounding single code lines. Applies throughout this PR :-)

  86 | +    BOOST_CHECK(neq);
  87 | +}
  88 | +
  89 | +BOOST_AUTO_TEST_CASE(util_AddTimeDataAlgorithmComputeOffsetWhenNewSampleCountIsGreaterEqualFiveAndUneven)
  90 | +{
  91 | +    int capacity = 20; //  can store up to 20 samples

practicalswift commented at 5:00 PM on December 29, 2018:

Should be unsigned to match what CMedianFilter expects?

Applies for all int capacity in this file.

  93 | +    std::set<CNetAddr> knownSet;
  94 | +    CMedianFilter<int64_t> offsetFilter(capacity, 0); // max size : 20 , init sample: 0
  95 | +
  96 | +
  97 | +    for (unsigned int sample = 1; sample < 10; sample++) { // precondition: at least 4 samples. (including the init sample : 0)
  98 | +        CNetAddr addr = UtilBuildAddress(0x001, 0x001, 0x001, sample);

practicalswift commented at 5:01 PM on December 29, 2018:

Make the implicit conversion from unsigned int to unsigned char explicit here.

Applies for all CNetAddr addr = UtilBuildAddress(…, sample); in this file.

 289 | @@ -290,25 +290,52 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
 290 |  // expanded to cover more corner cases of smart time logic.
 291 |  BOOST_AUTO_TEST_CASE(ComputeTimeSmart)
 292 |  {
 293 | +    // The ComputeTimeSmart function within the wallet reads the current time using the GetAdjustedTime function in timedata.
 294 | +    //
 295 | +    // This function computes the time based on the system time (which suports mocking)

practicalswift commented at 5:06 PM on December 29, 2018:

Should be “supports” :-)

 308 |  
 309 |      // Test that updating existing transaction does not change smart time.
 310 | -    BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 200, 220), 100);
 311 | +    SetMockTime(20);
 312 | +    int64_t newBlockTime = GetAdjustedTime() + 10;
 313 | +    BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, newBlockTime), clockTime); // time has not changed from the preceeding transaction

practicalswift commented at 5:07 PM on December 29, 2018:

Should be “preceding” :-)

 330 |      // New transaction should use latest entry time if higher than
 331 |      // min(block time, clock time).
 332 | -    BOOST_CHECK_EQUAL(AddTx(m_wallet, 4, 500, 390), 400);
 333 | +    SetMockTime(5);
 334 | +    newBlockTime = blockTime - 5;
 335 | +    BOOST_CHECK_EQUAL(AddTx(m_wallet, 4, newBlockTime), blockTime); // using the blocktime of the preceeding transaction

practicalswift commented at 5:07 PM on December 29, 2018:

Should be “preceding” :-)

  43 | @@ -44,20 +44,36 @@ static int64_t abs64(int64_t n)
  44 |  
  45 |  #define BITCOIN_TIMEDATA_MAX_SAMPLES 200
  46 |  
  47 | +
  48 | +static std::set<CNetAddr> setKnown;
  49 | +
  50 | +static CMedianFilter<int64_t> vTimeOffsets(BITCOIN_TIMEDATA_MAX_SAMPLES, 0);
  51 | +
  52 | +int CountOffsetSamples()

practicalswift commented at 5:08 PM on December 29, 2018:

Unused. Drop this :-)

  75 | @@ -75,4 +76,8 @@ int64_t GetTimeOffset();
  76 |  int64_t GetAdjustedTime();
  77 |  void AddTimeData(const CNetAddr& ip, int64_t nTime);
  78 |  
  79 | +/** Functions to isolate the contract in AddTimeData */
  80 | +int CountOffsetSamples();

practicalswift commented at 5:08 PM on December 29, 2018:

Unused :-)

 299 | +
 300 | +
 301 |      // New transaction should use clock time if lower than block time.
 302 | -    BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 100, 120), 100);
 303 | +    SetMockTime(10);
 304 | +    int64_t clockTime = GetAdjustedTime(); // time + time offset (unfortunately, from a statefull data structure)

practicalswift commented at 5:08 PM on December 29, 2018:

Should be “stateful” :-)

 143 | +BOOST_AUTO_TEST_CASE(util_AddTimeDataAlgorithmComputeOffsetWithMedianWithinBounds)
 144 | +{
 145 | +    int capacity = 10;
 146 | +    int64_t offset = 0;
 147 | +    std::set<CNetAddr> knownSet;
 148 | +    CMedianFilter<int64_t> offsetFilter(capacity, 0); // max size : 10 , init samplee: 0

practicalswift commented at 5:09 PM on December 29, 2018:

Should be “sample”?

  99 | +        AddTimeDataAlgorithm(addr, sample, knownSet, offsetFilter, offset); // 1.1.1.[1,2,3],  offsetSample = [1,2,3]
 100 | +    }
 101 | +
 102 | +    BOOST_CHECK(offset != 0); // offset has changed from the initial value
 103 | +
 104 | +    assert(offsetFilter.size() == 10); // next sample will be the 11th (uneven) and will trigger a new computation of the offset

practicalswift commented at 5:11 PM on December 29, 2018:

Why assert(…) and not BOOST_CHECK(…)? Applies throughout this PR.

 128 | +
 129 | +    BOOST_CHECK(offset != 0); // offset has changed from the initial value
 130 | +
 131 | +    assert(offsetFilter.size() == 9); // next sample will be the 10th (even) and will *not* trigger a new computation of the offset
 132 | +
 133 | +

practicalswift commented at 5:11 PM on December 29, 2018:

Drop one of the newlines. Applies for all double newlines in this PR.

 170 | +    CMedianFilter<int64_t> offsetFilter(capacity, 0); // max size : 10 , init sample: 0
 171 | +
 172 | +    for (int sample = 1; sample < 4; sample++) {                                                     // precondition: 4 samples, all outside bounds
 173 | +        CNetAddr addr = UtilBuildAddress(0x001, 0x001, 0x001, sample);                               // 1.1.1.[1,2,3]
 174 | +        AddTimeDataAlgorithm(addr, 2 * DEFAULT_MAX_TIME_ADJUSTMENT, knownSet, offsetFilter, offset); // offsetSample  = 2 * DEFAULT_MAX_TIME_ADJUSTMENT (out of bounds)
 175 | +

practicalswift commented at 5:12 PM on December 29, 2018:

Drop empty line at end of block.