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

Remove StorageItem.IsConstant #2351

Merged
merged 1 commit into from
Feb 19, 2021
Merged

Remove StorageItem.IsConstant #2351

merged 1 commit into from
Feb 19, 2021

Conversation

erikzhang
Copy link
Member

No description provided.

@roman-khimov
Copy link
Contributor

Not a trivial thing for me. On one hand it's a useful security feature (ensuring that whatever is written there couldn't be changed in any way), on the other hand the fact that constant entry couldn't be removed is a problem of its own and additional serialization overhead is also not a good thing.

I've added a simple check for attempts to store a constant entry to Neo 2 neo-go (the node will just panic and stop in this case) and running through the chain dump at the moment, 2.6M blocks processed and it's still running.

@shargon
Copy link
Member

shargon commented Feb 18, 2021

I've added a simple check for attempts to store a constant entry to Neo 2 neo-go (the node will just panic and stop in this case) and running through the chain dump at the moment, 2.6M blocks processed and it's still running.

Do you have the numbers?

@roman-khimov
Copy link
Contributor

roman-khimov commented Feb 18, 2021

Sorry, it ran out of disk space at 3.2M, but it means at least that there are exactly zero IsConstant uses up to this height. I'm running it again, but it'll take some time.

@shargon
Copy link
Member

shargon commented Feb 18, 2021

I think that the contract can ensure that is never modified, with a good code and security audits, in my opinion it's not needed

@erikzhang erikzhang merged commit b1a6ed0 into master Feb 19, 2021
@erikzhang erikzhang deleted the Remove-IsConstant branch February 19, 2021 10:58
@roman-khimov
Copy link
Contributor

FYI: I wasn't able to find a single use of this feature on Neo 2 chain, so looks like it won't be missed.

roman-khimov added a commit to roman-khimov/neo that referenced this pull request Mar 5, 2021
After neo-project#2351 StorageItem is just a thin wrapper for some byte array in its
essence so we can Use the same trick as was used for storage keys in neo-project#1566 and
avoid additional wrapping/unwrapping with VarBytes encoding.
shargon pushed a commit that referenced this pull request Mar 8, 2021
* StorageItem: optimize binary representation

After #2351 StorageItem is just a thin wrapper for some byte array in its
essence so we can Use the same trick as was used for storage keys in #1566 and
avoid additional wrapping/unwrapping with VarBytes encoding.

* StorageItem: optimize out reader.BaseStream.Position

It's always 0.
ixje added a commit to CityOfZion/neo-mamba that referenced this pull request Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants