fix: Gas limit check on PrepareProposal #277
Conversation
|
Important Auto Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChat with CodeRabbit Bot (
|
abci/abci.go
Outdated
| ) | ||
|
|
||
| bidTxIterator := h.mempool.AuctionBidSelect(ctx) | ||
| txsToRemove := make(map[sdk.Tx]struct{}, 0) | ||
| seenTxs := make(map[string]struct{}, 0) | ||
|
|
||
| // Do we need to deduct some from here since begin block gas gets carried over? |
There was a problem hiding this comment.
Upstream bug apparent on most v0.47.x chains makes me think we want to deduct some amount from here.
There was a problem hiding this comment.
It gets carried over into DeliverTxState's gas meter, I don't think that the PrepareProposal gas meter shld have anything to do here
abci/abci.go
Outdated
| ) | ||
|
|
||
| bidTxIterator := h.mempool.AuctionBidSelect(ctx) | ||
| txsToRemove := make(map[sdk.Tx]struct{}, 0) | ||
| seenTxs := make(map[string]struct{}, 0) | ||
|
|
||
| // Do we need to deduct some from here since begin block gas gets carried over? |
There was a problem hiding this comment.
It gets carried over into DeliverTxState's gas meter, I don't think that the PrepareProposal gas meter shld have anything to do here
|
@nivasan1 my worry is that the begin block gas meter creeps into deliver state, at which point a proposal that is at max block gas limit will exceed the consensus wide gas limit - effectively running into the same issue existing atm. |
| txsToRemove[tmpBidTx] = struct{}{} | ||
| continue selectBidTxLoop | ||
| } | ||
| gasLimit += feeTx.GetGas() |
There was a problem hiding this comment.
What happens if PrepareProposalVerifyTx fails?
There was a problem hiding this comment.
gas limit will be reset on the next iteration so this is safe
No description provided.