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

bug: Param estimator reuses accounts that rely on the storage #3601

Closed
evgenykuzyakov opened this issue Nov 12, 2020 · 1 comment · Fixed by #3616
Closed

bug: Param estimator reuses accounts that rely on the storage #3601

evgenykuzyakov opened this issue Nov 12, 2020 · 1 comment · Fixed by #3616
Assignees

Comments

@evgenykuzyakov
Copy link
Collaborator

Param estimator reuses accounts for measuring different host function gas numbers.
Some of these arguments depend on the existing storage. If we use an account, then the 2nd estimate run on this account will hit a modified storage and may receive a different result.

For example storage_remove_10b_key_10kib_value_1k has only 1 account with keys (for some reason). It requires the existing values in the trie to be able to count the number of bytes removed. It causes the cost to either overestimate or underestimate.

@ailisp
Copy link
Member

ailisp commented Nov 13, 2020

I think i finally understand this bug:

For example storage_remove_10b_key_10kib_value_1k
has only 1 account with keys (for some reason). It requires the
existing values in the trie to be able to count the number of bytes
removed. It causes the cost to either overestimate or underestimate.

it's because the key added is by the previous storage_write_10kib_key_10b_value_1k test, which is random select block_sizeiter_block accounts from total accounts-num accountsThen, storage_remove_10b_key_10kib_value_1k is randomly select block_sizeiter_block accounts from total accounts-num accounts, and they (most likely) have no keys at all.

A fix should be, before test like storage_remove_10b_key_10kib_value_1k (there's several similar storage_remove tests), either

  • add storage key,value to all accounts instead of random selected accounts
  • in storage_remove_10b_key_10kib_value_1k reuse exact same accounts in storage_write_10kib_key_10b_value_1k instead of currently we randomly select from total account-num accounts again.

near-bulldozer bot pushed a commit that referenced this issue Dec 8, 2020
Fix #3601. get new numbers which is very different from existing genesis.json, but reasonable compare to run runtime-param-estimator from master branch

Test Plan
------------
runtime-param-estimator works, no #3598 and #3601. And every delete storage actually delete, not noop.
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.

2 participants