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

core: use proper BaseExecFee and StoragePrice for interop context #2432

Merged
merged 4 commits into from
Apr 14, 2022

Conversation

AnnaShaleva
Copy link
Member

Problem

Outdated and non-unified BaseExecFee and StoragePrice values used for interop context pricing.

Solution

Merge.

The usage of the Blockchain's one leads to the same ExecFeeFactor within
a single block. What we need is to update ExecFeeFactor after each
transaction invocation, thus, cached DAO should be used as it contains
all relevant changes.
We shouldn't use StoragePrice from Blockchain because its dao doesn't
contain the whole set of changes from previouse transactions in the
current block. Instead, we should use an updated storage price for
each transaction and retrieve the price from cached DAO.
@AnnaShaleva AnnaShaleva added the bug Something isn't working label Apr 8, 2022
The InteropContext's one contains all relevant fee changes applied in
the prevouse transactions of the current block.
@AnnaShaleva AnnaShaleva mentioned this pull request Apr 8, 2022
6 tasks
@roman-khimov roman-khimov added this to the v0.99.0 milestone Apr 8, 2022
@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #2432 (51a54fa) into master (f5d5019) will increase coverage by 0.08%.
The diff coverage is 86.56%.

@@            Coverage Diff             @@
##           master    #2432      +/-   ##
==========================================
+ Coverage   85.09%   85.18%   +0.08%     
==========================================
  Files         289      289              
  Lines       36106    36119      +13     
==========================================
+ Hits        30724    30767      +43     
+ Misses       4098     4066      -32     
- Partials     1284     1286       +2     
Impacted Files Coverage Δ
cli/options/options.go 90.62% <ø> (ø)
cli/server/server.go 79.78% <0.00%> (ø)
cli/smartcontract/generate.go 83.33% <ø> (ø)
pkg/consensus/consensus.go 72.86% <ø> (ø)
pkg/vm/state.go 92.85% <ø> (ø)
pkg/core/native/oracle.go 72.19% <36.84%> (ø)
pkg/core/dao/dao.go 79.76% <54.23%> (ø)
pkg/core/native/policy.go 98.05% <77.77%> (ø)
pkg/core/blockchain.go 83.18% <90.00%> (+0.15%) ⬆️
pkg/neotest/basic.go 94.09% <92.00%> (ø)
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ff11ba...51a54fa. Read the comment docs.

@AnnaShaleva AnnaShaleva marked this pull request as draft April 8, 2022 10:11
For (bc *Blockchain).GetBaseExecFee().
@AnnaShaleva
Copy link
Member Author

Follows the neo-project/neo#2692 behaviour and can be merged. But CircleCI tests are failing due to timeout, not sure what's wrong with them.

@AnnaShaleva AnnaShaleva marked this pull request as ready for review April 14, 2022 07:41
@roman-khimov roman-khimov merged commit f73510b into master Apr 14, 2022
@roman-khimov roman-khimov deleted the fix-fees branch April 14, 2022 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants