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

Add reserved gas for system transactions #187

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

minh-bq
Copy link
Contributor

@minh-bq minh-bq commented Nov 30, 2022

Currently, when collecting transactions to commit to a block, we only check the accumulated gas with block's gas limit. Later, when finalizing the block, with system transactions included, the block's gas used may exceed the block's gas limit. As a result, that block is not mined. We want to avoid this scenario by introducing a new flag to reserve some gas in gas pool for system transactions.

@minh-bq minh-bq requested review from DNK90 and linh-1 November 30, 2022 06:24
miner/worker.go Outdated Show resolved Hide resolved
@@ -813,6 +819,12 @@ func (w *worker) commitTransactions(txs *types.TransactionsByPriceAndNonce, coin
gasLimit := w.current.header.GasLimit
if w.current.gasPool == nil {
w.current.gasPool = new(core.GasPool).AddGas(gasLimit)

// If the gas pool is newly created, reserve some gas for system transactions
if err := w.current.gasPool.SubGas(w.config.GasReserve); err != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are in pessimistic mode

Currently, when collecting transactions to commit to a block, we only check the
accumulated gas with block's gas limit. Later, when finalizing the block, with
system transactions included, the block's gas used may exceed the block's gas
limit. As a result, that block is not mined. We want to avoid this scenario by
introducing a new flag to reserve some gas in gas pool for system transactions.
@minh-bq minh-bq merged commit a1f3645 into axieinfinity:master Jan 16, 2023
@minh-bq minh-bq deleted the feat/gas-reserve branch January 16, 2023 04:23
minh-bq added a commit to minh-bq/ronin that referenced this pull request Oct 4, 2023
This partially reverts commit a1f3645 to make
the reserved gas a constant not a provided configuration from user. This still
keeps the created flag in that commit for compatibility.
minh-bq added a commit that referenced this pull request Oct 4, 2023
)

* Revert "Add reserved gas for system transactions (#187)"

This partially reverts commit a1f3645 to make
the reserved gas a constant not a provided configuration from user. This still
keeps the created flag in that commit for compatibility.

* miner, txpool: add a reserved gas constant for system transactions

This commit adds the reserved gas for system transaction, which is previously a
user provided flag, as a constant.

In the miner part, the miner stops commit normal transactions to block when
block gas used is over the block's gas limit - reserved gas. So when finalizing
and including system transactions into block, the block's gas used does not exceed
the block's gas limit.

In the txpool part, when validating a transaction before putting it into the
pool, previously, we only check if the transaction's gas limit is not over the
block's gas limit. However, if the transaction's gas limit is over the block's
gas limit - reserved gas, it is never included into the block but still always
stay in pending transaction pool. This commit fixes the check to reject the
transaction with gas limit over the block's gas limit - reserved gas.
minh-bq added a commit that referenced this pull request Oct 4, 2023
)

* Revert "Add reserved gas for system transactions (#187)"

This partially reverts commit a1f3645 to make
the reserved gas a constant not a provided configuration from user. This still
keeps the created flag in that commit for compatibility.

* miner, txpool: add a reserved gas constant for system transactions

This commit adds the reserved gas for system transaction, which is previously a
user provided flag, as a constant.

In the miner part, the miner stops commit normal transactions to block when
block gas used is over the block's gas limit - reserved gas. So when finalizing
and including system transactions into block, the block's gas used does not exceed
the block's gas limit.

In the txpool part, when validating a transaction before putting it into the
pool, previously, we only check if the transaction's gas limit is not over the
block's gas limit. However, if the transaction's gas limit is over the block's
gas limit - reserved gas, it is never included into the block but still always
stay in pending transaction pool. This commit fixes the check to reject the
transaction with gas limit over the block's gas limit - reserved gas.
andicrypt pushed a commit to andicrypt/ronin that referenced this pull request Nov 1, 2023
…xieinfinity#364)

* Revert "Add reserved gas for system transactions (axieinfinity#187)"

This partially reverts commit a1f3645 to make
the reserved gas a constant not a provided configuration from user. This still
keeps the created flag in that commit for compatibility.

* miner, txpool: add a reserved gas constant for system transactions

This commit adds the reserved gas for system transaction, which is previously a
user provided flag, as a constant.

In the miner part, the miner stops commit normal transactions to block when
block gas used is over the block's gas limit - reserved gas. So when finalizing
and including system transactions into block, the block's gas used does not exceed
the block's gas limit.

In the txpool part, when validating a transaction before putting it into the
pool, previously, we only check if the transaction's gas limit is not over the
block's gas limit. However, if the transaction's gas limit is over the block's
gas limit - reserved gas, it is never included into the block but still always
stay in pending transaction pool. This commit fixes the check to reject the
transaction with gas limit over the block's gas limit - reserved gas.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants