Introduce ‘max_gas_burnt_view’ client config and command line flag #4381
Conversation
mina86
requested review from
ailisp,
bowenwang1996,
evgenykuzyakov,
frol,
khorolets,
MaksymZavershynskyi and
SkidanovAlex
as code owners
|
I haven’t yet fully tested that the limit actually takes hold. Any hints how to do that welcome. :) |
I think the best way will be add a pytest (in pytest dir), where you can write a python test to override config.json and command line options, and send rpc request to check the setting is effective |
I’m struggling a bit with figuring out an RPC to test the setting. |
|
The check failure looks related to actix/actix-web#2185. I’ve tried updating |
|
It already is part of the RuntimeConfig (and is included in genesis config). This change will make it so that the actual value used by the runtime will be overridden. |
|
oh I didn't realize that. @evgenykuzyakov what was the motivation to have it as part of |
I encountered this fail in my PR too, one way of fix this is to pin actix-http: 7dcfd0d and also update Cargo.lock 6fa4bd8 |
| genesis_config: GenesisConfig, | ||
| genesis_runtime_config: Arc<RuntimeConfig>, | ||
| /// Runtime configuration. Note that it may be slightly different than | ||
| /// `genesis_config.runtime_config`. Consider `max_gas_burnt_view` value | ||
| /// which may be configured per node. | ||
| runtime_config: Arc<RuntimeConfig>, |
There was a problem hiding this comment.
I feel that config handling here accumulated quite a bit of technical debt, as we now have two separate paths that "patch" config: one with max_gas_burnt, and another with RuntimeConfig::from_protocol_version (which is especcially confusing, as it relies on hidden global state). I have a stalled PR which cleans parts of this up in #4263 (comment).
This isn't strictly related to the PR, so it isn't a blocker, but I want to raise awareness and signal that straightening this up is non-urgent, but important ;-)
|
Added tests though also added a |
The ‘max_gas_burnt_view’ affects the RPCs only and therefore doesn’t need to be part of the consensus. In other words, all nodes could have this value customised rather than using an agreed upon value from genesis configuration. Introduce a ‘max_gas_burnt_view’ field to ClientConfig and a corresponding flag to ‘init’ and ‘run’ subcommands. When present, it will override the limit used by the node ignoring whatever is stored in genesis configuration. Fixes: near#3080
The ‘max_gas_burnt_view’ affects the RPCs only and therefore doesn’t
need to be part of the consensus. In other words, all nodes could
have this value customised rather than using an agreed upon value
from genesis configuration.
Introduce a ‘max_gas_burnt_view’ field to ClientConfig and
a corresponding flag to ‘init’ and ‘run’ subcommands. When present,
it will override the limit used by the node ignoring whatever is
stored in genesis configuration.
Fixes: #3080