Skip to content

Conversation

hopeyen
Copy link
Contributor

@hopeyen hopeyen commented Jun 25, 2025

Why are these changes needed?

Added an error type of ErrPaymentDoesNotExist

If a reservation has zero on all the fields or ondemand account has zero balance, we treat that payment as it doesn't exist.
Within 4f0684f, we updated the behavior from allow zero values as fallback to all fields, to returning error when there's an error from the onchain payment state.
It is too strict for users who are not using on-demand, because ondemand being zero was considered an error by the eth reader. We update to make eth reader return a specific error type, and allow this error to use zero value when constructing payment states.

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 :(

@hopeyen hopeyen marked this pull request as ready for review June 25, 2025 02:06
"math/big"
"slices"

"github.com/Layr-Labs/eigenda/core"
"github.com/pingcap/errors"
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this import for? Can we use the standard library one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this package is also used in eth writer and reader. i think mainly for adding context to the error

@@ -15,6 +15,9 @@ import (

var (
maxNumberOfQuorums = 192

// ErrPaymentDoesNotExist is returned when a reservation/deposit does not exist (is zero)
ErrPaymentDoesNotExist = errors.New("payment does not exist")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this specific to reservations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, also ondemand. do you want them separate?

@hopeyen hopeyen requested a review from anupsv June 25, 2025 02:32
@hopeyen hopeyen merged commit 2031c1d into master Jun 25, 2025
15 of 16 checks passed
@hopeyen hopeyen deleted the hope/pmt-api-error-path branch June 25, 2025 17:35
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