Read only methods #1052
Read only methods #1052
Conversation
Read only methods
Codecov Report
@@ Coverage Diff @@
## master #1052 +/- ##
==========================================
+ Coverage 61.3% 61.34% +0.04%
==========================================
Files 196 196
Lines 13412 13420 +8
==========================================
+ Hits 8222 8233 +11
+ Misses 5190 5187 -3
Continue to review full report at Codecov.
|
| { | ||
| this.Method = method; | ||
| this.Hash = method.ToInteropMethodHash(); | ||
| this.Handler = handler; | ||
| this.AllowedTriggers = allowedTriggers; | ||
| this.RequireWriteAccess = requireWriteAccess; |
There was a problem hiding this comment.
I don't think this is a good idea @shargon... what is RequireWriteAccess? To Storage?
Could we remove this?
I thought that, if current ExecutionContextState is running on ReadOnly, then methods that require write access could simply fail (example: Storage.GetCurrentContext`).
There was a problem hiding this comment.
We can rename the var, but not remove it, because is used for ban the syscall when is in readOnly mode
| @@ -88,19 +88,21 @@ internal static bool Invoke(ApplicationEngine engine, uint method) | |||
| return false; | |||
| if (!descriptor.AllowedTriggers.HasFlag(engine.Trigger)) | |||
| return false; | |||
| if (descriptor.RequireWriteAccess && engine.CurrentContext.GetState<ExecutionContextState>().ReadOnly) | |||
There was a problem hiding this comment.
No need to check this @shargon... if something really needs write access, you can invoke it, and let it fail naturally when it tries to access something that requires writing.
Only thing we need to ensure is: if current context is ReadOnly, then next invocations context will also be ReadOnly, but this is simple, as we can do this when loading new context into stack.
There was a problem hiding this comment.
I want to ban more syscalls like destroy and create, i think thats is better with this flag
There was a problem hiding this comment.
Let's remove explicit InteropDescriptor for RequreWriteAccess, as it doesn't make sense on general...
We also don't need to check if invoking a readonly method, from a non-readonly, as the invoked method will naturally fail when it tries to access stuff that it can't.
We just need to ensure that, once the execution mode is invoked on ReadOnly mode, this is put into context, and all next invocations will also be ReadOnly forever.
|
|
|
Require update NEP-3 first |
|
I think this is not needed now. |
Close #927
TODO: