-
Notifications
You must be signed in to change notification settings - Fork 47
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
Computation parity with FVM #451
Computation parity with FVM #451
Conversation
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.
Thank you for this! Looks good overall, just had some minor comments.
emulator/script_test.go
Outdated
main() | ||
} | ||
` | ||
pub fun main() { |
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.
mixed tabs and spaces (here and in other cadence scripts)
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.
Fixed in 82b2bd6
emulator/script_test.go
Outdated
require.NoError(t, err) | ||
|
||
const code = ` | ||
pub fun main() { |
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.
maybe store the script in a const
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.
Good catch, I restructured the tests to remove the script duplication in 82b2bd6
Codecov Report
@@ Coverage Diff @@
## master #451 +/- ##
==========================================
- Coverage 54.39% 54.14% -0.25%
==========================================
Files 27 27
Lines 3519 3509 -10
==========================================
- Hits 1914 1900 -14
- Misses 1452 1455 +3
- Partials 153 154 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -692,6 +694,17 @@ func configureBootstrapProcedure(conf config, flowAccountKey flowgo.AccountPubli | |||
options = append(options, | |||
fvm.WithInitialTokenSupply(supply), | |||
fvm.WithRestrictedAccountCreationEnabled(false), | |||
fvm.WithTransactionFee(fvm.DefaultTransactionFees), |
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.
@m-Peter Could you please bring back the comment describing where these magic values come from, i.e. add
// This enables variable transaction fees AND execution effort metering
// as described in Variable Transaction Fees: Execution Effort FLIP: https://github.com/onflow/flow/pull/753)
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.
@turbolent Sure thing 😇
Works towards: onflow/developer-grants#178
Description
memoryEstimate
metric in logs (for scripts & transactions)For contributor use:
master
branchFiles changed
in the GitHub PR explorer