Fix ErgoStateContextSpec Sporadic Failures

[stenolog]

closes #2114

[stenolog]

@kushti , this should be ready to go in.

I can squash from my side, or you could enable UI squashing

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests

and do it from your side.

[kushti]

Good catch!

For

var imvValue = extensionKvGen(Extension.FieldKeySize, Extension.FieldValueMaxSize + 1).sample.get
    while (imvValue._1.head == 0) {
        imvValue = extensionKvGen(Extension.FieldKeySize, Extension.FieldValueMaxSize + 1).sample.get
    }
    sc.appendFullBlock(fbWithFields(imvValue +: oldFields)) shouldBe 'failure

the issue is that extensionKvGen is defined as

  def extensionKvGen(keySize: Int, valuesSize: Int): Gen[(Array[Byte], Array[Byte])] = for {
    key <- genSecureBoundedBytes(keySize, keySize)
    value <- if (key.head == 0) genSecureBoundedBytes(4, 4) else genSecureBoundedBytes(valuesSize, valuesSize)
  } yield (key, value)

so when imvValue._1.head == 0, value is 4 bytes long, not 65 (Extension.FieldValueMaxSize + 1)

I think dedicated generator should be used for the check

[kushti]

for

var validMKV = extensionKvGen(Extension.FieldKeySize, Extension.FieldValueMaxSize).sample.get
while (validMKV._1.head != 1) {
  validMKV = extensionKvGen(Extension.FieldKeySize, Extension.FieldValueMaxSize).sample.get
}

generation of test data here is good, as for validMKV._1.head == 1 interlinks would be packed improperly, and interlinks packing/unpacking is tested in another test

[stenolog]

while (validMKV._1.head != 1) {

It's validMKV._1.head == 1

This fails, too, there are two different sporadic failures.

[stenolog]

I think dedicated generator should be used for the check

My thought, too, see https://github.com/ergoplatform/ergo/issues/2118

"Possibly introduce extensionKvGenValidated()"

This PR/iteration can/should go in as-is, fixes the 2 different sporadic test-failures.

Next step/iteration/PR would be to clarify/rewrite/document

• https://github.com/ergoplatform/ergo/issues/2117
• https://github.com/ergoplatform/ergo/issues/2118

[kushti]

@stenolog remove

printf("\r%d05 ", i)

and for loop added, not related to fixes

[stenolog]

remove

printf("\r%d05 ", i)

this can be removed, yes.

and for loop added, not related to fixes

This is related because the test needs to loop through the randomness to catch any future regressions.

Without it, new similar bugs can slip through undetected.

[stenolog]

and for loop added, not related to fixes

I've removed (out-commented) it.

[stenolog]

As your goal is

My goal was to detect the sporadic bug (done) and fix it (done).

Further work was planned within

• https://github.com/ergoplatform/ergo/issues/2117
• https://github.com/ergoplatform/ergo/issues/2118

which you've closed prematurely.

It is very(!) difficult to produce quality results here (waiting for nearly a week, then change-request. Waiting 5 days, change-request).

Writing this just to file my objections.

I'll provide a "quick&dirty" generator, not because I agree, but because I'm forced to.

[stenolog]

I'll provide a "quick&dirty" generator,

I've problems with this (conditionals within generator functions)

Problem

to avoid this messy construct

    var validMKV = extensionKvGen(Extension.FieldKeySize, Extension.FieldValueMaxSize).sample.get
    while (validMKV._1.head == 1) {
      validMKV = extensionKvGen(Extension.FieldKeySize, Extension.FieldValueMaxSize).sample.get
    }

a dedicated generator should be provided.

Problem is that conditionals within the generator seem not trivial in scala (found no docs/articles)

  def extensionKvGen(keySize: Int, valuesSize: Int): Gen[(Array[Byte], Array[Byte])] = for {
   # key.head should NOT be 1
    key <- genSecureBoundedBytes(keySize, keySize)
    value <- genSecureBoundedBytes(valuesSize, valuesSize)
  } yield (key, value)

Any suggestion on how to check for key == 1 within the generator, to repeat the generation?

[stenolog]

Any suggestion on how to check for key == 1 within the generator, to repeat the generation?

from https://discord.com/channels/668903786361651200/669989266478202917/1214238632383217715

def extensionKvGen(keySize: Int, valueSize: Int): Gen[(Array[Byte], Array[Byte])] = {
  def genSecureBoundedBytes(size: Int): Gen[Array[Byte]] = {
    // Your existing implementation to generate Array[Byte] of given size.
  }

  def keyGen: Gen[Array[Byte]] = genSecureBoundedBytes(keySize).flatMap { key =>
    if (key.headOption.contains(1.toByte)) keyGen else Gen.const(key)
  }

  for {
    key <- keyGen
    value <- genSecureBoundedBytes(valueSize)
  } yield (key, value)
}

slightly modified final code:

  def extensionKvGenValidMKV(keySize: Int, valueSize: Int): Gen[(Array[Byte], Array[Byte])] = {
    def genSecureBoundedBytesWrapper(minSize: Int, maxSize: Int): Gen[Array[Byte]]= {
      genSecureBoundedBytes(minSize, maxSize)
    }

    def keyGen: Gen[Array[Byte]] = genSecureBoundedBytesWrapper(keySize, keySize).flatMap { key =>
      if (key.headOption.contains(1.toByte)) keyGen else Gen.const(key)
    }

    for {
      key <- keyGen
      value <- genSecureBoundedBytesWrapper(valueSize, valueSize)
    } yield (key, value)
  }

EDIT: one more iteration (in the morning with a clear mind)

  def extensionKvGenValidMKV(keySize: Int, valueSize: Int): Gen[(Array[Byte], Array[Byte])] = {
    def keyGenHeadNotOne: Gen[Array[Byte]] = genSecureBoundedBytes(keySize, keySize).flatMap { key =>
      if (key.headOption.contains(1.toByte)) keyGenHeadNotOne else Gen.const(key)
    }

    for {
      key <- keyGenHeadNotOne
      value <- genSecureBoundedBytes(valueSize, valueSize)
    } yield (key, value)
  }

[stenolog]

@kushti , in order to speed things up, please feel free to modify/extend this PR as you see fit.

I'm fine with it, even if stenolog is not shown as the committer.

As a sidenote: this (sporadic test bug) PR remained completely unprocessed, despite several pings:

• https://github.com/ergoplatform/ergo/pull/2121