Skip to content
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

Consider making the gas cost constant for PFB signature verification, tx size, read access to accounts #3011

Open
rootulp opened this issue Jan 12, 2024 · 11 comments
Labels
ice-box this label is automatically applied to all issues. it is removed after starting work proposal item is not yet actionable and is suggesting a change that must first be agreed upon

Comments

@rootulp
Copy link
Collaborator

rootulp commented Jan 12, 2024

Context

The "fixed cost" is an approximation of the gas consumed by operations outside the function GasToConsume (for example, signature verification, tx size, read access to accounts), which has a default value of 65,000.

https://celestiaorg.github.io/celestia-app/specs/resource_pricing.html#estimating-pfb-cost

Problem

The current implementation uses the default Cosmos SDK gas metering mechanism to account for the gas costs of PFBs. This mechanism isn't entirely predictable because it depends on state reads / writes. For example see this image which has pie chart segments for ReadPerByte, WritePerByte, etc. These are deterministic but it's not immeidately obvious how a user determines them. Can they be determined offline if a user doesn't have access to the current state?

Proposal

We could simplify the gas cost estimation for a PFB if the gas cost for PFBs did not depend on state reads / state writes. To make it more predictable we may consider:

  1. Charging a fixed cost of 65,000 gas for all the portions of gas accounting that are currently variable (i.e. signature verification, tx size, read access to accounts, etc.). This approach is basically updating the implementation to actually charge exactly what gas fee calculation describes.

Pros: more predictable for users
Cons: A user could craft a PFB tx that is significantly more gas expensive under the current implementation and only be charged 65,000 gas in the proposed implementation. For example, submitting PFBs from a multisig account with 7 signatures.

Motivated by a discussion with @bpiv400

@rootulp rootulp added the proposal item is not yet actionable and is suggesting a change that must first be agreed upon label Jan 12, 2024
@rootulp
Copy link
Collaborator Author

rootulp commented Mar 1, 2024

Closing as won't do because it doesn't seem like there is interest in this proposal.

@rootulp rootulp closed this as not planned Won't fix, can't repro, duplicate, stale Mar 1, 2024
@evan-forbes
Copy link
Member

I like it, imo I think we should keep it in the ice-box as a reference point

@evan-forbes evan-forbes added the ice-box this label is automatically applied to all issues. it is removed after starting work label Mar 1, 2024
@rootulp rootulp reopened this Mar 1, 2024
@cmwaters
Copy link
Contributor

cmwaters commented Apr 12, 2024

I've been thinking about this a bit more and am increasingly in favour of implementing this.

Some caveats:

  • memo's should not be allowed (as this can vary considerably)
  • tx must have only a single PFB message (which is currently imposed but might not always be)

Furthermore, I think both the fixed component and the variable component should become versioned parameters (i.e. not gov params neither completely hardcoded)

@rootulp
Copy link
Collaborator Author

rootulp commented Apr 12, 2024

I like that idea @cmwaters! We could collect data on how many PFB transactions are "vanilla" (i.e. no memo, a single signer). I expect that to be the the most frequent type of PFB transaction. Once confirmed, then we could proceed on implementing this issue strictly for "vanilla" PFB transactions.

If the PFB transaction has a memo, has multiple signers, or anything else "non-vanilla" that could drastically increase the gas cost (under the SDK metering) then the state machine falls back on using SDK metering to calculate gas for that transaction.

@cmwaters
Copy link
Contributor

Yeah there are a bunch of other options that affect the gas used that I think could simply be amortized in favour of better UX:

  • Setting a fee granter
  • Setting a timeout
  • Setting a tip (I don't know how this would be used in this case)

@musalbas
Copy link
Member

This mechanism isn't entirely predictable because it depends on state reads / writes. For example see this image which has pie chart segments for ReadPerByte, WritePerByte, etc. These are deterministic but it's not immeidately obvious how a user determines them. Can they be determined offline if a user doesn't have access to the current state?

  1. Transaction fees should be deterministic (like in Bitcoin) given Celestia doesn't have shared smart contract state. If it doesn't that should be fixed.
  2. Assuming transaction fees are deterministic, then it should be trivial to calculate the gas cost.

Shouldn't (1) and (2) be addressed first as the clean fix? Changing how gas pricing works without knowing the above, and introducing DoS vectors, seems like the dirtier fix.

@nashqueue
Copy link
Member

nashqueue commented Apr 16, 2024

Transaction fees should be deterministic (like in Bitcoin) given Celestia doesn't have shared smart contract state. If it doesn't that should be fixed.
Assuming transaction fees are deterministic, then it should be trivial to calculate the gas cost.

Shouldn't (1) and (2) be addressed first as the clean fix? Changing how gas pricing works without knowing the above, and introducing DoS vectors, seems like the dirtier fix.

From my understanding the one thing that is hard to calculate up front is how much state will be accessed. I think this is because if the IVAL tree changes the amount of writes and reads changes. If that's not the case than everything should be still deterministic no matter the state of Celestia.

On another note I would isolate all the things that can be calculated easily up front. One thing would be the the amount of bytes used by a PFB transaction. Otherwise you can create a PFB that submits 30 blobs which has a big transaction size but only pay for one PFB transaction.

This in turn could though be a feature to incentivize multiple blobs per PFB but could be abused so a function in between might be a good compromise.

@musalbas
Copy link
Member

musalbas commented Apr 18, 2024

I think this is because if the IVAL tree changes the amount of writes and reads changes.

I see if this is the case, then yes it does make sense to change the gas pricing model.

But are we sure we want to do this as a one-off hack for single PFBs rather than doing a more clean revisit of gas pricing in general? One-off hack could be fine to relieve dev UX issues short term if the latter is a bigger stream work, but I think it at least should be discussed perhaps so that we're sure.

@adlerjohn
Copy link
Member

adlerjohn commented Apr 22, 2024

A user could craft a PFB tx that is significantly more gas expensive under the current implementation and only be charged 65,000 gas in the proposed implementation

Without a way to address this, the proposal is DOA IMO.

Addressed with the suggestion of limiting to simple PBFs transactions with only a single message:

memo's should not be allowed (as this can vary considerably)
tx must have only a single PFB message (which is currently imposed but might not always be)

For this workaround, I agree it's not clean as per

But are we sure we want to do this as a one-off hack for single PFBs rather than doing a more clean revisit of gas pricing in general? One-off hack could be fine to relieve dev UX issues short term if the latter is a bigger stream work

IMO there are other ways to improve UX without special-casing anything. For example, a more reliable gas estimation API. While not stateless due to the state tree, it shouldn't change so quickly that a live query for gas estimation becomes completely off any time soon, e.g. https://forum.celestia.org/t/cip-standardised-gas-and-pricing-estimation-interface/1621

@cmwaters
Copy link
Contributor

From this conversation, something I would then propose is that all gas-related parameters be versioned constants. That means they are no longer changeable by governance and only can modify from one version to another. That will give users a more simpler way to estimate gas, as they no longer have to query for these parameters before submitting their transaction.

@cmwaters
Copy link
Contributor

As to having a special case for PFBs. I also don't think that estimating the exact gas amount is too much of an issue. Given currently how cheap DA is, users should be okay with having an overestimation valuing reliable delivery over the slight overhead in cost

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ice-box this label is automatically applied to all issues. it is removed after starting work proposal item is not yet actionable and is suggesting a change that must first be agreed upon
Projects
None yet
Development

No branches or pull requests

6 participants