Skip to content
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

Inconsistent min_gas_price value when in localnet/sandbox #7466

Closed
ChaoticTempest opened this issue Aug 23, 2022 · 7 comments · Fixed by #7475
Closed

Inconsistent min_gas_price value when in localnet/sandbox #7466

ChaoticTempest opened this issue Aug 23, 2022 · 7 comments · Fixed by #7475

Comments

@ChaoticTempest
Copy link
Member

ChaoticTempest commented Aug 23, 2022

Describe the bug
On localnet or a sandbox node, gas estimations are 10x the amount. Seems this happening since min_gas_price is set to 1_000_000_000 instead of 100_000_000 which is what is found on mainnet/testnet. From my findings, BlockEconomicsConfig::min_gas_price determines the min_gas_price dynamically and lowers the min_gas_price to 100_000_000 for mainnet/testnet since their genesis protocol version is below a threshold. Meanwhile newly launched localnet or sandbox nodes have protocol version of 56 as of the time of writing this issue which doesn't meet the threshold and causes this inconsistency.

To Reproduce

  • clone https://github.com/near/workspaces-rs/
  • checkout branch phuong/test/proxy
  • run cargo run --example proxy and then an error should occur like the following:
Error: handler error: [An error happened during transaction execution: InvalidAccessKeyError(NotEnoughAllowance { account_id: AccountId("dev-20220823224325-18831006822820"), public_key: ed25519:FzLCgiBnbhUvmcX78YWdugM7oapurcXcjAS4Fwt3hXi7, allowance: 56287890191155200000000, cost: 187424484981165931516950 })]
  • to test for newer changes once changes are made to nearcore:
    • cd nearcore
    • make sandbox
    • cd workspaces-rs
    • NEAR_SANDBOX_BIN_PATH=/abolute/path/to/nearcore/sandbox/debug/neard cargo run --example proxy where the error no longer occurs

Version (please complete the following information):

  • nearcore master
  • local

Additional context
Initial conversation on it in zulip:
https://near.zulipchat.com/#narrow/stream/295558-pagoda.2Fcore/topic/Inconsistent.20min_gas_price.20on.20localnet.2Fsandbox/near/294942074

@akhi3030
Copy link
Collaborator

CC: @jakmeier. Is this something that you can help diagnose please?

@matklad
Copy link
Contributor

matklad commented Aug 24, 2022

Code in question:

/// Compute min gas price according to protocol version and genesis protocol version.
pub fn min_gas_price(&self, protocol_version: ProtocolVersion) -> Balance {
if self.genesis_protocol_version < MIN_PROTOCOL_VERSION_NEP_92 {
if protocol_version >= MIN_PROTOCOL_VERSION_NEP_92_FIX {
MIN_GAS_PRICE_NEP_92_FIX
} else if protocol_version >= MIN_PROTOCOL_VERSION_NEP_92 {
MIN_GAS_PRICE_NEP_92
} else {
self.min_gas_price
}
} else if self.genesis_protocol_version < MIN_PROTOCOL_VERSION_NEP_92_FIX {
if protocol_version >= MIN_PROTOCOL_VERSION_NEP_92_FIX {
MIN_GAS_PRICE_NEP_92_FIX
} else {
MIN_GAS_PRICE_NEP_92
}
} else {
self.min_gas_price
}
}

I don't understand why genesis protocol version (as opposed to block's protocol version) would affect this in any way, this requires some git-blame archaeology.

@jakmeier
Copy link
Contributor

I don't think the code listed above is relevant. That method seems to only be used to validate that any given block does not exceed limits, which does depend on current limits and genesis limits.

The problems seems to be here:

pub const MIN_GAS_PRICE: Balance = 1_000_000_000;

For some reason, this value has never been updated. It's also not used at all for mainnet, testnet, betanet or shardnet. Only if the chain id is something else, then the genesis config is generated and actually used. Otherwise this constant seems to be used for tests only.

I wonder if the solution is just to update the constant. It seems to break only one test (test_chunk_transaction_validity) from what I checked locally, which should be fixable. And it resolves the reported issue in sandbox. But I am not very happy about this constant being what it is right now. Feels like it should be cleaned up somehow but I am not deep in it yet to see what would be clean solution.

@jakmeier
Copy link
Contributor

I think just increasing the constant really is the best way forward. The default gensis config should produce the gas price that is most recent. But I also added comments to make things easier to understand. See PR #7475.

I also noticed that shardnet has been launched with the old min_gas_price value, 10x the gas price used in mainnet/testnet/betanet. I don't think it matters in any way but probably not intentional. cc @mm-near @gmilescu

@mm-near
Copy link
Contributor

mm-near commented Aug 25, 2022

Ah - that explains why some people have seen transactions costing over 30 NEAR ..

@jakmeier
Copy link
Contributor

jakmeier commented Aug 25, 2022

Well, 30 NEAR still sounds like a bit much for a single transaction, if it was only because of the 10x. But I also noticed in the explorer that gas price has been inflated to the max, making it another 20x more expensive. With that, 150Tgas indeed costs 3 NEAR. (But 30 NEAR should still be out of range?)

@jakmeier
Copy link
Contributor

Oh, I just noticed that our pessimistic gas price charges go beyond the cap of 20x minimum price. Filed #7477. That explains why users needed more than 30 NEAR to initiate a transaction.

mina86 added a commit to mina86/nearcore that referenced this issue Aug 28, 2022

Verified

This commit was created on github.com and signed with GitHub’s verified signature.
Commit 028fa7c: ‘default genesis minimum gas price’ changed the
localnet gas price and broke Rosetta test.  Fix it.

Issues: near#7466
near-bulldozer bot pushed a commit that referenced this issue Aug 29, 2022

Verified

This commit was created on github.com and signed with GitHub’s verified signature. The key has expired.
Commit 028fa7c: ‘default genesis minimum gas price’ changed the
localnet gas price and broke Rosetta test.  Fix it.

Issues: #7466
mina86 added a commit to mina86/nearcore that referenced this issue Aug 29, 2022
Commit 028fa7c: ‘default genesis minimum gas price’ lowered gas price
in localnet which caused the test to start failing.  Lower balance in
the test to fix it.

While at it, refactor the code slightly to reduce all the indentation.

Issues: near#7466
near-bulldozer bot pushed a commit that referenced this issue Aug 30, 2022
…7500)

Commit 028fa7c: ‘default genesis minimum gas price’ changed default
minimum gas price in localnet which affected actual values in the
test.  Adjust the expected values to match thus fixing the test.

While at it, refactor the code slightly to reduce all the indentation.

Issues: #7466
nikurt pushed a commit that referenced this issue Nov 9, 2022
…7500)

Commit 028fa7c: ‘default genesis minimum gas price’ changed default
minimum gas price in localnet which affected actual values in the
test.  Adjust the expected values to match thus fixing the test.

While at it, refactor the code slightly to reduce all the indentation.

Issues: #7466
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants