From d51851a677d5da2c1f9f86a5fec39d8c591ab6ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nina=20/=20=E1=83=9C=E1=83=98=E1=83=9C=E1=83=90?= Date: Tue, 24 Sep 2024 18:32:18 +0200 Subject: [PATCH] chore: move cip-24 to review (#208) * chore: move cip-24 to review * docs: add first few bits of reference implementation * docs: add reference implementation section * docs: change it to diffs --- cips/cip-24.md | 45 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/cips/cip-24.md b/cips/cip-24.md index 55ec924..f19bbd0 100644 --- a/cips/cip-24.md +++ b/cips/cip-24.md @@ -4,7 +4,7 @@ | description | Transition to hard fork-only updates for gas scheduler variables | | author | Nina Barbakadze ([@ninabarbakadze](https://github.com/ninabarbakadze)) | | discussions-to | | -| status | Draft | +| status | Review | | type | Standards Track | | category | Core | | created | 2024-07-24 | @@ -45,7 +45,48 @@ Test cases should verify that gas scheduler variables are exclusively updated vi ## Reference Implementation -TBC +Starting from v3, we updated the `PayForBlobs()` function in `x/blob/keeper.go` to use versioned `GasPerBlobByte` parameter when calculating gas based on the size of the blobs while maintaining compatibility with previous versions. + +```diff +- gasToConsume := types.GasToConsume(msg.BlobSizes, k.GasPerBlobByte(ctx)) ++ // GasPerBlobByte is a versioned param from version 3 onwards. ++ var gasToConsume uint64 ++ if ctx.BlockHeader().Version.App <= v2.Version { ++ gasToConsume = types.GasToConsume(msg.BlobSizes, k.GasPerBlobByte(ctx)) ++ } else { ++ gasToConsume = types.GasToConsume(msg.BlobSizes, appconsts.GasPerBlobByte(ctx.BlockHeader().Version.App)) ++ } ++ +``` + +Additionally, we modified the PFB gas estimation logic to use `appconsts.DefaultTxSizeCostPerByte`. + +```diff +-// DefaultEstimateGas runs EstimateGas with the system defaults. The network may change these values +-// through governance, thus this function should predominantly be used in testing. ++// DefaultEstimateGas runs EstimateGas with the system defaults. + func DefaultEstimateGas(blobSizes []uint32) uint64 { +- return EstimateGas(blobSizes, appconsts.DefaultGasPerBlobByte, auth.DefaultTxSizeCostPerByte) ++ return EstimateGas(blobSizes, appconsts.DefaultGasPerBlobByte, appconsts.DefaultTxSizeCostPerByte) + } + ``` + +We also needed to update the gas consumption logic related to transaction size in the ante handler. The `AnteHandle` function within `NewConsumeGasForTxSizeDecorator` has been modified to retrieve the `TxSizeCostPerByte` value from app constants for versions v3 and later. The logic for earlier versions remains unchanged. + +```diff ++// consumeGasForTxSize consumes gas based on the size of the transaction. ++// It uses different parameters depending on the app version. ++func consumeGasForTxSize(ctx sdk.Context, txBytes uint64, params auth.Params) { ++ // For app v2 and below we should get txSizeCostPerByte from auth module ++ if ctx.BlockHeader().Version.App <= v2.Version { ++ ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*txBytes, "txSize") ++ } else { ++ // From v3 onwards, we should get txSizeCostPerByte from appconsts ++ txSizeCostPerByte := appconsts.TxSizeCostPerByte(ctx.BlockHeader().Version.App) ++ ctx.GasMeter().ConsumeGas(txSizeCostPerByte*txBytes, "txSize") ++ } ++} +``` ## Security Considerations