-
Notifications
You must be signed in to change notification settings - Fork 7
feat(payments): Update payment docs to be consistent with upcoming release #161
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: litt3 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Leaving some nits
| bandwidth. EigenDA supports two flexible payment modalities: | ||
|
|
||
| 1. **On-demand Bandwidth**: Users can make pre-paid payments for occasional bandwidth usage without time limitations or throughput guarantees, and payments are recored per blob dispersal request. Charges are applied only when the request is successfully validated by the disperser server, providing flexibility for users with dynamic bandwidth requirements. | ||
| 1. **On-demand Bandwidth**: Users can make pre-paid payments for occasional bandwidth usage without time limitations or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: when I read "pre-paid payments" it reads like a reservation.
Not super sure what's the best way to rephrase, but feel like turning the sentence on its head and mentioning the "per-dispersal charges" first would make more sense (and then at the end mention that the actual accounting requires ahead of time ETH deposits)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reworded to hopefully be more clear. I decided not to talk about the payment mechanism: this is just the intro, and the details are explained below (the introduction about reservation payments also doesn't get into specifics of the mechanism)
| On-demand payments are currently supported only through the EigenDA Disperser. Since EigenDA currently lacks a consensus | ||
| mechanism, validators cannot easily coordinate to limit total on-demand throughput across the network. The disperser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe there's a more positive way to rephrase instead of "lacks a consensus mechanism"? cc @mmurrs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There could definitely be. Open to suggestions
One option would be to just not mention why on-demand is limited to the EigenLabs disperser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, we can avoid mention of the why here. Slowly making the transition to more actionable docs.
"Since EigenDA currently lacks a consensus mechanism, validators cannot easily coordinate to limit total on-demand throughput across the network. The disperser serves as the arbiter, ensuring that total network throughput doesn't exceed configured levels."
This quoted section could be excluded, with the focus on what are on-demand payments, key behavior to callout (charges upon successful dispersal) & monitoring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| During a reservation's period of applicability, a user client can send a dispersal request authenticated by an account associated with one of these reservations. Such requests are subject to a simple fixed-bin rate limiting algorithm, which helps manage the network load effectively. The fixed-bin algorithm divides time into discrete periods (called `reservationPeriod`) and assigns a bandwidth usage limit to each period. During each period, requests are accepted until the period's capacity is exhausted. This approach ensures that the network is not overloaded during any specific period. | ||
| During a reservation's period of applicability, a user client can send a dispersal request authenticated by an account | ||
| associated with one of these reservations. Such requests are subject to a leaky bucket rate limiting algorithm, which |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we mention "replicated leaky bucket"? Aka mention that this happens in parallel on all validators?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I lean toward not bringing it up. At the most basic level, it's enough to understand that a leaky bucket algorithm governs rate limiting.
If we get into this specific detail, the natural next question is: "well, how can you be sure all validators have the same local state?" IMO if people are curious about the nitty gritty specifics of how the system works under the hood, there are other better documentation sources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, high level leaky bucket is sufficient for now. Does the detail make sense to include in the spec instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is described in detail in the payments spec
| the contract stores the total payment deposited to that account (`totalDeposit`). Users should be mindful in depositing | ||
| as they cannot withdrawal or request for refunds from the current Payment Vault contract. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if you mention this elsewhere but might be good to mention that a user can query the disperser to determine its current remaining balance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's mentioned in a couple other places, but added an additional note here
Signed-off-by: litt3 <[email protected]>
Signed-off-by: litt3 <[email protected]>
| ``` | ||
|
|
||
| Client can query the disperser for their own offchain payment state, which includes the cumulative payment and the recent period usages. Clients' accountant will prioritize using reservations before using on-demand payments. | ||
| Clients can query the disperser for their own offchain cumulative payment state. Clients' payment logic prioritizes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we be more explicit here, is the point about prioritization being ~ If you have a reservation you will be charged first from this allocation/bucket/insert correct term before charging you for an on-demand payment
I know this was partially prexisting docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed these two sentences, since I believe the explanation in the following paragraphs is easier to understand, and this is a bit redundant.
TLDR; the disperser never chooses what payment type to use. Clients keep track of their payment state locally, and use that information to decide which type of payment they want to use for each dispersal
| set, providing high censorship resistance without relying on a centralized disperser. | ||
| - Only the EigenDA Disperser supports on-demand payments. The disperser tracks cumulative usage and validates against | ||
| on-chain deposits. While clients populate the cumulative payment field, the disperser validates independently by | ||
| tracking total usage per account. By using on-demand payments, users are implicitly trusting the EigenDA disperser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"users are implicitly trusting..."
Is this something we need to callout here? in general do we have to call out all assumptions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given we have the section Security Considerations, I thought it makes sense to include this, because it's a big one. IMO we should either include this point as a security consideration, or remove the section entirely.
I'd be on board with removing it entirely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm onboard with excluding the section, it's currently the only place we have callouts and it's atypical to have them in general unless we were to do it in every section
| To use the EigenDA's disperser client, as described above, the user will need either negotiate a reservation with the EigenDA team, or permissionlessly deposit tokens into the payment vault contract for the account they want to use. They will supply the corresponding private key to the client, which will be used to sign the blob dispersal requests. | ||
| To use the EigenDA disperser client, as described above, the user will need to either negotiate a reservation with the | ||
| EigenDA team, or deposit tokens permissionlessly into the payment vault contract for the account they want to use. They | ||
| will supply the corresponding private key to the client, which will be used to sign the blob dispersal requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"They
will supply the corresponding private key to the client"
Who is they referring to here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remove this whole section, since IMO there's nothing really actionable that needs to be said.
Signed-off-by: litt3 <[email protected]>
Signed-off-by: litt3 <[email protected]>
Signed-off-by: litt3 <[email protected]>
This PR should merge when the release is made that updates to the new payment system. I'm creating it now so that it can be reviewed and ready at that time.