diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7da33931b..bbd4b48ed 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -262,7 +262,7 @@ jobs: git diff --name-only --exit-code && echo "✅ Generated proto matches committed proto" || (echo "❌ Generated proto differs from committed proto, run \`make proto -B\` and commit the result" >&2 && exit 1) make test - # Verify go sdk unit tests + # Verify go sdk unit/fuzz tests sdk_vaa: runs-on: ubuntu-20.04 steps: @@ -270,8 +270,8 @@ jobs: - uses: actions/setup-go@v2 with: go-version: "1.20.4" - - run: cd sdk/vaa && go test - + - run: cd sdk/vaa && go test && go test -v -fuzz FuzzCalculateQuorum -run FuzzCalculateQuorum -fuzztime 15s + # Run Go linters node-lint: # The linter is slow enough that we want to run it on the self-hosted runner diff --git a/sdk/vaa/quorum.go b/sdk/vaa/quorum.go index 4d2526755..5621670f5 100644 --- a/sdk/vaa/quorum.go +++ b/sdk/vaa/quorum.go @@ -5,8 +5,18 @@ package vaa // The canonical source is the calculation in the contracts (solana/bridge/src/processor.rs and // ethereum/contracts/Wormhole.sol), and this needs to match the implementation in the contracts. func CalculateQuorum(numGuardians int) int { + // A safety check to avoid caller from ever supplying a negative + // number, because we're dealing with signed integers if numGuardians < 0 { panic("Invalid numGuardians is less than zero") } + + // The goal here is to acheive a 2/3 quorum, but since we're + // dividing on int, we need to +1 to avoid the rounding down + // effect of integer division + // + // For example sake, 5 / 2 == 2, but really that's not an + // effective 2/3 quorum, so we add 1 for safety to get to 3 + // return ((numGuardians * 2) / 3) + 1 } diff --git a/sdk/vaa/quorum_test.go b/sdk/vaa/quorum_test.go index 1d6381209..b9531fcbc 100644 --- a/sdk/vaa/quorum_test.go +++ b/sdk/vaa/quorum_test.go @@ -55,3 +55,39 @@ func TestCalculateQuorum(t *testing.T) { }) } } + +func FuzzCalculateQuorum(f *testing.F) { + // Add examples to our fuzz corpus + f.Add(1) + f.Add(2) + f.Add(4) + f.Add(8) + f.Add(16) + f.Add(32) + f.Add(64) + f.Add(128) + f.Fuzz(func(t *testing.T, numGuardians int) { + // These are known cases, which the implementation will panic on and/or we have explicit + // unit-test coverage of above, so we can safely ignore it in our fuzz testing + if numGuardians <= 0 { + t.Skip() + } + + // Let's determine how many guardians are needed for quorum + num := CalculateQuorum(numGuardians) + + // Let's always be sure that there are enough guardians to maintain quorum + assert.LessOrEqual(t, num, numGuardians, "fuzz violation: quorum cannot be acheived because we require more guardians than we have") + + // Let's always be sure that num is never zero + assert.NotZero(t, num, "fuzz violation: no guardians are required to acheive quorum") + + var floorFloat float64 = 0.66666666666666666 + numGuardiansFloat := float64(numGuardians) + numFloat := float64(num) + actualFloat := numFloat / numGuardiansFloat + + // Let's always make sure that the int division does not violate the floor of our float division + assert.Greater(t, actualFloat, floorFloat, "fuzz violation: quorum has dropped below 2/3rds threshold") + }) +}