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

Provide storage read method for enumerator return #880

Closed
hal0x2328 opened this issue Jul 1, 2019 · 25 comments
Closed

Provide storage read method for enumerator return #880

hal0x2328 opened this issue Jul 1, 2019 · 25 comments
Labels
design Issue state - Feature accepted but the solution requires a design before being implemented feature Type: Large changes or new features vm New features that affect the Neo Virtual Machine or the Interop layer

Comments

@hal0x2328
Copy link
Contributor

Returning an enumerator type is a powerful way to allow developers to filter results of a query of a very large array, but as a standalone return value, it lacks the ability to select and include related data, leading to more complex contracts with dozens of custom operations, or inefficient queries requiring multiple requests and large amounts of unnecessary data transfer.

For example, if I have an NFT contract with 100K tokens each representing a unique physical property deed, and I want to query the contract for all deeds in a certain zip code, I have to write an internal contract function to return that result, one for each possible permutation of other query selectors (e.g. property acreage, building type, etc). Either that or return the entire list of tokens IDs and query their properties one at a time which is wildly inefficient.

But, if I could simply call the tokens or tokensOf methods and get an enumerator, while iterating over the set I could request related data (e.g. token.properties.zipcode) in the same contract (or even an external contract) and use it as a selector, returning just the desired result set - but only if I can make a storage read while running outside of the contract, e.g.

Storage.Read(scripthash, key);

or

ctx = GetReadOnlyContext(scripthash);`
Storage.Get(ctx, key);

or some similar method to get read-only access to a specific contract's storage.

@igormcoelho igormcoelho added the discussion Initial issue state - proposed but not yet accepted label Jul 1, 2019
@igormcoelho
Copy link
Contributor

From my understanding of NFT discussion, I think that returning the enumerator is already the wisest choice, like you guys pointed out.
In terms of data filtering, I think it's more related to the Enumerator than to Storage itself. If you want to process data off-chain, that's easy, you get all data displayed by RPC node when "deserializing" the returned enumerator. So you could return the whole thing, and filter it off-chain.
If you want to filter data on the contract, perhaps it would be the case to have another interop Enumerator.Filter, that receives some sort of "lambda function", and applies over your enumerator, returning another enumerator.
Could you clarify if you need off-chain or on-chain processing Hal?

@hal0x2328
Copy link
Contributor Author

In terms of data filtering, I think it's more related to the Enumerator than to Storage itself. If you want to process data off-chain, that's easy, you get all data displayed by RPC node when "deserializing" the returned enumerator. So you could return the whole thing, and filter it off-chain.

The problem is when the data you want isn't contained in that enumerator but instead in other related storage keys. From my interpretation the tokens operation in NEP-11 just returns token IDs and nothing else. So you could write another function like tokensProperties to return an enumerator containing all the properties of all tokens, but what if the properties you want to use as a query selector are in an external contract?

If you want to filter data on the contract, perhaps it would be the case to have another interop Enumerator.Filter, that receives some sort of "lambda function", and applies over your enumerator, returning another enumerator.

That is an interesting idea but I don't know how practical it is for all the currently supported languages.

Could you clarify if you need off-chain or on-chain processing Hal?

It would be useful either way. It would all be running in the VM for maximum efficiency, regardless if it was done inside an appcall from another contract or a post-appcall function.

@shargon
Copy link
Member

shargon commented Jul 1, 2019

With the last json feature, it could be easier if you store your data in a json format

{
"balance":0,
"lastUpdate":"2019-01-01'
}

We could have a syscall like this....

var iterator = Storage.FindJson(u => u.balance > 0);

@igormcoelho
Copy link
Contributor

@hal0x2328 I think that this direction may be possible: #914

If we create some delegate function, to pass this as a lambda, and also provide a ReadOnlyStorageContext... user may possibly to any crazy things he wants. Yesterday I talked to a really clever guy from language design field, and he emphasized to me that these kind of iterators shouldn't update data.. otherwise you could create some infinite loops. Let's see how this evolves.

@lock9
Copy link
Contributor

lock9 commented Jul 15, 2019

@igormcoelho is this something we can start working on? @hal0x2328 how important is this for your dApp? In a scale 0 to 10?

@hal0x2328
Copy link
Contributor Author

hal0x2328 commented Jul 15, 2019

It's not critical for my dApp because I use a neo-cli plugin providing a custom RPC method to iterate over storage keys. But it could be very useful in the future for devs wanting to use the stock neo-cli RPC service.

@lock9
Copy link
Contributor

lock9 commented Jul 15, 2019

So what you mean is that "it is important" but you found another way to do it?

@hal0x2328
Copy link
Contributor Author

Exactly.

@igormcoelho
Copy link
Contributor

It's probably good for onchain usages of NFT tokens. And for other things that keep arising... looks promising.

@lock9
Copy link
Contributor

lock9 commented Jul 15, 2019

@neo-project/core should we include this in the roadmap? This seems important. I'm not sure about "performance impacts". Not sure if we should add stuff that can be done without smart contracts, to the blockchain.
It seems a good feature, let's evaluate the side effects.

@shargon
Copy link
Member

shargon commented Jul 15, 2019

is a good feature, but it must have a cost acording to the performance impact

@lock9
Copy link
Contributor

lock9 commented Jul 15, 2019

This is the same case as the json serialization / deserialization.
We can either add it to NEO and make users pay for this feature, or we have them using offchain tools.

It makes more sense, as a product, that this is added in neo and people pay for this usage.

@lock9
Copy link
Contributor

lock9 commented Jul 15, 2019

@hal0x2328 is it important for you to know transactions that are happening in the same block? Let's say you are running 100 calls to your smart contract, does the order of these transactions impact the final result?

@lock9 lock9 closed this as completed Jul 15, 2019
@lock9 lock9 reopened this Jul 15, 2019
@hal0x2328
Copy link
Contributor Author

@hal0x2328 is it important for you to know transactions that are happening in the same block? Let's say you are running 100 calls to your smart contract, does the order of these transactions impact the final result?

The post-invocation code which is iterating over the enumerator is only reading the storage state of the node at that point in time, so the results could be different if the same invocation is ran again later, the same as if you had just returned an array on the stack. This is simply about being able to filter the results and "join" values from other storage keys.

@igormcoelho
Copy link
Contributor

igormcoelho commented Jul 16, 2019

Hal, offchain usage is enough for you? I agree, we would need an immutable storage snapshot (or a clone) to iterate over. A MPT would help too, as it is time synced.

Could you put (offchain) label on this title? So we discuss offchain solutions here, and onchain on the other thread.

@hal0x2328
Copy link
Contributor Author

I'm not sure why an immutable storage snapshot is needed, it's already possible to get a read-only storage context object in the VM. The iterator is still running in the VM, it's just that the currently executing script value has transitioned from the smart contract back to the entry script post-appcall so we aren't able to request the storage context at this point - but it's still in memory.

@erikzhang
Copy link
Member

var entries = Storage.Find("Hello");
return entries.Where(p => p.Length < 10);

We need to implement something like this.

@lock9
Copy link
Contributor

lock9 commented Jul 18, 2019

@erikzhang what does "Storage.Find" does? What is the difference between "Find" and "Get"? It is used for prefixes only?
If we have a "Potato" key, Find("P") will return "Potato" while Get("P") returns null?

@igormcoelho
Copy link
Contributor

@hal0x2328 that's the whole point of dividing discussion in offchain an onchain.
For offchain, an immutable storage snapshot is needed, since it may change during operation.
For onchain, a readonlystorage is desired to prevent updates during Where processing (and no snapshot is needed, you're right, since we are all on same block).

A helpful thing to implement this is like Shargon said, a Json parser, so we can apply this Where on a implicit conversion to json from the object, applying some delegated filtering... let's keep discussing, things will be clearer.

@shargon
Copy link
Member

shargon commented Jul 19, 2019

If we have a "Potato" key, Find("P") will return "Potato" while Get("P") returns null?

Yes @lock9

@hal0x2328
Copy link
Contributor Author

@hal0x2328 that's the whole point of dividing discussion in offchain an onchain.
For offchain, an immutable storage snapshot is needed, since it may change during operation.

Ok, maybe that's where I'm getting lost. How would you iterate over an enumerator returned by a smart contract offchain? Is this related to some Neo 3.0 code I haven't seen yet?

@steven1227
Copy link
Member

var entries = Storage.Find("Hello");
return entries.Where(p => p.Length < 10);

We need to implement something like this.

Is this feature going to be implemented ? @erikzhang

@erikzhang
Copy link
Member

Is this feature going to be implemented?

Yes, if it is feasible.

@igormcoelho
Copy link
Contributor

Guys, pehaps we have a solution (a safe one)! Let's try to discuss here a little bit: neo-project/neo-vm#190

@lock9 lock9 added design Issue state - Feature accepted but the solution requires a design before being implemented feature Type: Large changes or new features vm New features that affect the Neo Virtual Machine or the Interop layer and removed discussion Initial issue state - proposed but not yet accepted potential feature labels Aug 11, 2019
@igormcoelho
Copy link
Contributor

@erikzhang I'll reopen this. Finally doable with neo-project/neo-vm#190

@igormcoelho igormcoelho reopened this Dec 6, 2019
Thacryba pushed a commit to simplitech/neo that referenced this issue Feb 17, 2020
Thacryba pushed a commit to simplitech/neo that referenced this issue Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Issue state - Feature accepted but the solution requires a design before being implemented feature Type: Large changes or new features vm New features that affect the Neo Virtual Machine or the Interop layer
Projects
None yet
Development

No branches or pull requests

6 participants