-
Notifications
You must be signed in to change notification settings - Fork 146
Conversation
Codecov Report
@@ Coverage Diff @@
## master #120 +/- ##
==========================================
- Coverage 80.66% 80.32% -0.35%
==========================================
Files 45 45
Lines 4578 4529 -49
==========================================
- Hits 3693 3638 -55
- Misses 885 891 +6
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #120 +/- ##
==========================================
- Coverage 80.89% 80.56% -0.34%
==========================================
Files 45 45
Lines 4633 4584 -49
==========================================
- Hits 3748 3693 -55
- Misses 885 891 +6
Continue to review full report at Codecov.
|
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.
What changed? The aggressive inline?
Previously we cached the instructions inside ExecutionContext, so if you made a CALL inside other call, you will create two ExecutionContexts with different caches, after this change, the script have this cache, so every ExecutionContext who comes from one Script, share his cache. |
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.
Looks fine to me!
As soon as this is merged, we can improve #121 with the final tests missing here. |
/// <summary> | ||
/// Number of items to be returned | ||
/// </summary> | ||
internal int RVCount { get; } | ||
public int RVCount { get; } |
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.
Why change this to public
?
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.
Because is read only, and maybe we need it for debuggers or something like that
src/neo-vm/Script.cs
Outdated
@@ -8,6 +9,7 @@ public class Script | |||
|
|||
private readonly byte[] _value; | |||
private readonly ICrypto _crypto; | |||
private readonly IDictionary<int, Instruction> _instructions = new Dictionary<int, Instruction>(); |
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.
I think it's not necessary to use interface for a private
field.
Use Script class for caching the instructions instead of ExecutionContext