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

Allow configuration of blockchain's interpreter runtime #476

Merged

Conversation

m-Peter
Copy link
Contributor

@m-Peter m-Peter commented Sep 20, 2023

Work towards: onflow/cadence-tools#210

Description

Also expose a public method for creating script environments based on blockchain's ledger state.
These utility functions facilitate the usage of emulator's blockchain, as a library for 3rd party tools (e.g. cadence-tools/test).


For contributor use:

  • Targeted PR against master branch
  • Linked to GitHub issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the GitHub PR explorer
  • Added appropriate labels

@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2023

Codecov Report

Patch coverage: 85.71% and project coverage change: +0.03% 🎉

Comparison is base (bcf6545) 55.00% compared to head (e15a79d) 55.03%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
+ Coverage   55.00%   55.03%   +0.03%     
==========================================
  Files          28       28              
  Lines        3620     3632      +12     
==========================================
+ Hits         1991     1999       +8     
- Misses       1470     1474       +4     
  Partials      159      159              
Flag Coverage Δ
unittests 55.03% <85.71%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
emulator/blockchain.go 72.34% <85.71%> (-0.07%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

emulator/blockchain.go Outdated Show resolved Hide resolved
emulator/blockchain.go Outdated Show resolved Hide resolved
emulator/blockchain.go Show resolved Hide resolved
@turbolent turbolent added the Improvement Technical work without new features, refactoring, improving tests label Sep 20, 2023
@m-Peter
Copy link
Contributor Author

m-Peter commented Sep 21, 2023

I also came across a bug, which I have fixed here: cd45777

@m-Peter m-Peter force-pushed the configurable-blockchain-runtime-environment branch from cd45777 to 5566481 Compare September 21, 2023 13:44
@SupunS SupunS self-assigned this Sep 21, 2023
emulator/blockchain.go Outdated Show resolved Hide resolved
@janezpodhostnik
Copy link
Contributor

Is the injection via NewCustomReusableCadenceRuntimePool with the parameter newCustomRuntime CadenceRuntimeConstructor not usable? Not sure we need both.

…ader

We also rename Blockchain's newFVMContextFromHeader() to setFVMContextFromHeader()
We also update Blockchain.GetSourceFile() to use the
Blockchain.NewScriptEnvironment() method.
@m-Peter m-Peter force-pushed the configurable-blockchain-runtime-environment branch from e15a79d to 3be569a Compare September 27, 2023 08:35
@m-Peter
Copy link
Contributor Author

m-Peter commented Sep 27, 2023

@janezpodhostnik Very good point 👏
I checked again with the companion PR that depends on this change (onflow/cadence-tools#210) and the WithRuntime function is not actually needed. So I have removed it entirely.

@SupunS SupunS merged commit 75caeb6 into onflow:master Sep 28, 2023
3 checks passed
@m-Peter m-Peter deleted the configurable-blockchain-runtime-environment branch September 29, 2023 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Technical work without new features, refactoring, improving tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants