Fix set/unset bugs with fuzzer test

exp/lighthorizon/index/types/bitmap.go

@@ -2,9 +2,12 @@ package index
 
 import (
  "bytes"
+ "fmt"
  "io"
+ "strings"
  "sync"
 
+ "github.com/stellar/go/support/ordered"
  "github.com/stellar/go/xdr"
 )
 
@@ -102,20 +105,15 @@ func (i *BitmapIndex) setActive(index uint32) error {
  // Expand the bitmap
  if index < i.rangeFirstBit() {
  // ...to the left
- c := (i.rangeFirstBit() - index) / 8
- if (i.rangeFirstBit()-index)%8 != 0 {
- c++
- }
- newBytes := make([]byte, c)
+ newBytes := make([]byte, distance(index, i.rangeFirstBit()))
  i.bitmap = append(newBytes, i.bitmap...)
-
  b := bitShiftLeft(index)
  i.bitmap[0] = i.bitmap[0] | b
 
  i.firstBit = index
  } else if index > i.rangeLastBit() {
  // ... to the right
- newBytes := make([]byte, (index-i.rangeLastBit())/8+1)
+ newBytes := make([]byte, distance(i.rangeLastBit(), index))
  i.bitmap = append(i.bitmap, newBytes...)
  b := bitShiftLeft(index)
  loc := (index - i.rangeFirstBit()) / 8
@@ -129,6 +127,12 @@ func (i *BitmapIndex) setActive(index uint32) error {
  return nil
 }
 
+// distance returns how many bytes occur between the two given indices. Note
+// that j >= i, otherwise the result will be negative.
+func distance(i, j uint32) int {
+ return (int(j)-1)/8 - (int(i)-1)/8
+}
+
 func (i *BitmapIndex) setInactive(index uint32) error {
  // Is this index even active in the first place?
  if i.firstBit == 0 || index < i.rangeFirstBit() || index > i.rangeLastBit() {
@@ -152,54 +156,52 @@ func (i *BitmapIndex) setInactive(index uint32) error {
  return err
  } else {
  // Trim all (now-)empty bytes off the front.
- i.bitmap = i.bitmap[int(nextBit/8)-int(i.firstBit/8):]
+ diff := distance(i.firstBit, nextBit)
+ i.bitmap = i.bitmap[diff:]
  i.firstBit = nextBit
  }
  } else if int(loc) == len(i.bitmap)-1 {
- if i.bitmap[loc] == 0 {
- i.bitmap = i.bitmap[:loc-1] // trim last (now-empty) byte
+ idx := -1
 
+ if i.bitmap[loc] == 0 {
  // find the latest non-empty byte, to set as the new "end"
- for j := len(i.bitmap) - 1; j >= 0; j-- {
- if i.bitmap[j] == 0 {
- continue
- }
-
- offset, _ := maxBitAfter(i.bitmap[j], 0)
- byteOffset := uint32(len(i.bitmap)-1) * 8
- i.lastBit = i.rangeFirstBit() + byteOffset + offset
- i.bitmap = i.bitmap[:j+1]
- break
+ j := len(i.bitmap) - 1
+ for i.bitmap[j] == 0 {
+ j--
  }
- } else if i.lastBit == index {
- // Otherwise, do we need to adjust the range? Imagine we had
- // 0b0011_0100 and we unset the last active bit.
- // ^
- // Then, we need to adjust our internal lastBit tracker to represent
- // the ^ bit above. This means finding the first previous set bit.
 
+ i.bitmap = i.bitmap[:j+1]
+ idx = 8
+ } else if i.lastBit == index {
  // Get the "bit number" of the last active bit (i.e. the one we just
  // turned off) to mark the starting point for the search.
- idx := uint32(1)
+ idx = 8
  if index%8 != 0 {
- idx = 8 - (index % 8)
+ idx = int(index % 8)
  }
+ }
 
+ // Do we need to adjust the range? Imagine we had 0b0011_0100 and we
+ // unset the last active bit.
+ // ^
+ // Then, we need to adjust our internal lastBit tracker to represent the
+ // ^ bit above. This means finding the first previous set bit.
+ if idx > -1 {
+ l := uint32(len(i.bitmap) - 1)
  // Imagine we had 0b0011_0100 and we unset the last active bit.
  // ^
  // Then, we need to adjust our internal lastBit tracker to represent
  // the ^ bit above. This means finding the first previous set bit.
  j, ok := int(idx), false
  for ; j >= 0 && !ok; j-- {
- _, ok = maxBitAfter(i.bitmap[loc], uint32(j))
+ _, ok = maxBitAfter(i.bitmap[l], uint32(j))
  }
 
  // We know from the earlier conditional that *some* bit is set, so
  // we know that j represents the index of the bit that's the new
  // "last active" bit.
  firstByte := i.rangeFirstBit()
- byteOffset := 8 * uint32(len(i.bitmap)-1)
- i.lastBit = firstByte + byteOffset + uint32(j) + 1
+ i.lastBit = firstByte + (l * 8) + uint32(j) + 1
  }
  }
 
@@ -343,3 +345,24 @@ func (i *BitmapIndex) Buffer() *bytes.Buffer {
 func (i *BitmapIndex) Flush() []byte {
  return i.Buffer().Bytes()
 }
+
+// DebugCompare returns a string that compares this bitmap to another bitmap
+// byte-by-byte in binary form as two columns.
+func (i *BitmapIndex) DebugCompare(j *BitmapIndex) string {
+ output := make([]string, ordered.Max(len(i.bitmap), len(j.bitmap)))
+ for n := 0; n < len(output); n++ {
+ if n < len(i.bitmap) {
+ output[n] += fmt.Sprintf("%08b", i.bitmap[n])
+ } else {
+ output[n] += " "
+ }
+
+ output[n] += " | "
+
+ if n < len(j.bitmap) {
+ output[n] += fmt.Sprintf("%08b", j.bitmap[n])
+ }
+ }
+
+ return strings.Join(output, "\n")
+}

exp/lighthorizon/index/types/bitmap_test.go

@@ -3,6 +3,7 @@ package index
 import (
  "fmt"
  "io"
+ "math/rand"
  "testing"
 
  "github.com/stretchr/testify/assert"
@@ -194,6 +195,39 @@ func TestSetInactive(t *testing.T) {
  assert.Equal(t, []byte{0b0100_0000, 0b0010_0000}, index.bitmap)
  assert.EqualValues(t, 2, index.firstBit)
  assert.EqualValues(t, 2+9, index.lastBit)
+
+ index.setInactive(1) // should be a no-op
+ assert.Equal(t, []byte{0b0100_0000, 0b0010_0000}, index.bitmap)
+ assert.EqualValues(t, 2, index.firstBit)
+ assert.EqualValues(t, 2+9, index.lastBit)
+}
+
+// TestFuzzerSetInactive attempt to fuzz random bits into two bitmap sets, one
+// by addition, and one by subtraction - then, it compares the outcome.
+func TestFuzzySetUnset(t *testing.T) {
+ permLen := uint32(128) // should be a multiple of 8
+ setBitsCount := permLen / 2
+
+ for n := 0; n < 10_000; n++ {
+ randBits := rand.Perm(int(permLen))
+ setBits := randBits[:setBitsCount]
+ clearBits := randBits[setBitsCount:]
+
+ // set all first, then clear the others
+ clearBitmap := &BitmapIndex{}
+ for i := uint32(1); i <= permLen; i++ {
+ clearBitmap.setActive(i)
+ }
+
+ setBitmap := &BitmapIndex{}
+ for i := range setBits {
+ setBitmap.setActive(uint32(setBits[i]) + 1)
+ clearBitmap.setInactive(uint32(clearBits[i]) + 1)
+ }
+
+ require.Equalf(t, setBitmap, clearBitmap,
+ "bitmaps aren't equal:\n%s", setBitmap.DebugCompare(clearBitmap))
+ }
 }
 
 func TestNextActive(t *testing.T) {