celestiaorg / celestia-app

Celestia consensus node
https://celestiaorg.github.io/celestia-app/
Apache License 2.0
345 stars 289 forks source link

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

Open rootulp opened 10 months ago

rootulp commented 10 months ago

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 commented 8 months ago

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

evan-forbes commented 8 months ago

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

cmwaters commented 7 months ago

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

Some caveats:

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

rootulp commented 7 months ago

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 commented 7 months ago

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:

musalbas commented 7 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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