Skip to content

Conversation

samlaf
Copy link
Collaborator

@samlaf samlaf commented Jul 2, 2025

Bug found by @cody-littley. We really need the correctness_tests running in CI to prevent these kinds of bugs.

legacyReply.Reservation can be nil, which causes a panic when it is dereferenced. I can reproduce this by running TestDispersalWithInvalidSignature() in correctness_test.go.

Why are these changes needed?

Checks

  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

legacyReply.Reservation can be nil, which causes a panic when it is dereferenced. I can reproduce this by running TestDispersalWithInvalidSignature() in correctness_test.go.
@samlaf samlaf requested a review from a team as a code owner July 2, 2025 14:01
@samlaf samlaf requested a review from cody-littley July 2, 2025 14:01
cody-littley
cody-littley previously approved these changes Jul 2, 2025
cody-littley
cody-littley previously approved these changes Jul 2, 2025
litt3
litt3 previously approved these changes Jul 2, 2025
@samlaf samlaf dismissed stale reviews from litt3 and cody-littley via 32309fe July 2, 2025 14:48
@samlaf samlaf merged commit a5acd93 into master Jul 2, 2025
14 of 15 checks passed
@samlaf samlaf deleted the fix--disperser-client-nil-pointer-bug branch July 2, 2025 15:26
samlaf added a commit that referenced this pull request Jul 11, 2025
samlaf added a commit that referenced this pull request Jul 16, 2025
* Revert "refactor: account ledger interface incorporates debit slip (#1732)"

This reverts commit b4cf3d9.

* Revert "fix: period record circular wrapping refresh (#1731)"

This reverts commit 1091f46.

* Revert "fix: disperser_client v2 nil ptr bug (#1710)"

This reverts commit a5acd93.

* Revert "fix: disperser_client v2 should use default onDemandQuorums (#1700)"

This reverts commit 005e0a6.

* Revert "refactor: unify accountant debit slip (#1695)"

This reverts commit a9c0868.

* Revert "feat: AccountantLedger interface for next step unifications (#1694)"

This reverts commit 354e42f.

* Revert "feat: extract generic payment logic (#1693)"

This reverts commit 1003b98.

* Revert "fix: make disperser client backwards compatible (#1686)"

This reverts commit 26c612f.

* Revert "fix: payment state api doesn't fail at zero valued reservations/ondemand (#1682)"

This reverts commit 2031c1d.

* Revert "refactor: quorum specific metrics for metered bytes (#1668)"

This reverts commit 7ac688d.

* Revert "feat: GetPaymentStateForAllQuorums api impl (#1664)"

This reverts commit ed89588.

* Revert "refactor: core meterer consolidation (#1663)"

This reverts commit 319a865.

* Revert "refactor: onchain state interface (#1662)"

This reverts commit d795dfa.

* Revert "refactor: core meterer period record module (#1661)"

This reverts commit afbef90.

* Revert "refactor: payment common functions (#1653)"

This reverts commit 36a3d33.

* Revert "fix: period index calculation (#1641)"

This reverts commit c229748.

* Revert "refactor: use generic column name (#1626)"

This reverts commit 4d1994e.

* Revert "feat: payment onchain state interface (#1625)"

This reverts commit 61e46ac.

* Revert "feat: offchain quorum period record getter (#1620)"

This reverts commit a309139.

* Revert "feat: protobuf payment state api quorum specific variant (#1613)"

This reverts commit 58acdfd.

* Revert "fix: nil assignment to new account (#1612)"

This reverts commit bcbf9f5.

* Revert "feat: offchain batch writes (#1580)"

This reverts commit ce4d2f5.

* Revert "feat: validator authenticate on-demand request by disperser key (#1539)"

This reverts commit f545b16.

* fix: issues after series of reverts (scary)

* chore: add back ErrZeroSymbols error

This error was likely removed by mistake during a revert conflict fix from one of the previous set of reverted PRs.

* style: fix lint

* test: fix accountant_test hardcoded error string

* Revert "Revert "fix: period index calculation (#1641)""

This reverts commit b8767ae.
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.

3 participants