-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix CallFlags for native contracts and syscalls #2339
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.
We might as well make GetContext
require States
(with GetReadOnlyContext
requiring ReadStates
only), although it can make writing simple getters more complex (it's very convenient to do GetContext
in the _initialize
and use it everywhere).
public static readonly InteropDescriptor System_Storage_Delete = Register("System.Storage.Delete", nameof(Delete), 0, CallFlags.WriteStates); | ||
public static readonly InteropDescriptor System_Storage_Put = Register("System.Storage.Put", nameof(Put), 0, CallFlags.States); | ||
public static readonly InteropDescriptor System_Storage_PutEx = Register("System.Storage.PutEx", nameof(PutEx), 0, CallFlags.States); | ||
public static readonly InteropDescriptor System_Storage_Delete = Register("System.Storage.Delete", nameof(Delete), 0, CallFlags.States); |
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'd probably leave these as is.
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.
We need to read the IsConstant
field before writing. Do we need IsConstant
? Maybe we can remove it.
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.
@shargon @roman-khimov What do you think?
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.
It can be a significant storage item simplification, but we'll loose a safety feature at the same time that I can't see any substitute for (other than a separate "constant storage context", but that's not really better than the flag we have at the moment). But I'm not sure it's really used by contracts.
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 never used.
* neo-project/neo#2295 * neo-project/neo#2290 * neo-project/neo#2292 * neo-project/neo#2296 * neo-project/neo#2301 * neo-project/neo#2298 * neo-project/neo#2312 * neo-project/neo#2300 * neo-project/neo#2333 * neo-project/neo#2337 * neo-project/neo#2331 * neo-project/neo#2332 * neo-project/neo#2343 * neo-project/neo#2339 * neo-project/neo#2350 * neo-project/neo#2351 * neo-project/neo#2353 * neo-project/neo#2356 * neo-project/neo#2375 * neo-project/neo#2377 * neo-project/neo#2379 * https://github.com/neo-project/neo/pull/2392/files * neo-project/neo#2400 * audit updates * refactor and bump VM requirement
Close #2316