Add gas meter to tokenfactory trackBeforeSend #5927
Conversation
|
Marking as draft since final decision of whether we should suppress the panic happening or not has not been made. |
x/tokenfactory/keeper/before_send.go
Outdated
| em := sdk.NewEventManager() | ||
| // if its track before send, apply gas meter to prevent infinite loop | ||
| if !blockBeforeSend { | ||
| ctx = ctx.WithGasMeter(sdk.NewGasMeter(types.TrackBeforeSendGasLimit)) |
There was a problem hiding this comment.
We want to write this consumed gas meter result to the parent gas meter though. (So if that panics, then its fine)
I think that looks like keeping parentCtx, and doing "parentCtx.GasMeter.Use(childCtx.GasMeter.GasUsed)"
There was a problem hiding this comment.
If it's because of panic catching, it's already fine as of right now since we recover thread from panic though using defer function above right? (Probably me missing something)
There was a problem hiding this comment.
Suppose the parent gas meter had say 5 million gas.
It does this + some other stuff. Work here should still contribute to that gas
We still want the gas consumed here to get added to that gas meter. Its totally fine if it can't out of gas error inside this function, the next thing that consumes gas will cause the gas panic
Closes: #5725
What is the purpose of the change
Previously, we had a bug in tokenfactory module where registering an infinite contract as send hook contract then calling module to module send would cause infinite loop in the state machine as well, since module to module sends are not properly gas metered.
This PR adds gas meter for track before sned to prevent problem mentioned above.
Testing and Verifying
According tests have been added
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)