-
Notifications
You must be signed in to change notification settings - Fork 378
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
feat: dynamic gas price, keeper implementation #2838
base: master
Are you sure you want to change the base?
Changes from 9 commits
cb8cc2f
e7cf296
7696e58
ae25c58
6da36bb
6f89b6b
8b71aa3
5d6be56
1504d8f
53dfbd3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
module github.com/gnolang/gno/contribs/gnodev | ||
|
||
go 1.22 | ||
go 1.22.0 | ||
|
||
replace github.com/gnolang/gno => ../.. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import ( | |
"github.com/gnolang/gno/tm2/pkg/sdk" | ||
"github.com/gnolang/gno/tm2/pkg/sdk/auth" | ||
"github.com/gnolang/gno/tm2/pkg/sdk/bank" | ||
"github.com/gnolang/gno/tm2/pkg/sdk/params" | ||
"github.com/gnolang/gno/tm2/pkg/std" | ||
"github.com/gnolang/gno/tm2/pkg/store" | ||
"github.com/gnolang/gno/tm2/pkg/store/dbadapter" | ||
|
@@ -86,14 +87,16 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { | |
baseApp.MountStoreWithDB(baseKey, dbadapter.StoreConstructor, cfg.DB) | ||
|
||
// Construct keepers. | ||
acctKpr := auth.NewAccountKeeper(mainKey, ProtoGnoAccount) | ||
paramsKpr := params.NewKeeper(mainKey, nil) | ||
acctKpr := auth.NewAccountKeeper(mainKey, paramsKpr, ProtoGnoAccount) | ||
gpKpr := auth.NewGasPriceKeeper(mainKey) | ||
bankKpr := bank.NewBankKeeper(acctKpr) | ||
vmk := vm.NewVMKeeper(baseKey, mainKey, acctKpr, bankKpr, cfg.MaxCycles) | ||
|
||
// Set InitChainer | ||
icc := cfg.InitChainerConfig | ||
icc.baseApp = baseApp | ||
icc.acctKpr, icc.bankKpr, icc.vmKpr = acctKpr, bankKpr, vmk | ||
icc.acctKpr, icc.bankKpr, icc.vmKpr, icc.gpKpr = acctKpr, bankKpr, vmk, gpKpr | ||
baseApp.SetInitChainer(icc.InitChainer) | ||
|
||
// Set AnteHandler | ||
|
@@ -107,9 +110,11 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { | |
func(ctx sdk.Context, tx std.Tx, simulate bool) ( | ||
newCtx sdk.Context, res sdk.Result, abort bool, | ||
) { | ||
// Add last gas price in the context | ||
ctx = ctx.WithValue(auth.GasPriceContextKey{}, gpKpr.LastGasPrice(ctx)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe a refactor is necessary generally, for this and other key-value pairs being put in the std.Context instance, but it seems like data that is always necessary should be part of the Context struct's definition. This approach still makes sense for storing key-value pairs for module data that is not required. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good point. We can address the refactor in a separate PR later |
||
|
||
// Override auth params. | ||
ctx = ctx. | ||
WithValue(auth.AuthParamsContextKey{}, auth.DefaultParams()) | ||
ctx = ctx.WithValue(auth.AuthParamsContextKey{}, acctKpr.GetParams(ctx)) | ||
// Continue on with default auth ante handler. | ||
newCtx, res, abort = authAnteHandler(ctx, tx, simulate) | ||
return | ||
|
@@ -140,6 +145,8 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { | |
baseApp.SetEndBlocker( | ||
EndBlocker( | ||
c, | ||
acctKpr, | ||
gpKpr, | ||
vmk, | ||
baseApp, | ||
), | ||
|
@@ -229,6 +236,7 @@ type InitChainerConfig struct { | |
vmKpr vm.VMKeeperI | ||
acctKpr auth.AccountKeeperI | ||
bankKpr bank.BankKeeperI | ||
gpKpr auth.GasPriceKeeperI | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dream of the day we |
||
} | ||
|
||
// InitChainer is the function that can be used as a [sdk.InitChainer]. | ||
|
@@ -286,6 +294,10 @@ func (cfg InitChainerConfig) loadAppState(ctx sdk.Context, appState any) ([]abci | |
if !ok { | ||
return nil, fmt.Errorf("invalid AppState of type %T", appState) | ||
} | ||
cfg.acctKpr.InitGenesis(ctx, state.Auth) | ||
params := cfg.acctKpr.GetParams(ctx) | ||
ctx = ctx.WithValue(auth.AuthParamsContextKey{}, params) | ||
auth.InitChainer(ctx, cfg.gpKpr.(auth.GasPriceKeeper), params.InitialGasPrice) | ||
|
||
// Parse and set genesis state balances | ||
for _, bal := range state.Balances { | ||
|
@@ -335,19 +347,33 @@ type endBlockerApp interface { | |
// validator set changes | ||
func EndBlocker( | ||
collector *collector[validatorUpdate], | ||
acctKpr auth.AccountKeeperI, | ||
gpKpr auth.GasPriceKeeperI, | ||
vmk vm.VMKeeperI, | ||
app endBlockerApp, | ||
) func( | ||
ctx sdk.Context, | ||
req abci.RequestEndBlock, | ||
) abci.ResponseEndBlock { | ||
return func(ctx sdk.Context, _ abci.RequestEndBlock) abci.ResponseEndBlock { | ||
// set the auth params value in the ctx. The EndBlocker will use InitialGasPrice in | ||
// the params to calculate the updated gas price. | ||
if acctKpr != nil { | ||
ctx = ctx.WithValue(auth.AuthParamsContextKey{}, acctKpr.GetParams(ctx)) | ||
} | ||
if acctKpr != nil && gpKpr != nil { | ||
auth.EndBlocker(ctx, gpKpr) | ||
} | ||
// Check if there was a valset change | ||
if len(collector.getEvents()) == 0 { | ||
// No valset updates | ||
return abci.ResponseEndBlock{} | ||
} | ||
|
||
if vmk == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we need this level of defensive programming; when is this situation possible? |
||
return abci.ResponseEndBlock{} | ||
} | ||
|
||
// Run the VM to get the updates from the chain | ||
response, err := vmk.QueryEval( | ||
ctx, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can skip the minor version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is to fix the issue of being unable to download the toolchain. I just realized that adding toolchain go1.22.4 is a better solution.