CIP: Refund unspent gas

Celestia Improvement Proposals (CIPs)
1

This post is to discuss docs: CIP for refund unspent gas by rootulp · Pull Request #30 · celestiaorg/CIPs · GitHub

4 Likes
2

I see this CIP was withdrawn. I think that means it should be updated with a withdrawal-reason, per CIPs/cips/cip-1.md at main · celestiaorg/CIPs · GitHub ?

Re the DoS attacks described, we are already using Prepare/Process proposal, so we could additionally execute the txs there and set the gas limit for the block based on the actual gas used during execution. Is this what’s considered too complicated as per https://github.com/celestiaorg/celestia-app/pull/2887#issuecomment-1881566122 or was there something else? I would expect this to be relatively straightforward but maybe introduce some additional expense/latency into block production.

As for gas metering during the refund and this attack:

an attacker may be able to craft a transaction that performs a large amount of computation while executing the unspent gas refund posthandler

Do you have an example of how this would occur? I would have expected this computation to be quite simple, and the cost to be priced into the base gas costs for txs, so not something an attacker could exploit, but I might be missing something.

One thing to note about gas refunds is that they may make the system more susceptible to mis priced gas costs, vs being required to pay the whole fee. Fortunately I imagine Celestias gas pricing is relatively simple, though it does inherit elements from other modules (eg. bank, staking, ibc) so it might be worth reviewing relative gas costs across the whole app if we’re going to reconsider doing the refund.

2 Likes
3

Yes, I’ve made a PR to update this. Thanks for catching @ebuchman!

1 Like
4

Thanks josh. Would still appreciate some clarity as to why its deemed too complex. Is it just the desire to avoid executing txs during Prepare/ProcessProposal?

2 Likes
5

Thanks for pointing out the withdrawal-reason! Added in this PR.

Is this what’s considered too complicated as per https://github.com/celestiaorg/celestia-app/pull/2887#issuecomment-1881566122 or was there something else?

So we had mitigation idea 1 implemented in https://github.com/celestiaorg/celestia-app/pull/2887 but that was deemed too complicated.

We briefly explored mitigation idea 2 but as you mentioned, that would involve some flavor of “immediate” execution in Prepare/Process proposal. “Immediate” in this context means that the txs would be executed in Prepare/Process proposal to determine how much gas was used (to account for the impact to the block gas limit) but the state root for a block would not be updated to account for the state changes in those transactions. In other words, the current deferred execution model will still exist.

We ultimately decided to not pursue mitigation idea 2 because implementing this flavor of “immediate” execution was larger in scope than we anticipated.

Do you have an example of how this would occur?

I don’t have a concrete example. This line was included in the CIP to draw attention to the fact that the proposed implementation charged a fixed cost for the refund gas operation. The fixed cost may not accurately reflect what the Cosmos SDK default gas metering mechanism would’ve charged for the refund operation in all scenarios.

3 Likes
6

Cool thanks. Agree that idea 1 is too complicated. How important is it to address the deferred execution ? Should we be thinking about a separate CIP for that? Are there specific things it would unlock for Celstia in the short term?

Solving the deferred execution overall in tendermint is an old problem, see eg.

https://github.com/tendermint/tendermint/issues/7898#issuecomment-721407590
(I cant seem to post this link for some reason)

But now with ABCI++ there’s probably a way to add a version of it in that’s not too invasive on the underlying tendermint if all that’s desired is a state root in the current block, though that might be a bit hacky. At least it should be possible for a proposer to compute a current state root on executing the block before proposal, and include that state root in the proposal, and have everyone confirm it in the ProcessProposal. Perhaps worth thinking about again. Though it would be weird to be doing that and still have the off by one state root in the underlying canonical block structure.

2 Likes