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

StorageIterator does not work #199

Closed
RavenXce opened this issue Mar 18, 2018 · 4 comments · Fixed by #200
Closed

StorageIterator does not work #199

RavenXce opened this issue Mar 18, 2018 · 4 comments · Fixed by #200
Labels
Bug Used to tag confirmed bugs

Comments

@RavenXce
Copy link
Contributor

RavenXce commented Mar 18, 2018

Please see neo-project/neo-devpack-dotnet#19 (comment) for example reproduction.

The issue is that when doing the prefix building in Storage.Find, the contract script hash and storage prefix are simply concatenated.

prefix = context.ScriptHash.ToArray().Concat(prefix).ToArray();

However, for Storage.Put/Get, I believe that serialization uses WriteVarBytes for the key portion:

writer.WriteVarBytes(Key);

Which prefixes the length of the key:

writer.WriteVarInt(value.Length);

I think we can safely construct and then serialize a StorageKey instead to ensure they match?

Edit: Actually, the length prefix messes up the whole implementation - one way to make it work is by specifying key length, so the keys iterated on are constrained to a single length and must be known.

@ixje
Copy link
Contributor

ixje commented Mar 18, 2018

For what it's worth, in neo-python the ScriptHash + prefix concatenation is workable, we just had to fix Storage Put/Get/Delete calls because they were creating a key that was not prefix searchable (e.g. ScriptHash + murmur_hash(key) -> convert to bigint then get bytes as key). The same might apply in C#, not sure

@RavenXce
Copy link
Contributor Author

RavenXce commented Mar 18, 2018

Since the keys in the db change does that mean that nodes need to re-build storage on upgrade?

The other solution here is to change how Put/Get/Delete works as well, but I thought that would be more complicated.

@ixje
Copy link
Contributor

ixje commented Mar 18, 2018

Yes for neo-python the user will have to rebuild storage. There was no way around it because previously the following 2 Put calls in a smart contract

    Put(ctx, 'my_prefixA', 1)
    Put(ctx, 'my_prefixB', 2)

would generate the following DB keys:

b'^\xbe\xe4\xf4\x00'
b'\xaa.\xd5t'

It was deterministic befor and would save space, but as can be seen that would never reasonable be prefix searchable.

@RavenXce
Copy link
Contributor Author

RavenXce commented Mar 19, 2018

I guess thats a possible solution here too.

I've tested and can confirm the issue is due to the key prefix as mentioned in OP. Manually prefixing an additional key length in addition to the desired prefix allows keys with that length and prefix to be returned.

erikzhang pushed a commit that referenced this issue Mar 20, 2018
erikzhang pushed a commit that referenced this issue Mar 20, 2018
This reverts commit d82d01a.
@erikzhang erikzhang added the Bug Used to tag confirmed bugs label Mar 21, 2018
Thacryba pushed a commit to simplitech/neo that referenced this issue Feb 17, 2020
…o-project#199)

* correction for parameter types in deploy example

* correction for parameter types in Woolong example code comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Used to tag confirmed bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants