Open itsdevbear opened 1 year ago
Mhhh I don't think this has anything to do with caching TBH. Reward calculation relies heavily on the current block height, i.e. ctx.BlockHeight()
. Are the heights between the two query paths the same? I presume you're using the latest height?
Yeah @alexanderbez we are using the latest height as confirmed with some printf's, here is the codepath for creating the querycontext for use in the estimate gas rpc routine.
Notably if requested height == ctx.BlockHeight() we just return a cache context.
I see. Maybe caching does have something to do with it. Recall that reward actually does some state modifications via IncrementValidatorPeriod
. I wonder if that has something to do with it.
This might be one of those situations where we might need to whip out the good ole fmt.Println
debugging in the withdrawDelegationRewards
function. I'm curious what endingPeriod
is between the two query paths?
Summary of Bug
When testing the EstimateGas for stateful precompiles in Polaris (https://github.com/berachain/polaris) we experienced a strange phenonemum where the estimated gas when using a QueryContext vs the canonical DeliverStateContxt resulted in two different numbers for the consumed gas. For context, Polaris supports stateful precompile evm contracts, which allows for smart contracts in our EVM to call Cosmos-SDK modules directly, in this example we are using x/staking (and by proxy x/distribution).
This difference in rounding lead to situations where our Ethereum JSON-RPC's Estimate Gas was reporting a lower required gas then what is actually required to execute the transaction leading to failed transactions. Most notably this occurs when a user tries to call
delegate()
. Part of delegating to a validator in the SDK requires withdrawing outstanding rewards in order to allow the passive distribution math ot functional. Notably, on an alternating fashion, we saw that when using the QueryContext the pending rewards for a user was 0, leading to the following control flow to be skipped (https://github.com/cosmos/cosmos-sdk/blob/ba9d4df6d0d1e8b28631459090b50f8da6aa71b0/x/distribution/keeper/delegation.go#L170) The rewards being 0 (when they shouldnt be) causes the control flow to skip calling into the bank module and thus consumes less gas. This estimated gas is then utilized when the transaciton is submitted on chain, in which the outstanding is no 0, the control flow enters the bank module, consuming the extra gas and resulting in an evm out of gas error.Here is a link to the issue in our repo: https://github.com/berachain/polaris/issues/480
@tac0turtle and I have chatted a bit and he thinks that it may be a caching issue with the QueryContext and/or the Root Multistore, but would love others thoughts on this for sure.
Version
From our
go.mod
Steps to Reproduce
mage start
(this will start a chain)