Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ErgoStateContextSpec Sporadic Failures #2116

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

stenolog
Copy link
Contributor

@stenolog stenolog commented Feb 21, 2024

closes #2114

@stenolog stenolog mentioned this pull request Feb 21, 2024
3 tasks
@stenolog stenolog changed the title [DO NOT MERGE] reproducer for issue #2114 Fix ErgoStateContextSpec Sporadic Failures Feb 23, 2024
@stenolog

This comment was marked as outdated.

@kushti kushti changed the base branch from master to v5.0.21 February 26, 2024 16:34
@kushti
Copy link
Member

kushti commented Feb 26, 2024

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
Copy link
Member

kushti commented Feb 26, 2024

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
Copy link
Contributor Author

stenolog commented Feb 26, 2024

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

It's validMKV._1.head == 1

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

@stenolog

This comment was marked as outdated.

@kushti
Copy link
Member

kushti commented Feb 26, 2024

@stenolog remove

printf("\r%d05 ", i)

and for loop added, not related to fixes

@stenolog

This comment was marked as outdated.

@stenolog stenolog force-pushed the i2114-repro branch 2 times, most recently from c27ea5b to 7c0b17f Compare February 28, 2024 13:18
@stenolog

This comment was marked as outdated.

@stenolog stenolog force-pushed the i2114-repro branch 2 times, most recently from 8d933a6 to f558fe2 Compare March 2, 2024 05:29
Copy link
Member

@kushti kushti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think custom generators should be used , reiterating over extensionKvGen is messy . As your goal is to avoid specific key spaces anyway, better to do that directly

@stenolog

This comment was marked as resolved.

@stenolog

This comment was marked as resolved.

@stenolog

This comment was marked as resolved.

fix test 'valid application of correct extension'

fix test 'validation of field value sizes'
@stenolog

This comment was marked as outdated.

@kushti kushti merged commit bd9db19 into ergoplatform:v5.0.21 Mar 5, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ErgoStateContextSpec is unstable
2 participants